Do not copy nullable value type receiver of a constrained call. (#66311)

The check that we used to detect value types at runtime didn’t quite work for
nullable value types due to special boxing behavior for them.
Switching to `typeof(T).IsValueType` check instead.

Fixes #66162.
Related to #63221.

Author: AlekseyTs

src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs

@@ -4579,6 +4579,21 @@ internal static bool HasHome(
                         stackLocalsOpt));
                     goto case BoundKind.ConditionalReceiver;
 
+                case BoundKind.ComplexReceiver:
+                    Debug.Assert(HasHome(
+                        ((BoundComplexReceiver)expression).ValueTypeReceiver,
+                        addressKind,
+                        containingSymbol,
+                        peVerifyCompatEnabled,
+                        stackLocalsOpt));
+                    Debug.Assert(HasHome(
+                        ((BoundComplexReceiver)expression).ReferenceTypeReceiver,
+                        addressKind,
+                        containingSymbol,
+                        peVerifyCompatEnabled,
+                        stackLocalsOpt));
+                    goto case BoundKind.ConditionalReceiver;
+
                 case BoundKind.ConditionalReceiver:
                     //ConditionalReceiver is a noop from Emit point of view. - it represents something that has already been pushed. 
                     //We should never need a temp for it. 

src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml

@@ -1714,8 +1714,8 @@
     <Field Name="Id" Type="int"/>
   </Node>
 
-  <!--  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. 
+  <!--  This node represents a complex receiver for a conditional access.  
+        At runtime, when its type is a non-nullable value type, ValueTypeReceiver should be used as a receiver. 
         Otherwise, ReferenceTypeReceiver should be used.  
         This kind of receiver is created only by SpillSequenceSpiller rewriter. 
   -->
@@ -1725,6 +1725,16 @@
     <Field Name="ReferenceTypeReceiver" Type="BoundExpression" Null="disallow"/>
   </Node>
 
+  <!--  This node represents a complex receiver for a call.  
+        At runtime, when its type is a value type (including nullable value type), ValueTypeReceiver should be used as a receiver. 
+        Otherwise, ReferenceTypeReceiver should be used.  
+  -->
+  <Node Name="BoundComplexReceiver" Base="BoundExpression">
+    <Field Name="Type" Type="TypeSymbol" Override="true" Null="disallow"/>
+    <Field Name="ValueTypeReceiver" Type="BoundExpression" Null="disallow"/>
+    <Field Name="ReferenceTypeReceiver" Type="BoundExpression" Null="disallow"/>
+  </Node>
+
   <Node Name="BoundMethodGroup" Base="BoundMethodOrPropertyGroup">
     <!-- SPEC: A method group is a set of overloaded methods resulting from a member lookup. 
          SPEC: A method group may have an associated instance expression and 

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;

src/Compilers/CSharp/Portable/CodeGen/EmitAddress.cs

@@ -48,6 +48,10 @@ private LocalDefinition EmitAddress(BoundExpression expression, AddressKind addr
                     EmitComplexConditionalReceiverAddress((BoundComplexConditionalReceiver)expression);
                     break;
 
+                case BoundKind.ComplexReceiver:
+                    EmitComplexReceiverAddress((BoundComplexReceiver)expression);
+                    break;
+
                 case BoundKind.Parameter:
                     return EmitParameterAddress((BoundParameter)expression, addressKind);
 

src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs

@@ -316,6 +316,10 @@ private void EmitExpressionCore(BoundExpression expression, bool used)
                     EmitComplexConditionalReceiver((BoundComplexConditionalReceiver)expression, used);
                     break;
 
+                case BoundKind.ComplexReceiver:
+                    EmitComplexReceiver((BoundComplexReceiver)expression, used);
+                    break;
+
                 case BoundKind.PseudoVariable:
                     EmitPseudoVariableValue((BoundPseudoVariable)expression, used);
                     break;
@@ -365,7 +369,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 +420,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 +573,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);
@@ -1752,25 +1815,21 @@ LocalDefinition emitGenericReceiver(BoundCall call, out CallKind callKind)
                     // reference to the stack. So, for a class we need to emit a reference to a temporary
                     // location, rather than to the original location
 
-                    // Struct values are never nulls.
-                    // We will emit a check for such case, but the check is really a JIT-time 
+                    // We will emit a runtime check for a class, but the check is really a JIT-time 
                     // constant since JIT will know if T is a struct or not.
 
-                    // if ((object)default(T) == null) 
+                    // if (!typeof(T).IsValueType)
                     // {
                     //     temp = receiverRef
                     //     receiverRef = ref temp
                     // }
 
-                    object whenNotNullLabel = null;
+                    object whenValueTypeLabel = null;
 
                     if (!receiverType.IsReferenceType)
                     {
-                        // if ((object)default(T) == null) 
-                        EmitDefaultValue(receiverType, true, receiver.Syntax);
-                        EmitBox(receiverType, receiver.Syntax);
-                        whenNotNullLabel = new object();
-                        _builder.EmitBranch(ILOpCode.Brtrue, whenNotNullLabel);
+                        // if (!typeof(T).IsValueType)
+                        whenValueTypeLabel = TryEmitIsValueTypeBranch(receiverType, receiver.Syntax);
                     }
 
                     //     temp = receiverRef
@@ -1780,16 +1839,91 @@ LocalDefinition emitGenericReceiver(BoundCall call, out CallKind callKind)
                     _builder.EmitLocalStore(tempOpt);
                     _builder.EmitLocalAddress(tempOpt);
 
-                    if (whenNotNullLabel is not null)
+                    if (whenValueTypeLabel is not null)
                     {
-                        _builder.MarkLabel(whenNotNullLabel);
+                        _builder.MarkLabel(whenValueTypeLabel);
                     }
                 }
 
                 return tempOpt;
             }
         }
 
+        private object TryEmitIsValueTypeBranch(TypeSymbol type, SyntaxNode syntax)
+        {
+            if (Binder.GetWellKnownTypeMember(_module.Compilation, WellKnownMember.System_Type__GetTypeFromHandle, _diagnostics, syntax: syntax, isOptional: false) is MethodSymbol getTypeFromHandle &&
+                Binder.GetWellKnownTypeMember(_module.Compilation, WellKnownMember.System_Type__get_IsValueType, _diagnostics, syntax: syntax, isOptional: false) is MethodSymbol getIsValueType)
+            {
+                _builder.EmitOpCode(ILOpCode.Ldtoken);
+                EmitSymbolToken(type, syntax);
+                _builder.EmitOpCode(ILOpCode.Call, stackAdjustment: 0); // argument off, return value on
+                EmitSymbolToken(getTypeFromHandle, syntax, null);
+                _builder.EmitOpCode(ILOpCode.Call, stackAdjustment: 0); // instance off, return value on
+                EmitSymbolToken(getIsValueType, syntax, null);
+                var whenValueTypeLabel = new object();
+                _builder.EmitBranch(ILOpCode.Brtrue, whenValueTypeLabel);
+
+                return whenValueTypeLabel;
+            }
+
+            return null;
+        }
+
+        private void EmitComplexReceiver(BoundComplexReceiver expression, bool used)
+        {
+            Debug.Assert(!used);
+            Debug.Assert(!expression.Type.IsReferenceType);
+            Debug.Assert(!expression.Type.IsValueType);
+
+            var receiverType = expression.Type;
+
+            // if (!typeof(T).IsValueType)
+            object whenValueTypeLabel = TryEmitIsValueTypeBranch(receiverType, expression.Syntax);
+
+            EmitExpression(expression.ReferenceTypeReceiver, used);
+            var doneLabel = new object();
+            _builder.EmitBranch(ILOpCode.Br, doneLabel);
+
+            if (whenValueTypeLabel is not null)
+            {
+                if (used)
+                {
+                    _builder.AdjustStack(-1);
+                }
+
+                _builder.MarkLabel(whenValueTypeLabel);
+                EmitExpression(expression.ValueTypeReceiver, used);
+            }
+
+            _builder.MarkLabel(doneLabel);
+        }
+
+        private void EmitComplexReceiverAddress(BoundComplexReceiver expression)
+        {
+            Debug.Assert(!expression.Type.IsReferenceType);
+            Debug.Assert(!expression.Type.IsValueType);
+
+            var receiverType = expression.Type;
+
+            // if (!typeof(T).IsValueType)
+            object whenValueTypeLabel = TryEmitIsValueTypeBranch(receiverType, expression.Syntax);
+
+            var receiverTemp = EmitAddress(expression.ReferenceTypeReceiver, AddressKind.ReadOnly);
+            Debug.Assert(receiverTemp == null);
+            var doneLabel = new Object();
+            _builder.EmitBranch(ILOpCode.Br, doneLabel);
+
+            if (whenValueTypeLabel is not null)
+            {
+                _builder.AdjustStack(-1);
+                _builder.MarkLabel(whenValueTypeLabel);
+                // we will not write through this receiver, but it could be a target of mutating calls
+                EmitReceiverRef(expression.ValueTypeReceiver, AddressKind.Constrained);
+            }
+
+            _builder.MarkLabel(doneLabel);
+        }
+
         internal static bool IsPossibleReferenceTypeReceiverOfConstrainedCall(BoundExpression receiver)
         {
             var receiverType = receiver.Type;
@@ -1811,7 +1945,9 @@ internal static bool ReceiverIsKnownToReferToTempIfReferenceType(BoundExpression
 
             if (receiver is
                     BoundLocal { LocalSymbol.IsKnownToReferToTempIfReferenceType: true } or
-                    BoundComplexConditionalReceiver)
+                    BoundComplexConditionalReceiver or
+                    BoundComplexReceiver or
+                    BoundConditionalReceiver { Type: { IsReferenceType: false, IsValueType: false } })
             {
                 return true;
             }

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;
@@ -1563,6 +1564,43 @@ public override BoundNode VisitComplexConditionalReceiver(BoundComplexConditiona
             return node.Update(valueTypeReceiver, referenceTypeReceiver, node.Type);
         }
 
+        public override BoundNode VisitComplexReceiver(BoundComplexReceiver node)
+        {
+            EnsureOnlyEvalStack();
+
+            var origStack = StackDepth();
+
+            PushEvalStack(null, ExprContext.None);
+
+            var cookie = GetStackStateCookie(); // implicit goto here 
+
+            SetStackDepth(origStack); // consequence is evaluated with original stack 
+
+            var valueTypeReceiver = (BoundExpression)this.Visit(node.ValueTypeReceiver);
+
+            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 
+
+            return node.Update(valueTypeReceiver, referenceTypeReceiver, node.Type);
+        }
+
         public override BoundNode VisitUnaryOperator(BoundUnaryOperator node)
         {
             // checked(-x) is emitted as "0 - x"
@@ -2214,7 +2252,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();
         }

src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs

@@ -2940,6 +2940,20 @@ public override BoundNode VisitComplexConditionalReceiver(BoundComplexConditiona
             return null;
         }
 
+        public override BoundNode VisitComplexReceiver(BoundComplexReceiver node)
+        {
+            var savedState = this.State.Clone();
+
+            VisitRvalue(node.ValueTypeReceiver);
+            Join(ref this.State, ref savedState);
+
+            savedState = this.State.Clone();
+            VisitRvalue(node.ReferenceTypeReceiver);
+            Join(ref this.State, ref savedState);
+
+            return null;
+        }
+
         public override BoundNode VisitSequence(BoundSequence node)
         {
             var sideEffects = node.SideEffects;