Extensions: fix lowering of implicit conversion on receiver in pattern-based scenarios (#78685)

Author: Julien Couvreur

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

@@ -3030,13 +3030,13 @@ private bool CheckInvocationArgMixingWithUpdatedRules(
                 }
             }
 
-            inferDeclarationExpressionValEscape();
+            inferDeclarationExpressionValEscape(argsOpt, localScopeDepth, escapeValues);
 
             mixableArguments.Free();
             escapeValues.Free();
             return valid;
 
-            void inferDeclarationExpressionValEscape()
+            void inferDeclarationExpressionValEscape(ImmutableArray<BoundExpression> argsOpt, SafeContext localScopeDepth, ArrayBuilder<EscapeValue> escapeValues)
             {
                 // find the widest scope that arguments could safely escape to.
                 // use this scope as the inferred STE of declaration expressions.

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.PatternLocalRewriter.cs

@@ -152,7 +152,8 @@ protected BoundExpression LowerEvaluation(BoundDagEvaluation evaluation)
                             PropertySymbol property = p.Property;
                             var outputTemp = new BoundDagTemp(p.Syntax, property.Type, p);
                             BoundExpression output = _tempAllocator.GetTemp(outputTemp);
-                            input = _factory.ConvertReceiverForExtensionMemberIfNeeded(property, input);
+                            // Tracked by https://github.com/dotnet/roslyn/issues/76130 : Consider preserving the BoundConversion from initial binding instead of using markAsChecked here
+                            input = _localRewriter.ConvertReceiverForExtensionMemberIfNeeded(property, input, markAsChecked: true);
                             return _factory.AssignmentExpression(output, _localRewriter.MakePropertyAccess(_factory.Syntax, input, property, LookupResultKind.Viable, property.Type, isLeftOfAssignment: false));
                         }
 
@@ -191,7 +192,8 @@ void addArg(RefKind refKind, BoundExpression expression)
                                 addArg(RefKind.Out, _tempAllocator.GetTemp(outputTemp));
                             }
 
-                            receiver = _factory.ConvertReceiverForExtensionMemberIfNeeded(method, receiver);
+                            // Tracked by https://github.com/dotnet/roslyn/issues/76130 : Consider preserving the BoundConversion from initial binding instead of using markAsChecked here
+                            receiver = _localRewriter.ConvertReceiverForExtensionMemberIfNeeded(method, receiver, markAsChecked: true);
                             return _factory.Call(receiver, method, refKindBuilder.ToImmutableAndFree(), argBuilder.ToImmutableAndFree());
                         }
 

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs

@@ -1136,6 +1136,25 @@ private CompoundUseSiteInfo<AssemblySymbol> GetNewCompoundUseSiteInfo()
             return new CompoundUseSiteInfo<AssemblySymbol>(_diagnostics, _compilation.Assembly);
         }
 
+        private BoundExpression ConvertReceiverForExtensionMemberIfNeeded(Symbol member, BoundExpression receiver, bool markAsChecked)
+        {
+            if (member.GetIsNewExtensionMember())
+            {
+                Debug.Assert(!member.IsStatic);
+                ParameterSymbol? extensionParameter = member.ContainingType.ExtensionParameter;
+                Debug.Assert(extensionParameter is not null);
+#if DEBUG
+                var discardedUseSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;
+                Debug.Assert(Conversions.IsValidExtensionMethodThisArgConversion(this._compilation.Conversions.ClassifyConversionFromType(receiver.Type, extensionParameter.Type, isChecked: false, ref discardedUseSiteInfo)));
+#endif
+
+                // We don't need to worry about checked context because only implicit conversions are allowed on the receiver of an extension member
+                return MakeConversionNode(receiver, extensionParameter.Type, @checked: false, acceptFailingConversion: false, markAsChecked: markAsChecked);
+            }
+
+            return receiver;
+        }
+
 #if DEBUG
         /// <summary>
         /// Note: do not use a static/singleton instance of this type, as it holds state.

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Conversion.cs

@@ -781,6 +781,10 @@ bool IsInRange(SpecialType type, SpecialType low, SpecialType high) =>
         /// (e.g. if the default value was specified by an attribute and was, therefore, not checked by the compiler).
         /// Set acceptFailingConversion if you want to see default(rewrittenType) in such cases.
         /// The error will be suppressed only for conversions from <see cref="decimal"/> or <see cref="DateTime"/>.
+        ///
+        /// Generally, conversions should be checked and BoundConversion nodes created during initial binding and re-used in lowering. But in some cases,
+        /// we do not preserve the conversion node. In such case, <paramref name="markAsChecked"/> is set to true
+        /// to indicate that the conversion has been previously checked during initial binding.
         /// </remarks>
         private BoundExpression MakeConversionNode(BoundExpression rewrittenOperand, TypeSymbol rewrittenType, bool @checked, bool acceptFailingConversion = false, bool markAsChecked = false)
         {

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs

@@ -354,8 +354,9 @@ private BoundStatement InitializeFixedStatementGetPinnable(
                 callReceiver = initializerExpr;
             }
 
+            // Tracked by https://github.com/dotnet/roslyn/issues/76130 : Consider preserving the BoundConversion from initial binding instead of using markAsChecked here
             // .GetPinnable()
-            callReceiver = factory.ConvertReceiverForExtensionMemberIfNeeded(getPinnableMethod, callReceiver);
+            callReceiver = this.ConvertReceiverForExtensionMemberIfNeeded(getPinnableMethod, callReceiver, markAsChecked: true);
             var getPinnableCall = getPinnableMethod.IsStatic ?
                 factory.Call(null, getPinnableMethod, callReceiver) :
                 factory.Call(callReceiver, getPinnableMethod);

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ObjectOrCollectionInitializerExpression.cs

@@ -702,7 +702,8 @@ private BoundExpression MakeObjectInitializerMemberAccess(
                          _compilation.Conversions.HasImplicitConversionToOrImplementsVarianceCompatibleInterface(rewrittenReceiver.Type, memberSymbol.ContainingType, ref discardedUseSiteInfo, out _));
             // It is possible there are use site diagnostics from the above, but none that we need report as we aren't generating code for the conversion
 #endif
-            rewrittenReceiver = _factory.ConvertReceiverForExtensionMemberIfNeeded(memberSymbol, rewrittenReceiver);
+            // Tracked by https://github.com/dotnet/roslyn/issues/76130 : Consider preserving the BoundConversion from initial binding instead of using markAsChecked here
+            rewrittenReceiver = this.ConvertReceiverForExtensionMemberIfNeeded(memberSymbol, rewrittenReceiver, markAsChecked: true);
 
             switch (memberSymbol.Kind)
             {

src/Compilers/CSharp/Portable/Lowering/SyntheticBoundNodeFactory.cs

@@ -431,24 +431,6 @@ public BoundExpression AssignmentExpression(BoundExpression left, BoundExpressio
             return AssignmentExpression(Syntax, left, right, isRef: isRef, wasCompilerGenerated: true);
         }
 
-        public BoundExpression ConvertReceiverForExtensionMemberIfNeeded(Symbol member, BoundExpression receiver)
-        {
-            if (member.GetIsNewExtensionMember())
-            {
-                Debug.Assert(!member.IsStatic);
-                ParameterSymbol? extensionParameter = member.ContainingType.ExtensionParameter;
-                Debug.Assert(extensionParameter is not null);
-#if DEBUG
-                var discardedUseSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;
-                Debug.Assert(Conversions.IsValidExtensionMethodThisArgConversion(this.Compilation.Conversions.ClassifyConversionFromType(receiver.Type, extensionParameter.Type, isChecked: false, ref discardedUseSiteInfo)));
-#endif
-
-                return this.Convert(extensionParameter.Type, receiver);
-            }
-
-            return receiver;
-        }
-
         /// <summary>
         /// Creates a general assignment that might be instrumented.
         /// </summary>