Hold Receiver directly in bound node for implicit indexer access

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

@@ -7565,15 +7565,16 @@ private BoundExpression BindArrayAccess(SyntaxNode node, BoundExpression expr, A
                 Debug.Assert(convertedArguments.Length == 1);
 
                 var int32 = GetSpecialType(SpecialType.System_Int32, diagnostics, node);
-                var receiverPlaceholder = new BoundImplicitIndexerReceiverPlaceholder(expr.Syntax, GetValEscape(expr, LocalScopeDepth), expr.Type) { WasCompilerGenerated = true };
+                var receiverPlaceholder = new BoundImplicitIndexerReceiverPlaceholder(expr.Syntax, GetValEscape(expr, LocalScopeDepth), isEquivalentToThisReference: expr.IsEquivalentToThisReference, expr.Type) { WasCompilerGenerated = true };
                 var argumentPlaceholders = ImmutableArray.Create(new BoundImplicitIndexerValuePlaceholder(convertedArguments[0].Syntax, int32) { WasCompilerGenerated = true });
 
                 return new BoundImplicitIndexerAccess(
                     node,
+                    receiver: expr,
                     argument: convertedArguments[0],
                     lengthOrCountAccess: new BoundArrayLength(node, receiverPlaceholder, int32) { WasCompilerGenerated = true },
                     receiverPlaceholder,
-                    indexerOrSliceAccess: new BoundArrayAccess(node, expr, ImmutableArray<BoundExpression>.CastUp(argumentPlaceholders), resultType),
+                    indexerOrSliceAccess: new BoundArrayAccess(node, receiverPlaceholder, ImmutableArray<BoundExpression>.CastUp(argumentPlaceholders), resultType) { WasCompilerGenerated = true },
                     argumentPlaceholders,
                     resultType);
             }
@@ -8103,8 +8104,8 @@ private bool TryBindIndexOrRangeImplicitIndexer(
 
             bool argIsIndex = argIsIndexNotRange.Value();
             var receiverValEscape = GetValEscape(receiver, LocalScopeDepth);
-            var receiverPlaceholder = new BoundImplicitIndexerReceiverPlaceholder(receiver.Syntax, receiverValEscape, receiver.Type) { WasCompilerGenerated = true };
-            if (!TryBindIndexOrRangeImplicitIndexerParts(syntax, receiverPlaceholder, receiver, argIsIndex: argIsIndex,
+            var receiverPlaceholder = new BoundImplicitIndexerReceiverPlaceholder(receiver.Syntax, receiverValEscape, isEquivalentToThisReference: receiver.IsEquivalentToThisReference, receiver.Type) { WasCompilerGenerated = true };
+            if (!TryBindIndexOrRangeImplicitIndexerParts(syntax, receiverPlaceholder, argIsIndex: argIsIndex,
                     out var lengthOrCountAccess, out var indexerOrSliceAccess, out var argumentPlaceholders, diagnostics))
             {
                 return false;
@@ -8116,6 +8117,7 @@ private bool TryBindIndexOrRangeImplicitIndexer(
 
             implicitIndexerAccess = new BoundImplicitIndexerAccess(
                 syntax,
+                receiver: receiver,
                 argument: BindToNaturalType(argument, diagnostics),
                 lengthOrCountAccess: lengthOrCountAccess,
                 receiverPlaceholder,
@@ -8152,13 +8154,10 @@ void checkWellKnown(WellKnownMember member)
 
         /// <summary>
         /// Finds pattern-based implicit indexer and Length/Count property.
-        /// We'll use the receiver placeholder to bind the length access, but the real
-        /// receiver to bind the indexer access.
         /// </summary>
         private bool TryBindIndexOrRangeImplicitIndexerParts(
             SyntaxNode syntax,
-            BoundExpression receiverPlaceholder,
-            BoundExpression receiver,
+            BoundImplicitIndexerReceiverPlaceholder receiverPlaceholder,
             bool argIsIndex,
             [NotNullWhen(true)] out BoundExpression? lengthOrCountAccess,
             [NotNullWhen(true)] out BoundExpression? indexerOrSliceAccess,
@@ -8175,8 +8174,8 @@ private bool TryBindIndexOrRangeImplicitIndexerParts(
             // 2. For Index: Has an accessible indexer with a single int parameter
             //    For Range: Has an accessible Slice method that takes two int parameters
 
-            if (TryBindLengthOrCount(syntax, receiverPlaceholder, receiver, out lengthOrCountAccess, diagnostics) &&
-                tryBindUnderlyingIndexerOrSliceAccess(syntax, receiver, argIsIndex, out indexerOrSliceAccess, out argumentPlaceholders, diagnostics))
+            if (TryBindLengthOrCount(syntax, receiverPlaceholder, out lengthOrCountAccess, diagnostics) &&
+                tryBindUnderlyingIndexerOrSliceAccess(syntax, receiverPlaceholder, argIsIndex, out indexerOrSliceAccess, out argumentPlaceholders, diagnostics))
             {
                 return true;
             }
@@ -8191,7 +8190,7 @@ private bool TryBindIndexOrRangeImplicitIndexerParts(
             // - for Range indexer, this will find `Slice(int, int)` or `string.Substring(int, int)`.
             bool tryBindUnderlyingIndexerOrSliceAccess(
                 SyntaxNode syntax,
-                BoundExpression receiver,
+                BoundImplicitIndexerReceiverPlaceholder receiver,
                 bool argIsIndex,
                 [NotNullWhen(true)] out BoundExpression? indexerOrSliceAccess,
                 out ImmutableArray<BoundImplicitIndexerValuePlaceholder> argumentPlaceholders,
@@ -8324,8 +8323,7 @@ void makeCall(SyntaxNode syntax, BoundExpression receiver, MethodSymbol method,
 
         private bool TryBindLengthOrCount(
             SyntaxNode syntax,
-            BoundExpression receiverPlaceholder,
-            BoundExpression? receiver,
+            BoundValuePlaceholderBase receiverPlaceholder,
             out BoundExpression lengthOrCountAccess,
             BindingDiagnosticBag diagnostics)
         {
@@ -8338,9 +8336,6 @@ private bool TryBindLengthOrCount(
                 lengthOrCountAccess = BindPropertyAccess(syntax, receiverPlaceholder, lengthOrCountProperty, diagnostics, lookupResult.Kind, hasErrors: false).MakeCompilerGenerated();
                 lengthOrCountAccess = CheckValue(lengthOrCountAccess, BindValueKind.RValue, diagnostics);
 
-                // CheckValue does not handle placeholders well yet, so we're doing some extra check for `this` receiver
-                CheckImplicitThisCopyInReadOnlyMember(receiver, lengthOrCountProperty.GetOwnOrInheritedGetMethod(), diagnostics);
-
                 lookupResult.Free();
                 return true;
             }

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

@@ -1464,7 +1464,7 @@ internal bool CheckImplicitThisCopyInReadOnlyMember(BoundExpression receiver, Me
         {
             // For now we are warning only in implicit copy scenarios that are only possible with readonly members.
             // Eventually we will warn on implicit value copies in more scenarios. See https://github.com/dotnet/roslyn/issues/33968.
-            if (receiver is BoundThisReference &&
+            if (receiver?.IsEquivalentToThisReference == true &&
                 receiver.Type.IsValueType &&
                 ContainingMemberOrLambda is MethodSymbol containingMethod &&
                 containingMethod.IsEffectivelyReadOnly &&

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

@@ -359,7 +359,7 @@ private bool BindLengthAndIndexerForListPattern(SyntaxNode node, TypeSymbol inpu
             }
             else
             {
-                hasErrors |= !TryBindLengthOrCount(node, receiverPlaceholder, receiver: null, out lengthAccess, diagnostics);
+                hasErrors |= !TryBindLengthOrCount(node, receiverPlaceholder, out lengthAccess, diagnostics);
             }
 
             var analyzedArguments = AnalyzedArguments.GetInstance();

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

@@ -1566,7 +1566,22 @@ internal static PropertySymbol GetPropertySymbol(BoundExpression expr, out Bound
                 case BoundKind.ImplicitIndexerAccess:
                     {
                         var implicitIndexerAccess = (BoundImplicitIndexerAccess)expr;
-                        propertySymbol = GetPropertySymbol(implicitIndexerAccess.IndexerOrSliceAccess, out receiver, out propertySyntax);
+
+                        switch (implicitIndexerAccess.IndexerOrSliceAccess)
+                        {
+                            case BoundIndexerAccess indexerAccess:
+                                propertySymbol = indexerAccess.Indexer;
+                                receiver = implicitIndexerAccess.Receiver;
+                                break;
+
+                            case BoundCall or BoundArrayAccess:
+                                receiver = null;
+                                propertySyntax = null;
+                                return null;
+
+                            default:
+                                throw ExceptionUtilities.UnexpectedValue(implicitIndexerAccess.IndexerOrSliceAccess.Kind);
+                        }
                     }
                     break;
                 default:

src/Compilers/CSharp/Portable/BoundTree/BoundDagEvaluation.cs

@@ -52,9 +52,11 @@ private Symbol? Symbol
                 {
                     switch (indexerAccess)
                     {
+                        // array[Range]
                         case BoundArrayAccess arrayAccess:
                             return arrayAccess.Expression.Type;
 
+                        // array[Index]
                         case BoundImplicitIndexerAccess { IndexerOrSliceAccess: BoundArrayAccess arrayAccess }:
                             return arrayAccess.Expression.Type;
 

src/Compilers/CSharp/Portable/BoundTree/BoundExpression.cs

@@ -123,6 +123,78 @@ public virtual bool SuppressVirtualCalls
 
         public CodeAnalysis.ITypeSymbol? GetPublicTypeSymbol()
             => Type?.GetITypeSymbol(TopLevelNullability.FlowState.ToAnnotation());
+
+        public virtual bool IsEquivalentToThisReference => false;
+    }
+
+    internal partial class BoundValuePlaceholderBase
+    {
+        public abstract override bool IsEquivalentToThisReference { get; }
+    }
+
+    internal partial class BoundValuePlaceholder
+    {
+        public sealed override bool IsEquivalentToThisReference => throw ExceptionUtilities.Unreachable;
+    }
+
+    internal partial class BoundInterpolatedStringHandlerPlaceholder
+    {
+        public sealed override bool IsEquivalentToThisReference => false;
+    }
+
+    internal partial class BoundDeconstructValuePlaceholder
+    {
+        public sealed override bool IsEquivalentToThisReference => false; // Preserving old behavior
+    }
+
+    internal partial class BoundTupleOperandPlaceholder
+    {
+        public sealed override bool IsEquivalentToThisReference => throw ExceptionUtilities.Unreachable;
+    }
+
+    internal partial class BoundAwaitableValuePlaceholder
+    {
+        public sealed override bool IsEquivalentToThisReference => false; // Preserving old behavior
+    }
+
+    internal partial class BoundDisposableValuePlaceholder
+    {
+        public sealed override bool IsEquivalentToThisReference => false;
+    }
+
+    internal partial class BoundObjectOrCollectionValuePlaceholder
+    {
+        public sealed override bool IsEquivalentToThisReference => false;
+    }
+
+    internal partial class BoundImplicitIndexerValuePlaceholder
+    {
+        public sealed override bool IsEquivalentToThisReference => throw ExceptionUtilities.Unreachable;
+    }
+
+    internal partial class BoundListPatternReceiverPlaceholder
+    {
+        public sealed override bool IsEquivalentToThisReference => false;
+    }
+
+    internal partial class BoundListPatternIndexPlaceholder
+    {
+        public sealed override bool IsEquivalentToThisReference => throw ExceptionUtilities.Unreachable;
+    }
+
+    internal partial class BoundSlicePatternReceiverPlaceholder
+    {
+        public sealed override bool IsEquivalentToThisReference => false;
+    }
+
+    internal partial class BoundSlicePatternRangePlaceholder
+    {
+        public sealed override bool IsEquivalentToThisReference => throw ExceptionUtilities.Unreachable;
+    }
+
+    internal partial class BoundThisReference
+    {
+        public sealed override bool IsEquivalentToThisReference => true;
     }
 
     internal partial class BoundPassByCopy

src/Compilers/CSharp/Portable/BoundTree/BoundImplicitIndexerAccess.cs

@@ -11,26 +11,10 @@ internal partial class BoundImplicitIndexerAccess
     {
         internal BoundImplicitIndexerAccess WithLengthOrCountAccess(BoundExpression lengthOrCountAccess)
         {
-            return this.Update(this.Argument, lengthOrCountAccess, this.ReceiverPlaceholder,
+            return this.Update(this.Receiver, this.Argument, lengthOrCountAccess, this.ReceiverPlaceholder,
                 this.IndexerOrSliceAccess, this.ArgumentPlaceholders, this.Type);
         }
 
-        // The receiver expression is the receiver of IndexerAccess.
-        // The LengthOrCountAccess uses a placeholder as receiver.
-        internal BoundExpression GetReceiver()
-        {
-            var receiver = this.IndexerOrSliceAccess switch
-            {
-                BoundIndexerAccess { ReceiverOpt: var r } => r,
-                BoundCall { ReceiverOpt: var r } => r,
-                BoundArrayAccess { Expression: var r } => r,
-                _ => throw ExceptionUtilities.UnexpectedValue(this.IndexerOrSliceAccess.Kind)
-            };
-
-            Debug.Assert(receiver is not null);
-            return receiver;
-        }
-
         private partial void Validate()
         {
             Debug.Assert(LengthOrCountAccess is BoundPropertyAccess or BoundArrayLength or BoundLocal or BoundBadExpression);

src/Compilers/CSharp/Portable/BoundTree/BoundInterpolatedStringArgumentPlaceholder.cs

@@ -9,5 +9,7 @@ internal partial class BoundInterpolatedStringArgumentPlaceholder
         public const int InstanceParameter = -1;
         public const int TrailingConstructorValidityParameter = -2;
         public const int UnspecifiedParameter = -3;
+
+        public sealed override bool IsEquivalentToThisReference => throw Roslyn.Utilities.ExceptionUtilities.Unreachable;
     }
 }

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

@@ -147,6 +147,7 @@
   <Node Name="BoundImplicitIndexerReceiverPlaceholder" Base="BoundValuePlaceholderBase">
     <Field Name="Type" Type="TypeSymbol" Override="true" Null="disallow"/>
     <Field Name="ValEscape" Type="uint" Null="NotApplicable"/>
+    <Field Name="IsEquivalentToThisReference" Type="bool" PropertyOverrides="true"/>
   </Node>
 
   <!-- This node represents the receiver for a list pattern. It does not survive lowering -->
@@ -2069,21 +2070,21 @@
   <Node Name="BoundImplicitIndexerAccess" Base="BoundExpression" SkipInNullabilityRewriter="true" HasValidate="true">
     <Field Name="Type" Type="TypeSymbol" Override="true" Null="disallow" />
 
+    <!-- The target of the index operation -->
+    <Field Name="Receiver" Type="BoundExpression" Null="disallow" />
+
     <!-- An expression with type Index or Range -->
     <Field Name="Argument" Type="BoundExpression" Null="disallow" />
 
     <Field Name="LengthOrCountAccess" Type="BoundExpression" SkipInVisitor="true" />
 
-    <!-- The receiver placeholder for length access -->
+    <!-- The receiver placeholder for length access and IndexerOrSliceAccess-->
     <Field Name="ReceiverPlaceholder" Type="BoundImplicitIndexerReceiverPlaceholder" SkipInVisitor="true" />
 
     <!--
-    May be BoundIndexerAccess or BoundCall or BoundArrayAccess,
-    which are built using the real receiver (not a placeholder) and placeholder(s) for argument(s).
-    We don't mark this property as SkipInVisitor="true" because it contains real receiver, but we
-    make sure SemanticModel doesn't dig too deep into the node.
+    May be BoundIndexerAccess or BoundCall or BoundArrayAccess
     -->
-    <Field Name="IndexerOrSliceAccess" Type="BoundExpression" Null="disallow" />
+    <Field Name="IndexerOrSliceAccess" Type="BoundExpression" Null="disallow" SkipInVisitor="true" />
 
     <!-- The receiver placeholder(s) of type Int32 for indexer access (one if implicit Index indexer, two if implicit Range indexer) -->
     <Field Name="ArgumentPlaceholders" Type="ImmutableArray&lt;BoundImplicitIndexerValuePlaceholder&gt;" Null="disallow" SkipInVisitor="true" />

src/Compilers/CSharp/Portable/BoundTree/Expression.cs

@@ -177,7 +177,7 @@ internal partial class BoundPassByCopy
 
     internal partial class BoundImplicitIndexerAccess
     {
-        protected override ImmutableArray<BoundNode?> Children => ImmutableArray.Create<BoundNode?>(this.GetReceiver(), Argument);
+        protected override ImmutableArray<BoundNode?> Children => ImmutableArray.Create<BoundNode?>(this.Receiver, Argument);
     }
 
     internal partial class BoundFunctionPointerInvocation : IBoundInvalidNode

src/Compilers/CSharp/Portable/BoundTree/NullabilityRewriter.cs

@@ -164,19 +164,20 @@ Symbol remapLocal(SourceLocalSymbol local)
 
         public override BoundNode? VisitImplicitIndexerAccess(BoundImplicitIndexerAccess node)
         {
+            BoundExpression receiver = (BoundExpression)this.Visit(node.Receiver);
             BoundExpression argument = (BoundExpression)this.Visit(node.Argument);
             BoundExpression lengthOrCountAccess = node.LengthOrCountAccess;
             BoundExpression indexerAccess = (BoundExpression)this.Visit(node.IndexerOrSliceAccess);
             BoundImplicitIndexerAccess updatedNode;
 
             if (_updatedNullabilities.TryGetValue(node, out (NullabilityInfo Info, TypeSymbol? Type) infoAndType))
             {
-                updatedNode = node.Update(argument, lengthOrCountAccess, node.ReceiverPlaceholder, indexerAccess, node.ArgumentPlaceholders, infoAndType.Type!);
+                updatedNode = node.Update(receiver, argument, lengthOrCountAccess, node.ReceiverPlaceholder, indexerAccess, node.ArgumentPlaceholders, infoAndType.Type!);
                 updatedNode.TopLevelNullability = infoAndType.Info;
             }
             else
             {
-                updatedNode = node.Update(argument, lengthOrCountAccess, node.ReceiverPlaceholder, indexerAccess, node.ArgumentPlaceholders, node.Type);
+                updatedNode = node.Update(receiver, argument, lengthOrCountAccess, node.ReceiverPlaceholder, indexerAccess, node.ArgumentPlaceholders, node.Type);
             }
             return updatedNode;
         }

src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.NodeMapBuilder.cs

@@ -258,14 +258,6 @@ public override BoundNode VisitQueryClause(BoundQueryClause node)
                 return null;
             }
 
-            public override BoundNode VisitImplicitIndexerAccess(BoundImplicitIndexerAccess node)
-            {
-                // We only need to visit receiver and the argument. SemanticModel isn't interested in anything else. 
-                Visit(node.GetReceiver());
-                this.Visit(node.Argument);
-                return null;
-            }
-
             public override BoundNode VisitRangeVariable(BoundRangeVariable node)
             {
                 return null;