Preserve changes that are still relevant after revert of dotnet#66311

Related to dotnet#63221.
AlekseyTs · Jan 17, 2023 · ed3ed3a · ed3ed3a
1 parent 368fb1e
commit ed3ed3a
Show file tree

Hide file tree

Showing 14 changed files with 479 additions and 100 deletions.
diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml
@@ -1717,7 +1717,7 @@
   <!--  This node represents a complex receiver for a call, or a conditional access.  
         At runtime, when its type is a value type, ValueTypeReceiver should be used as a receiver. 
         Otherwise, ReferenceTypeReceiver should be used.  
-        This kind of receiver is created only by SpillSequenceSpiller rewriter. 
+        This kind of receiver is created only by SpillSequenceSpiller and LocalRewriter. 
   -->
   <Node Name="BoundComplexConditionalReceiver" Base="BoundExpression">
     <Field Name="Type" Type="TypeSymbol" Override="true" Null="disallow"/>

diff --git a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator.cs b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator.cs
@@ -224,14 +224,14 @@ public void Generate(
             hasStackAlloc = _sawStackalloc;
             Debug.Assert(_asyncCatchHandlerOffset >= 0);
 
-            asyncCatchHandlerOffset = _builder.GetILOffsetFromMarker(_asyncCatchHandlerOffset);
+            asyncCatchHandlerOffset = _diagnostics.HasAnyErrors() ? -1 : _builder.GetILOffsetFromMarker(_asyncCatchHandlerOffset);
 
             ArrayBuilder<int> yieldPoints = _asyncYieldPoints;
             ArrayBuilder<int> resumePoints = _asyncResumePoints;
 
             Debug.Assert((yieldPoints == null) == (resumePoints == null));
 
-            if (yieldPoints == null)
+            if (yieldPoints == null || _diagnostics.HasAnyErrors())
             {
                 asyncYieldPoints = ImmutableArray<int>.Empty;
                 asyncResumePoints = ImmutableArray<int>.Empty;

diff --git a/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs b/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs
@@ -365,7 +365,11 @@ private void EmitComplexConditionalReceiver(BoundComplexConditionalReceiver expr
 
             EmitExpression(expression.ReferenceTypeReceiver, used);
             _builder.EmitBranch(ILOpCode.Br, doneLabel);
-            _builder.AdjustStack(-1);
+
+            if (used)
+            {
+                _builder.AdjustStack(-1);
+            }
 
             _builder.MarkLabel(whenValueTypeLabel);
             EmitExpression(expression.ValueTypeReceiver, used);
@@ -412,7 +416,8 @@ private void EmitLoweredConditionalAccessExpression(BoundLoweredConditionalAcces
                                    ((TypeParameterSymbol)receiverType).EffectiveInterfacesNoUseSiteDiagnostics.IsEmpty) || // This could be a nullable value type, which must be copied in order to not mutate the original value
                                    LocalRewriter.CanChangeValueBetweenReads(receiver, localsMayBeAssignedOrCaptured: false) ||
                                    (receiverType.IsReferenceType && receiverType.TypeKind == TypeKind.TypeParameter) ||
-                                   (receiver.Kind == BoundKind.Local && IsStackLocal(((BoundLocal)receiver).LocalSymbol));
+                                   (receiver.Kind == BoundKind.Local && IsStackLocal(((BoundLocal)receiver).LocalSymbol)) ||
+                                   (notConstrained && IsConditionalConstrainedCallThatMustUseTempForReferenceTypeReceiverWalker.Analyze(expression));
 
             // ===== RECEIVER
             if (nullCheckOnCopy)
@@ -564,6 +569,60 @@ private void EmitLoweredConditionalAccessExpression(BoundLoweredConditionalAcces
             }
         }
 
+        private sealed class IsConditionalConstrainedCallThatMustUseTempForReferenceTypeReceiverWalker : BoundTreeWalkerWithStackGuardWithoutRecursionOnTheLeftOfBinaryOperator
+        {
+            private readonly BoundLoweredConditionalAccess _conditionalAccess;
+            private bool? _result;
+
+            private IsConditionalConstrainedCallThatMustUseTempForReferenceTypeReceiverWalker(BoundLoweredConditionalAccess conditionalAccess)
+                : base()
+            {
+                _conditionalAccess = conditionalAccess;
+            }
+
+            public static bool Analyze(BoundLoweredConditionalAccess conditionalAccess)
+            {
+                var walker = new IsConditionalConstrainedCallThatMustUseTempForReferenceTypeReceiverWalker(conditionalAccess);
+                walker.Visit(conditionalAccess.WhenNotNull);
+                Debug.Assert(walker._result.HasValue);
+                return walker._result.GetValueOrDefault();
+            }
+
+            public override BoundNode Visit(BoundNode node)
+            {
+                if (_result.HasValue)
+                {
+                    return null;
+                }
+
+                return base.Visit(node);
+            }
+
+            public override BoundNode VisitCall(BoundCall node)
+            {
+                if (node.ReceiverOpt is BoundConditionalReceiver { Id: var id } && id == _conditionalAccess.Id)
+                {
+                    Debug.Assert(!_result.HasValue);
+                    _result = !IsSafeToDereferenceReceiverRefAfterEvaluatingArguments(node.Arguments);
+                    return null;
+                }
+
+                return base.VisitCall(node);
+            }
+
+            public override BoundNode VisitConditionalReceiver(BoundConditionalReceiver node)
+            {
+                if (node.Id == _conditionalAccess.Id)
+                {
+                    Debug.Assert(!_result.HasValue);
+                    _result = false;
+                    return null;
+                }
+
+                return base.VisitConditionalReceiver(node);
+            }
+        }
+
         private void EmitConditionalReceiver(BoundConditionalReceiver expression, bool used)
         {
             Debug.Assert(!expression.Type.IsValueType);
@@ -1809,7 +1868,8 @@ internal static bool ReceiverIsKnownToReferToTempIfReferenceType(BoundExpression
 
             if (receiver is
                     BoundLocal { LocalSymbol.IsKnownToReferToTempIfReferenceType: true } or
-                    BoundComplexConditionalReceiver)
+                    BoundComplexConditionalReceiver or
+                    BoundConditionalReceiver { Type: { IsReferenceType: false, IsValueType: false } })
             {
                 return true;
             }

diff --git a/src/Compilers/CSharp/Portable/CodeGen/Optimizer.cs b/src/Compilers/CSharp/Portable/CodeGen/Optimizer.cs
@@ -9,6 +9,7 @@
 using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Linq;
+using System.Runtime.CompilerServices;
 using System.Text;
 using Microsoft.CodeAnalysis.Collections;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
@@ -1556,6 +1557,19 @@ public override BoundNode VisitComplexConditionalReceiver(BoundComplexConditiona
             EnsureStackState(cookie); // implicit label here 
 
             SetStackDepth(origStack); // alternative is evaluated with original stack 
+
+            var unwrappedSequence = node.ReferenceTypeReceiver;
+
+            while (unwrappedSequence is BoundSequence sequence)
+            {
+                unwrappedSequence = sequence.Value;
+            }
+
+            if (unwrappedSequence is BoundLocal { LocalSymbol: { } localSymbol })
+            {
+                ShouldNotSchedule(localSymbol);
+            }
+
             var referenceTypeReceiver = (BoundExpression)this.Visit(node.ReferenceTypeReceiver);
 
             EnsureStackState(cookie); // implicit label here 
@@ -2214,7 +2228,14 @@ internal override SyntaxNode ScopeDesignatorOpt
             get { return null; }
         }
 
-        internal override LocalSymbol WithSynthesizedLocalKindAndSyntax(SynthesizedLocalKind kind, SyntaxNode syntax)
+        internal override LocalSymbol WithSynthesizedLocalKindAndSyntax(
+            SynthesizedLocalKind kind, SyntaxNode syntax
+#if DEBUG
+            ,
+            [CallerLineNumber] int createdAtLineNumber = 0,
+            [CallerFilePath] string createdAtFilePath = null
+#endif
+            )
         {
             throw new NotImplementedException();
         }

diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.PlaceholderLocal.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.PlaceholderLocal.cs
@@ -6,6 +6,7 @@
 
 using System.Collections.Immutable;
 using System.Diagnostics;
+using System.Runtime.CompilerServices;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Roslyn.Utilities;
 
@@ -60,7 +61,14 @@ public override bool Equals(Symbol obj, TypeCompareKind compareKind)
             internal override ConstantValue GetConstantValue(SyntaxNode node, LocalSymbol inProgress, BindingDiagnosticBag diagnostics = null) => null;
             internal override ImmutableBindingDiagnostic<AssemblySymbol> GetConstantValueDiagnostics(BoundExpression boundInitValue) => ImmutableBindingDiagnostic<AssemblySymbol>.Empty;
             internal override SyntaxNode GetDeclaratorSyntax() => throw ExceptionUtilities.Unreachable();
-            internal override LocalSymbol WithSynthesizedLocalKindAndSyntax(SynthesizedLocalKind kind, SyntaxNode syntax) => throw ExceptionUtilities.Unreachable();
+            internal override LocalSymbol WithSynthesizedLocalKindAndSyntax(
+                SynthesizedLocalKind kind, SyntaxNode syntax
+#if DEBUG
+                ,
+                [CallerLineNumber] int createdAtLineNumber = 0,
+                [CallerFilePath] string createdAtFilePath = null
+#endif
+                ) => throw ExceptionUtilities.Unreachable();
             internal override ScopedKind Scope => ScopedKind.None;
         }
     }

diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs
@@ -694,29 +694,23 @@ private void ReferToTempIfReferenceTypeReceiver(BoundLocal receiverTemp, ref Bou
                 temps.Add(intermediateRef.LocalSymbol);
                 extraRefInitialization = assignmentToTemp.Update(intermediateRef, assignmentToTemp.Right, assignmentToTemp.IsRef, assignmentToTemp.Type);
 
-                // If condition `(object)default(T) != null` is true at execution time,
-                // the T is a value type. And it is a reference type otherwise.
-                var isValueTypeCheck = _factory.ObjectNotEqual(
-                                            _factory.Convert(_factory.SpecialType(SpecialType.System_Object), _factory.Default(receiverType)),
-                                            _factory.Null(_factory.SpecialType(SpecialType.System_Object)));
-
                 // `receiverTemp` initialization is adjusted as follows:
                 // If we are dealing with a value type, use value of the intermediate ref.
                 // Otherwise, use an address of a temp where we store the underlying reference type instance.
                 assignmentToTemp =
                     assignmentToTemp.Update(
                         assignmentToTemp.Left,
-                        _factory.Conditional(
-                            isValueTypeCheck,
-                            intermediateRef,
-                            _factory.Sequence(new BoundExpression[] { _factory.AssignmentExpression(cache, intermediateRef) }, cache),
-                            receiverType,
-                            isRef: true),
+#pragma warning disable format
+                        new BoundComplexConditionalReceiver(receiverTemp.Syntax,
+                                                            intermediateRef,
+                                                            _factory.Sequence(new BoundExpression[] { _factory.AssignmentExpression(cache, intermediateRef) }, cache),
+                                                            receiverType) { WasCompilerGenerated = true },
+#pragma warning restore format
                         assignmentToTemp.IsRef,
                         assignmentToTemp.Type);
 
                 // SpillSequenceSpiller should be able to recognize this node in order to handle its spilling. 
-                Debug.Assert(SpillSequenceSpiller.IsComplexConditionalInitializationOfReceiverRef(assignmentToTemp, out _, out _, out _, out _, out _));
+                Debug.Assert(SpillSequenceSpiller.IsComplexConditionalInitializationOfReceiverRef(assignmentToTemp, out _, out _, out _, out _));
             }
             else
             {

diff --git a/src/Compilers/CSharp/Portable/Lowering/SpillSequenceSpiller.cs b/src/Compilers/CSharp/Portable/Lowering/SpillSequenceSpiller.cs
@@ -356,15 +356,14 @@ private BoundExpression Spill(
                                 IsComplexConditionalInitializationOfReceiverRef(
                                     assignment,
                                     out LocalSymbol receiverRefLocal,
-                                    out BoundBinaryOperator isValueTypeCheck,
-                                    out BoundAssignmentOperator referenceTypeReceiverCloning,
+                                    out BoundComplexConditionalReceiver complexReceiver,
                                     out BoundLocal valueTypeReceiver,
                                     out BoundLocal referenceTypeReceiver))
                             {
                                 Debug.Assert(receiverRefLocal.IsKnownToReferToTempIfReferenceType);
-                                builder.AddStatement(_F.If(_F.Not(isValueTypeCheck), _F.ExpressionStatement(referenceTypeReceiverCloning)));
+                                builder.AddStatement(_F.ExpressionStatement(complexReceiver));
 
-                                _receiverSubstitution.Add(receiverRefLocal, _F.ComplexConditionalReceiver(valueTypeReceiver, referenceTypeReceiver));
+                                _receiverSubstitution.Add(receiverRefLocal, complexReceiver.Update(valueTypeReceiver, referenceTypeReceiver, complexReceiver.Type));
                                 return null;
                             }
                             else
@@ -458,33 +457,18 @@ private BoundExpression Spill(
         internal static bool IsComplexConditionalInitializationOfReceiverRef(
             BoundAssignmentOperator assignment,
             out LocalSymbol outReceiverRefLocal,
-            out BoundBinaryOperator outIsValueTypeCheck,
-            out BoundAssignmentOperator outReferenceTypeReceiverCloning,
+            out BoundComplexConditionalReceiver outComplexReceiver,
             out BoundLocal outValueTypeReceiver,
             out BoundLocal outReferenceTypeReceiver)
         {
             if (assignment is
                 {
                     IsRef: true,
                     Left: BoundLocal { LocalSymbol: { SynthesizedKind: SynthesizedLocalKind.LoweringTemp, RefKind: RefKind.Ref } receiverRefLocal },
-                    Right: BoundConditionalOperator
+                    Right: BoundComplexConditionalReceiver
                     {
-                        IsRef: true,
-                        Condition: BoundBinaryOperator
-                        {
-                            OperatorKind: BinaryOperatorKind.ObjectNotEqual,
-                            Left: BoundConversion
-                            {
-                                Conversion: { IsUserDefined: false },
-                                Operand: BoundDefaultExpression { Type: var typeOfDefault },
-                                Type.SpecialType: SpecialType.System_Object
-                            },
-                            Right: BoundLiteral { ConstantValueOpt.IsNull: true, Type.SpecialType: SpecialType.System_Object }
-                        } isValueTypeCheck,
-
-                        Consequence: BoundLocal { LocalSymbol: { SynthesizedKind: SynthesizedLocalKind.LoweringTemp, RefKind: RefKind.Ref } } valueTypeReceiver,
-
-                        Alternative: BoundSequence
+                        ValueTypeReceiver: BoundLocal { LocalSymbol: { SynthesizedKind: SynthesizedLocalKind.LoweringTemp, RefKind: RefKind.Ref } } valueTypeReceiver,
+                        ReferenceTypeReceiver: BoundSequence
                         {
                             Locals.IsEmpty: true,
                             SideEffects:
@@ -494,11 +478,11 @@ internal static bool IsComplexConditionalInitializationOfReceiverRef(
                                 IsRef: false,
                                 Left: BoundLocal { LocalSymbol: { SynthesizedKind: SynthesizedLocalKind.LoweringTemp, RefKind: RefKind.None } referenceTypeClone },
                                 Right: BoundLocal { LocalSymbol: { SynthesizedKind: SynthesizedLocalKind.LoweringTemp, RefKind: RefKind.Ref } originalReceiverReference }
-                            } referenceTypeReceiverCloning
+                            }
                             ],
                             Value: BoundLocal { LocalSymbol: { SynthesizedKind: SynthesizedLocalKind.LoweringTemp, RefKind: RefKind.None } } referenceTypeReceiver
                         }
-                    }
+                    } complexReceiver,
                 }
                 && (object)referenceTypeClone == referenceTypeReceiver.LocalSymbol
                 && (object)originalReceiverReference == valueTypeReceiver.LocalSymbol
@@ -507,22 +491,19 @@ internal static bool IsComplexConditionalInitializationOfReceiverRef(
                 && receiverRefLocal.Type.IsTypeParameter()
                 && !receiverRefLocal.Type.IsReferenceType
                 && !receiverRefLocal.Type.IsValueType
-                && typeOfDefault.Equals(receiverRefLocal.Type, TypeCompareKind.AllIgnoreOptions)
                 && valueTypeReceiver.Type.Equals(receiverRefLocal.Type, TypeCompareKind.AllIgnoreOptions)
                 && referenceTypeReceiver.Type.Equals(receiverRefLocal.Type, TypeCompareKind.AllIgnoreOptions)
             )
             {
                 outReceiverRefLocal = receiverRefLocal;
-                outIsValueTypeCheck = isValueTypeCheck;
-                outReferenceTypeReceiverCloning = referenceTypeReceiverCloning;
+                outComplexReceiver = complexReceiver;
                 outValueTypeReceiver = valueTypeReceiver;
                 outReferenceTypeReceiver = referenceTypeReceiver;
                 return true;
             }
 
             outReceiverRefLocal = null;
-            outIsValueTypeCheck = null;
-            outReferenceTypeReceiverCloning = null;
+            outComplexReceiver = null;
             outValueTypeReceiver = null;
             outReferenceTypeReceiver = null;
             return false;
@@ -998,17 +979,15 @@ public override BoundNode VisitCall(BoundCall node)
                     // reference to the stack. So, for a class we need to emit a reference to a temporary
                     // location, rather than to the original location
 
-                    // If condition `(object)default(T) != null` is true at execution time,
-                    // the T is a value type. And it is a reference type otherwise.
-                    var isValueTypeCheck = _F.ObjectNotEqual(
-                                                _F.Convert(_F.SpecialType(SpecialType.System_Object), _F.Default(receiverType)),
-                                                _F.Null(_F.SpecialType(SpecialType.System_Object)));
+                    var save_Syntax = _F.Syntax;
+                    _F.Syntax = node.Syntax;
 
                     var cache = _F.Local(_F.SynthesizedLocal(receiverType));
                     receiverBuilder.AddLocal(cache.LocalSymbol);
-                    receiverBuilder.AddStatement(_F.If(_F.Not(isValueTypeCheck), _F.Assignment(cache, receiver)));
+                    receiverBuilder.AddStatement(_F.ExpressionStatement(new BoundComplexConditionalReceiver(node.Syntax, cache, _F.Sequence(new[] { _F.AssignmentExpression(cache, receiver) }, cache), receiverType) { WasCompilerGenerated = true }));
 
                     receiver = _F.ComplexConditionalReceiver(receiver, cache);
+                    _F.Syntax = save_Syntax;
                 }
 
                 receiverBuilder.Include(builder);

diff --git a/src/Compilers/CSharp/Portable/Symbols/LocalSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/LocalSymbol.cs
@@ -6,6 +6,7 @@
 
 using System.Collections.Immutable;
 using System.Diagnostics;
+using System.Runtime.CompilerServices;
 using Microsoft.CodeAnalysis.CodeGen;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Symbols;
@@ -37,7 +38,14 @@ internal abstract SynthesizedLocalKind SynthesizedKind
         /// </summary>
         internal abstract SyntaxNode ScopeDesignatorOpt { get; }
 
-        internal abstract LocalSymbol WithSynthesizedLocalKindAndSyntax(SynthesizedLocalKind kind, SyntaxNode syntax);
+        internal abstract LocalSymbol WithSynthesizedLocalKindAndSyntax(
+            SynthesizedLocalKind kind, SyntaxNode syntax
+#if DEBUG
+            ,
+            [CallerLineNumber] int createdAtLineNumber = 0,
+            [CallerFilePath] string createdAtFilePath = null
+#endif
+            );
 
         internal abstract bool IsImportedFromMetadata
         {