Skip to content

Commit ac2327f

Browse files
authored
Avoid premature full binding of a receiver as a value in the process of binding possible type-or-value receiver (so-called “Color Color” scenario) (#80978)
Fixes #45739.
1 parent fdcc565 commit ac2327f

21 files changed

+789
-366
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -871,14 +871,15 @@ private BoundExpression CheckValue(BoundExpression expr, BindValueKind valueKind
871871
// Since we have a concrete member in hand, we can resolve the receiver.
872872
var typeOrValue = (BoundTypeOrValueExpression)receiver;
873873
receiver = otherSymbol.RequiresInstanceReceiver()
874-
? typeOrValue.Data.ValueExpression
874+
? ReplaceTypeOrValueReceiver(typeOrValue, useType: false, BindingDiagnosticBag.Discarded)
875875
: null; // no receiver required
876876
}
877+
877878
return new BoundBadExpression(
878879
expr.Syntax,
879880
methodGroup.ResultKind,
880881
(object)otherSymbol == null ? ImmutableArray<Symbol>.Empty : ImmutableArray.Create(otherSymbol),
881-
receiver == null ? ImmutableArray<BoundExpression>.Empty : ImmutableArray.Create(receiver),
882+
receiver == null ? ImmutableArray<BoundExpression>.Empty : ImmutableArray.Create(AdjustBadExpressionChild(receiver)),
882883
GetNonMethodMemberType(otherSymbol));
883884
}
884885
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ private BoundExpression SelectField(SimpleNameSyntax node, BoundExpression recei
8686
node,
8787
LookupResultKind.Empty,
8888
ImmutableArray.Create<Symbol>(receiver.ExpressionSymbol),
89-
ImmutableArray.Create(BindToTypeForErrorRecovery(receiver)),
89+
ImmutableArray.Create(AdjustBadExpressionChild(BindToTypeForErrorRecovery(receiver))),
9090
new ExtendedErrorTypeSymbol(this.Compilation, "", 0, info));
9191
}
9292

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,12 @@ source.Type is { } sourceType &&
285285
}
286286
}
287287

288+
if (source is BoundMethodGroup methodGroup)
289+
{
290+
source = FixMethodGroupWithTypeOrValue(methodGroup, conversion, diagnostics);
291+
Debug.Assert(source == (object)methodGroup || !conversion.IsValid);
292+
}
293+
288294
return new BoundConversion(
289295
syntax,
290296
BindToNaturalType(source, diagnostics),
@@ -2529,7 +2535,7 @@ private BoundExpression CreateTupleLiteralConversion(SyntaxNode syntax, BoundTup
25292535
return result;
25302536
}
25312537

2532-
private static bool IsMethodGroupWithTypeOrValueReceiver(BoundNode node)
2538+
internal static bool IsMethodGroupWithTypeOrValueReceiver(BoundNode node)
25332539
{
25342540
if (node.Kind != BoundKind.MethodGroup)
25352541
{

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

Lines changed: 296 additions & 222 deletions
Large diffs are not rendered by default.

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

Lines changed: 38 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ private BoundExpression BindDynamicInvocation(
453453
// Ideally the runtime binder would choose between type and value based on the result of the overload resolution.
454454
// We need to pick one or the other here. Dev11 compiler passes the type only if the value can't be accessed.
455455
bool inStaticContext;
456-
bool useType = IsInstance(typeOrValue.Data.ValueSymbol) && !HasThis(isExplicit: false, inStaticContext: out inStaticContext);
456+
bool useType = IsInstance(typeOrValue.ValueSymbol) && !HasThis(isExplicit: false, inStaticContext: out inStaticContext);
457457

458458
BoundExpression finalReceiver = ReplaceTypeOrValueReceiver(typeOrValue, useType, diagnostics);
459459

@@ -772,7 +772,7 @@ private BoundExpression BindMethodGroupInvocation(
772772
methodGroup.Syntax,
773773
methodGroup.ResultKind,
774774
[methodGroup.LookupSymbolOpt],
775-
receiverOpt == null ? [] : [receiverOpt],
775+
receiverOpt == null ? [] : [AdjustBadExpressionChild(receiverOpt)],
776776
GetNonMethodMemberType(methodGroup.LookupSymbolOpt));
777777
}
778778
}
@@ -1239,7 +1239,14 @@ private BoundCall BindInvocationExpressionContinued(
12391239
// instance methods. Therefore we must detect this scenario here, rather than in
12401240
// overload resolution.
12411241

1242-
var receiver = ReplaceTypeOrValueReceiver(methodGroup.Receiver, !method.RequiresInstanceReceiver && !invokedAsExtensionMethod, diagnostics);
1242+
var receiver = ReplaceTypeOrValueReceiver(methodGroup.Receiver, useType: !method.RequiresInstanceReceiver && !invokedAsExtensionMethod, diagnostics);
1243+
1244+
if (invokedAsExtensionMethod && (object)receiver != methodGroup.Receiver)
1245+
{
1246+
// we will have a different receiver if ReplaceTypeOrValueReceiver has unwrapped TypeOrValue
1247+
Debug.Assert(analyzedArguments.Arguments[0] == (object)methodGroup.Receiver);
1248+
analyzedArguments.Arguments[0] = receiver;
1249+
}
12431250

12441251
ImmutableArray<int> argsToParams;
12451252
this.CheckAndCoerceArguments(node, methodResult, analyzedArguments, diagnostics, receiver, invokedAsExtensionMethod: invokedAsExtensionMethod, out argsToParams);
@@ -1261,15 +1268,6 @@ private BoundCall BindInvocationExpressionContinued(
12611268
BoundExpression receiverArgument = analyzedArguments.Argument(0);
12621269
ParameterSymbol receiverParameter = method.Parameters.First();
12631270

1264-
// we will have a different receiver if ReplaceTypeOrValueReceiver has unwrapped TypeOrValue
1265-
if ((object)receiver != methodGroup.Receiver)
1266-
{
1267-
// Because the receiver didn't pass through CoerceArguments, we need to apply an appropriate conversion here.
1268-
Debug.Assert(argsToParams.IsDefault || argsToParams[0] == 0);
1269-
receiverArgument = CreateConversion(receiver, methodResult.Result.ConversionForArg(0),
1270-
receiverParameter.Type, diagnostics);
1271-
}
1272-
12731271
if (receiverParameter.RefKind == RefKind.Ref)
12741272
{
12751273
// If this was a ref extension method, receiverArgument must be checked for L-value constraints.
@@ -1955,28 +1953,28 @@ private BoundExpression ReplaceTypeOrValueReceiver(BoundExpression receiver, boo
19551953
{
19561954
case BoundKind.TypeOrValueExpression:
19571955
var typeOrValue = (BoundTypeOrValueExpression)receiver;
1956+
var identifier = (IdentifierNameSyntax)typeOrValue.Syntax;
1957+
Debug.Assert(typeOrValue.Binder == (object)this);
1958+
19581959
if (useType)
19591960
{
1960-
diagnostics.AddRange(typeOrValue.Data.TypeDiagnostics);
1961-
1962-
foreach (Diagnostic d in typeOrValue.Data.ValueDiagnostics.Diagnostics)
1961+
if (typeOrValue.Binder.GetShadowedPrimaryConstructorParameter(identifier, typeOrValue.ValueSymbol, invoked: false, membersOpt: null) is { } shadowedParameter &&
1962+
!shadowedParameter.Type.Equals(typeOrValue.Type, TypeCompareKind.AllIgnoreOptions)) // If the type and the name match, we would resolve to the same type rather than a value at the end.
19631963
{
1964-
// Avoid forcing resolution of lazy diagnostics to avoid cycles.
1965-
var code = d is DiagnosticWithInfo { HasLazyInfo: true, LazyInfo.Code: var lazyCode } ? lazyCode : d.Code;
1966-
if (code == (int)ErrorCode.WRN_PrimaryConstructorParameterIsShadowedAndNotPassedToBase &&
1967-
!(d.Arguments is [ParameterSymbol shadowedParameter] && shadowedParameter.Type.Equals(typeOrValue.Data.ValueExpression.Type, TypeCompareKind.AllIgnoreOptions))) // If the type and the name match, we would resolve to the same type rather than a value at the end.
1968-
{
1969-
Debug.Assert(d is not DiagnosticWithInfo { HasLazyInfo: true }, "Adjust the Arguments access to handle lazy diagnostics to avoid cycles.");
1970-
diagnostics.Add(d);
1971-
}
1964+
diagnostics.Add(ErrorCode.WRN_PrimaryConstructorParameterIsShadowedAndNotPassedToBase, identifier.Location, shadowedParameter);
19721965
}
19731966

1974-
return typeOrValue.Data.TypeExpression;
1967+
return typeOrValue.Binder.BindNamespaceOrType(identifier, diagnostics);
19751968
}
19761969
else
19771970
{
1978-
diagnostics.AddRange(typeOrValue.Data.ValueDiagnostics);
1979-
return CheckValue(typeOrValue.Data.ValueExpression, BindValueKind.RValue, diagnostics);
1971+
var boundValue = typeOrValue.Binder.BindIdentifier(identifier, invoked: false, indexed: false, diagnostics: diagnostics);
1972+
1973+
Debug.Assert(typeOrValue.Type.Equals(boundValue.Type, TypeCompareKind.ConsiderEverything));
1974+
Debug.Assert(typeOrValue.ValueSymbol == (boundValue.ExpressionSymbol ?? ((BoundConversion)boundValue).Operand.ExpressionSymbol));
1975+
1976+
boundValue = BindToNaturalType(boundValue, diagnostics);
1977+
return CheckValue(boundValue, BindValueKind.RValue, diagnostics);
19801978
}
19811979

19821980
case BoundKind.QueryClause:
@@ -1991,7 +1989,7 @@ private BoundExpression ReplaceTypeOrValueReceiver(BoundExpression receiver, boo
19911989
}
19921990
}
19931991

1994-
private static BoundExpression GetValueExpressionIfTypeOrValueReceiver(BoundExpression receiver)
1992+
private static Symbol GetValueSymbolIfTypeOrValueReceiver(BoundExpression receiver)
19951993
{
19961994
if ((object)receiver == null)
19971995
{
@@ -2001,11 +1999,11 @@ private static BoundExpression GetValueExpressionIfTypeOrValueReceiver(BoundExpr
20011999
switch (receiver)
20022000
{
20032001
case BoundTypeOrValueExpression typeOrValueExpression:
2004-
return typeOrValueExpression.Data.ValueExpression;
2002+
return typeOrValueExpression.ValueSymbol;
20052003

20062004
case BoundQueryClause queryClause:
20072005
// a query clause may wrap a TypeOrValueExpression.
2008-
return GetValueExpressionIfTypeOrValueReceiver(queryClause.Value);
2006+
return GetValueSymbolIfTypeOrValueReceiver(queryClause.Value);
20092007

20102008
default:
20112009
return null;
@@ -2385,6 +2383,17 @@ private BoundExpression BindNameofOperatorInternal(InvocationExpressionSyntax no
23852383
EnsureNameofExpressionSymbols(methodGroup, diagnostics);
23862384
}
23872385
}
2386+
2387+
boundArgument = methodGroup.Update(
2388+
methodGroup.TypeArgumentsOpt,
2389+
methodGroup.Name,
2390+
methodGroup.Methods,
2391+
methodGroup.LookupSymbolOpt,
2392+
methodGroup.LookupError,
2393+
methodGroup.Flags,
2394+
methodGroup.FunctionType,
2395+
receiverOpt: ReplaceTypeOrValueReceiver(methodGroup.ReceiverOpt, useType: false, BindingDiagnosticBag.Discarded), //only change
2396+
methodGroup.ResultKind);
23882397
}
23892398
else if (boundArgument is BoundPropertyAccess propertyAccess)
23902399
{

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1515,16 +1515,16 @@ private BoundExpression CheckAmbiguousPrimaryConstructorParameterAsColorColorRec
15151515
else
15161516
{
15171517
Error(diagnostics, ErrorCode.ERR_NoSuchMemberOrExtension, right, receiver.Type, plainName);
1518-
receiver = new BoundBadExpression(receiver.Syntax, LookupResultKind.Empty, ImmutableArray<Symbol>.Empty, childBoundNodes: [receiver], receiver.Type, hasErrors: true).MakeCompilerGenerated();
1518+
receiver = new BoundBadExpression(receiver.Syntax, LookupResultKind.Empty, ImmutableArray<Symbol>.Empty, childBoundNodes: [AdjustBadExpressionChild(receiver)], receiver.Type, hasErrors: true).MakeCompilerGenerated();
15191519
}
15201520

15211521
return receiver;
15221522

15231523
bool isPossiblyCapturingPrimaryConstructorParameterReference(BoundExpression receiver, out ParameterSymbol parameterSymbol)
15241524
{
1525-
BoundExpression colorColorValueReceiver = GetValueExpressionIfTypeOrValueReceiver(receiver);
1525+
Symbol colorColorValueSymbol = GetValueSymbolIfTypeOrValueReceiver(receiver);
15261526

1527-
if (colorColorValueReceiver is BoundParameter { ParameterSymbol: { ContainingSymbol: SynthesizedPrimaryConstructor primaryConstructor } parameter } &&
1527+
if (colorColorValueSymbol is ParameterSymbol { ContainingSymbol: SynthesizedPrimaryConstructor primaryConstructor } parameter &&
15281528
IsInDeclaringTypeInstanceMember(primaryConstructor) &&
15291529
!InFieldInitializer &&
15301530
this.ContainingMember() != (object)primaryConstructor &&
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System.Diagnostics;
6+
using System.Linq;
7+
8+
namespace Microsoft.CodeAnalysis.CSharp
9+
{
10+
internal partial class BoundBadExpression
11+
{
12+
private partial void Validate()
13+
{
14+
Debug.Assert(!this.ChildBoundNodes.Any(c => Binder.IsTypeOrValueExpression(c) || Binder.IsMethodGroupWithTypeOrValueReceiver(c)));
15+
}
16+
}
17+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System.Diagnostics;
6+
7+
namespace Microsoft.CodeAnalysis.CSharp
8+
{
9+
internal partial class BoundConversion
10+
{
11+
private partial void Validate()
12+
{
13+
Debug.Assert(!Binder.IsTypeOrValueExpression(Operand));
14+
Debug.Assert(!Binder.IsMethodGroupWithTypeOrValueReceiver(Operand));
15+
}
16+
}
17+
}

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

Lines changed: 0 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -667,66 +667,6 @@ public override bool SuppressVirtualCalls
667667
}
668668
}
669669

670-
// NOTE: this type exists in order to hide the presence of {Value,Type}Expression inside of a
671-
// BoundTypeOrValueExpression from the bound tree generator, which would otherwise generate
672-
// a constructor that may spuriously set hasErrors to true if either field had errors.
673-
// A BoundTypeOrValueExpression should never have errors if it is present in the tree.
674-
internal readonly struct BoundTypeOrValueData : System.IEquatable<BoundTypeOrValueData>
675-
{
676-
public Symbol ValueSymbol { get; }
677-
public BoundExpression ValueExpression { get; }
678-
public ReadOnlyBindingDiagnostic<AssemblySymbol> ValueDiagnostics { get; }
679-
public BoundExpression TypeExpression { get; }
680-
public ReadOnlyBindingDiagnostic<AssemblySymbol> TypeDiagnostics { get; }
681-
682-
public BoundTypeOrValueData(Symbol valueSymbol, BoundExpression valueExpression, ReadOnlyBindingDiagnostic<AssemblySymbol> valueDiagnostics, BoundExpression typeExpression, ReadOnlyBindingDiagnostic<AssemblySymbol> typeDiagnostics)
683-
{
684-
Debug.Assert(valueSymbol != null, "Field 'valueSymbol' cannot be null (use Null=\"allow\" in BoundNodes.xml to remove this check)");
685-
Debug.Assert(valueExpression != null, "Field 'valueExpression' cannot be null (use Null=\"allow\" in BoundNodes.xml to remove this check)");
686-
Debug.Assert(typeExpression != null, "Field 'typeExpression' cannot be null (use Null=\"allow\" in BoundNodes.xml to remove this check)");
687-
688-
this.ValueSymbol = valueSymbol;
689-
this.ValueExpression = valueExpression;
690-
this.ValueDiagnostics = valueDiagnostics;
691-
this.TypeExpression = typeExpression;
692-
this.TypeDiagnostics = typeDiagnostics;
693-
}
694-
695-
// operator==, operator!=, GetHashCode, and Equals are needed by the generated bound tree.
696-
697-
public static bool operator ==(BoundTypeOrValueData a, BoundTypeOrValueData b)
698-
{
699-
return (object)a.ValueSymbol == (object)b.ValueSymbol &&
700-
(object)a.ValueExpression == (object)b.ValueExpression &&
701-
a.ValueDiagnostics == b.ValueDiagnostics &&
702-
(object)a.TypeExpression == (object)b.TypeExpression &&
703-
a.TypeDiagnostics == b.TypeDiagnostics;
704-
}
705-
706-
public static bool operator !=(BoundTypeOrValueData a, BoundTypeOrValueData b)
707-
{
708-
return !(a == b);
709-
}
710-
711-
public override bool Equals(object? obj)
712-
{
713-
return obj is BoundTypeOrValueData && (BoundTypeOrValueData)obj == this;
714-
}
715-
716-
public override int GetHashCode()
717-
{
718-
return Hash.Combine(ValueSymbol.GetHashCode(),
719-
Hash.Combine(ValueExpression.GetHashCode(),
720-
Hash.Combine(ValueDiagnostics.GetHashCode(),
721-
Hash.Combine(TypeExpression.GetHashCode(), TypeDiagnostics.GetHashCode()))));
722-
}
723-
724-
bool System.IEquatable<BoundTypeOrValueData>.Equals(BoundTypeOrValueData b)
725-
{
726-
return b == this;
727-
}
728-
}
729-
730670
internal partial class BoundTupleExpression
731671
{
732672
/// <summary>
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System.Diagnostics;
6+
7+
namespace Microsoft.CodeAnalysis.CSharp
8+
{
9+
internal partial class BoundNameOfOperator
10+
{
11+
private partial void Validate()
12+
{
13+
Debug.Assert(!Binder.IsTypeOrValueExpression(Argument));
14+
Debug.Assert(!Binder.IsMethodGroupWithTypeOrValueReceiver(Argument));
15+
}
16+
}
17+
}

0 commit comments

Comments
 (0)