Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -871,14 +871,15 @@ private BoundExpression CheckValue(BoundExpression expr, BindValueKind valueKind
// Since we have a concrete member in hand, we can resolve the receiver.
var typeOrValue = (BoundTypeOrValueExpression)receiver;
receiver = otherSymbol.RequiresInstanceReceiver()
? typeOrValue.Data.ValueExpression
? ReplaceTypeOrValueReceiver(typeOrValue, useType: false, BindingDiagnosticBag.Discarded)
: null; // no receiver required
}

return new BoundBadExpression(
expr.Syntax,
methodGroup.ResultKind,
(object)otherSymbol == null ? ImmutableArray<Symbol>.Empty : ImmutableArray.Create(otherSymbol),
receiver == null ? ImmutableArray<BoundExpression>.Empty : ImmutableArray.Create(receiver),
receiver == null ? ImmutableArray<BoundExpression>.Empty : ImmutableArray.Create(AdjustBadExpressionChild(receiver)),
GetNonMethodMemberType(otherSymbol));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ private BoundExpression SelectField(SimpleNameSyntax node, BoundExpression recei
node,
LookupResultKind.Empty,
ImmutableArray.Create<Symbol>(receiver.ExpressionSymbol),
ImmutableArray.Create(BindToTypeForErrorRecovery(receiver)),
ImmutableArray.Create(AdjustBadExpressionChild(BindToTypeForErrorRecovery(receiver))),
new ExtendedErrorTypeSymbol(this.Compilation, "", 0, info));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,12 @@ source.Type is { } sourceType &&
}
}

if (source is BoundMethodGroup methodGroup)
{
source = FixMethodGroupWithTypeOrValue(methodGroup, conversion, diagnostics);
Debug.Assert(source == (object)methodGroup || !conversion.IsValid);
}

return new BoundConversion(
syntax,
BindToNaturalType(source, diagnostics),
Expand Down Expand Up @@ -2529,7 +2535,7 @@ private BoundExpression CreateTupleLiteralConversion(SyntaxNode syntax, BoundTup
return result;
}

private static bool IsMethodGroupWithTypeOrValueReceiver(BoundNode node)
internal static bool IsMethodGroupWithTypeOrValueReceiver(BoundNode node)
{
if (node.Kind != BoundKind.MethodGroup)
{
Expand Down
518 changes: 296 additions & 222 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs

Large diffs are not rendered by default.

67 changes: 38 additions & 29 deletions src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ private BoundExpression BindDynamicInvocation(
// Ideally the runtime binder would choose between type and value based on the result of the overload resolution.
// We need to pick one or the other here. Dev11 compiler passes the type only if the value can't be accessed.
bool inStaticContext;
bool useType = IsInstance(typeOrValue.Data.ValueSymbol) && !HasThis(isExplicit: false, inStaticContext: out inStaticContext);
bool useType = IsInstance(typeOrValue.ValueSymbol) && !HasThis(isExplicit: false, inStaticContext: out inStaticContext);

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

Expand Down Expand Up @@ -772,7 +772,7 @@ private BoundExpression BindMethodGroupInvocation(
methodGroup.Syntax,
methodGroup.ResultKind,
[methodGroup.LookupSymbolOpt],
receiverOpt == null ? [] : [receiverOpt],
receiverOpt == null ? [] : [AdjustBadExpressionChild(receiverOpt)],
GetNonMethodMemberType(methodGroup.LookupSymbolOpt));
}
}
Expand Down Expand Up @@ -1239,7 +1239,14 @@ private BoundCall BindInvocationExpressionContinued(
// instance methods. Therefore we must detect this scenario here, rather than in
// overload resolution.

var receiver = ReplaceTypeOrValueReceiver(methodGroup.Receiver, !method.RequiresInstanceReceiver && !invokedAsExtensionMethod, diagnostics);
var receiver = ReplaceTypeOrValueReceiver(methodGroup.Receiver, useType: !method.RequiresInstanceReceiver && !invokedAsExtensionMethod, diagnostics);

if (invokedAsExtensionMethod && (object)receiver != methodGroup.Receiver)
{
// we will have a different receiver if ReplaceTypeOrValueReceiver has unwrapped TypeOrValue
Debug.Assert(analyzedArguments.Arguments[0] == (object)methodGroup.Receiver);
analyzedArguments.Arguments[0] = receiver;
}

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

// we will have a different receiver if ReplaceTypeOrValueReceiver has unwrapped TypeOrValue
if ((object)receiver != methodGroup.Receiver)
{
// Because the receiver didn't pass through CoerceArguments, we need to apply an appropriate conversion here.
Debug.Assert(argsToParams.IsDefault || argsToParams[0] == 0);
receiverArgument = CreateConversion(receiver, methodResult.Result.ConversionForArg(0),
receiverParameter.Type, diagnostics);
}

if (receiverParameter.RefKind == RefKind.Ref)
{
// If this was a ref extension method, receiverArgument must be checked for L-value constraints.
Expand Down Expand Up @@ -1955,28 +1953,28 @@ private BoundExpression ReplaceTypeOrValueReceiver(BoundExpression receiver, boo
{
case BoundKind.TypeOrValueExpression:
var typeOrValue = (BoundTypeOrValueExpression)receiver;
var identifier = (IdentifierNameSyntax)typeOrValue.Syntax;
Debug.Assert(typeOrValue.Binder == (object)this);

if (useType)
{
diagnostics.AddRange(typeOrValue.Data.TypeDiagnostics);

foreach (Diagnostic d in typeOrValue.Data.ValueDiagnostics.Diagnostics)
if (typeOrValue.Binder.GetShadowedPrimaryConstructorParameter(identifier, typeOrValue.ValueSymbol, invoked: false, membersOpt: null) is { } shadowedParameter &&
!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.
{
// Avoid forcing resolution of lazy diagnostics to avoid cycles.
var code = d is DiagnosticWithInfo { HasLazyInfo: true, LazyInfo.Code: var lazyCode } ? lazyCode : d.Code;
if (code == (int)ErrorCode.WRN_PrimaryConstructorParameterIsShadowedAndNotPassedToBase &&
!(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.
{
Debug.Assert(d is not DiagnosticWithInfo { HasLazyInfo: true }, "Adjust the Arguments access to handle lazy diagnostics to avoid cycles.");
diagnostics.Add(d);
}
diagnostics.Add(ErrorCode.WRN_PrimaryConstructorParameterIsShadowedAndNotPassedToBase, identifier.Location, shadowedParameter);
}

return typeOrValue.Data.TypeExpression;
return typeOrValue.Binder.BindNamespaceOrType(identifier, diagnostics);
}
else
{
diagnostics.AddRange(typeOrValue.Data.ValueDiagnostics);
return CheckValue(typeOrValue.Data.ValueExpression, BindValueKind.RValue, diagnostics);
var boundValue = typeOrValue.Binder.BindIdentifier(identifier, invoked: false, indexed: false, diagnostics: diagnostics);

Debug.Assert(typeOrValue.Type.Equals(boundValue.Type, TypeCompareKind.ConsiderEverything));
Copy link
Member

@jcouv jcouv Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug.Assert(typeOrValue.Type.Equals(boundValue.Type, TypeCompareKind.ConsiderEverything)); (line 1973)
Could the nullability of the value differ? For example Color? Color = ...; the Color value would have type Color? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the nullability of the value differ?

Bound tree doesn't preserve top level nullability of types and we are binding simple identifiers. Perhaps nested nullability can somehow sneak in. Since this is an assert, I am not worried about this and I am comfortable going with a possibly too strong condition. We can relax it once the assert starts causing problems.

Debug.Assert(typeOrValue.ValueSymbol == (boundValue.ExpressionSymbol ?? ((BoundConversion)boundValue).Operand.ExpressionSymbol));

boundValue = BindToNaturalType(boundValue, diagnostics);
return CheckValue(boundValue, BindValueKind.RValue, diagnostics);
}

case BoundKind.QueryClause:
Expand All @@ -1991,7 +1989,7 @@ private BoundExpression ReplaceTypeOrValueReceiver(BoundExpression receiver, boo
}
}

private static BoundExpression GetValueExpressionIfTypeOrValueReceiver(BoundExpression receiver)
private static Symbol GetValueSymbolIfTypeOrValueReceiver(BoundExpression receiver)
{
if ((object)receiver == null)
{
Expand All @@ -2001,11 +1999,11 @@ private static BoundExpression GetValueExpressionIfTypeOrValueReceiver(BoundExpr
switch (receiver)
{
case BoundTypeOrValueExpression typeOrValueExpression:
return typeOrValueExpression.Data.ValueExpression;
return typeOrValueExpression.ValueSymbol;

case BoundQueryClause queryClause:
// a query clause may wrap a TypeOrValueExpression.
return GetValueExpressionIfTypeOrValueReceiver(queryClause.Value);
return GetValueSymbolIfTypeOrValueReceiver(queryClause.Value);

default:
return null;
Expand Down Expand Up @@ -2385,6 +2383,17 @@ private BoundExpression BindNameofOperatorInternal(InvocationExpressionSyntax no
EnsureNameofExpressionSymbols(methodGroup, diagnostics);
}
}

boundArgument = methodGroup.Update(
methodGroup.TypeArgumentsOpt,
methodGroup.Name,
methodGroup.Methods,
methodGroup.LookupSymbolOpt,
methodGroup.LookupError,
methodGroup.Flags,
methodGroup.FunctionType,
receiverOpt: ReplaceTypeOrValueReceiver(methodGroup.ReceiverOpt, useType: false, BindingDiagnosticBag.Discarded), //only change
methodGroup.ResultKind);
}
else if (boundArgument is BoundPropertyAccess propertyAccess)
{
Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1515,16 +1515,16 @@ private BoundExpression CheckAmbiguousPrimaryConstructorParameterAsColorColorRec
else
{
Error(diagnostics, ErrorCode.ERR_NoSuchMemberOrExtension, right, receiver.Type, plainName);
receiver = new BoundBadExpression(receiver.Syntax, LookupResultKind.Empty, ImmutableArray<Symbol>.Empty, childBoundNodes: [receiver], receiver.Type, hasErrors: true).MakeCompilerGenerated();
receiver = new BoundBadExpression(receiver.Syntax, LookupResultKind.Empty, ImmutableArray<Symbol>.Empty, childBoundNodes: [AdjustBadExpressionChild(receiver)], receiver.Type, hasErrors: true).MakeCompilerGenerated();
}

return receiver;

bool isPossiblyCapturingPrimaryConstructorParameterReference(BoundExpression receiver, out ParameterSymbol parameterSymbol)
{
BoundExpression colorColorValueReceiver = GetValueExpressionIfTypeOrValueReceiver(receiver);
Symbol colorColorValueSymbol = GetValueSymbolIfTypeOrValueReceiver(receiver);

if (colorColorValueReceiver is BoundParameter { ParameterSymbol: { ContainingSymbol: SynthesizedPrimaryConstructor primaryConstructor } parameter } &&
if (colorColorValueSymbol is ParameterSymbol { ContainingSymbol: SynthesizedPrimaryConstructor primaryConstructor } parameter &&
IsInDeclaringTypeInstanceMember(primaryConstructor) &&
!InFieldInitializer &&
this.ContainingMember() != (object)primaryConstructor &&
Expand Down
17 changes: 17 additions & 0 deletions src/Compilers/CSharp/Portable/BoundTree/BoundBadExpression.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Diagnostics;
using System.Linq;

namespace Microsoft.CodeAnalysis.CSharp
{
internal partial class BoundBadExpression
{
private partial void Validate()
{
Debug.Assert(!this.ChildBoundNodes.Any(c => Binder.IsTypeOrValueExpression(c) || Binder.IsMethodGroupWithTypeOrValueReceiver(c)));
}
}
}
17 changes: 17 additions & 0 deletions src/Compilers/CSharp/Portable/BoundTree/BoundConversion.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Diagnostics;

namespace Microsoft.CodeAnalysis.CSharp
{
internal partial class BoundConversion
{
private partial void Validate()
{
Debug.Assert(!Binder.IsTypeOrValueExpression(Operand));
Debug.Assert(!Binder.IsMethodGroupWithTypeOrValueReceiver(Operand));
}
}
}
60 changes: 0 additions & 60 deletions src/Compilers/CSharp/Portable/BoundTree/BoundExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -667,66 +667,6 @@ public override bool SuppressVirtualCalls
}
}

// NOTE: this type exists in order to hide the presence of {Value,Type}Expression inside of a
// BoundTypeOrValueExpression from the bound tree generator, which would otherwise generate
// a constructor that may spuriously set hasErrors to true if either field had errors.
// A BoundTypeOrValueExpression should never have errors if it is present in the tree.
internal readonly struct BoundTypeOrValueData : System.IEquatable<BoundTypeOrValueData>
{
public Symbol ValueSymbol { get; }
public BoundExpression ValueExpression { get; }
public ReadOnlyBindingDiagnostic<AssemblySymbol> ValueDiagnostics { get; }
public BoundExpression TypeExpression { get; }
public ReadOnlyBindingDiagnostic<AssemblySymbol> TypeDiagnostics { get; }

public BoundTypeOrValueData(Symbol valueSymbol, BoundExpression valueExpression, ReadOnlyBindingDiagnostic<AssemblySymbol> valueDiagnostics, BoundExpression typeExpression, ReadOnlyBindingDiagnostic<AssemblySymbol> typeDiagnostics)
{
Debug.Assert(valueSymbol != null, "Field 'valueSymbol' cannot be null (use Null=\"allow\" in BoundNodes.xml to remove this check)");
Debug.Assert(valueExpression != null, "Field 'valueExpression' cannot be null (use Null=\"allow\" in BoundNodes.xml to remove this check)");
Debug.Assert(typeExpression != null, "Field 'typeExpression' cannot be null (use Null=\"allow\" in BoundNodes.xml to remove this check)");

this.ValueSymbol = valueSymbol;
this.ValueExpression = valueExpression;
this.ValueDiagnostics = valueDiagnostics;
this.TypeExpression = typeExpression;
this.TypeDiagnostics = typeDiagnostics;
}

// operator==, operator!=, GetHashCode, and Equals are needed by the generated bound tree.

public static bool operator ==(BoundTypeOrValueData a, BoundTypeOrValueData b)
{
return (object)a.ValueSymbol == (object)b.ValueSymbol &&
(object)a.ValueExpression == (object)b.ValueExpression &&
a.ValueDiagnostics == b.ValueDiagnostics &&
(object)a.TypeExpression == (object)b.TypeExpression &&
a.TypeDiagnostics == b.TypeDiagnostics;
}

public static bool operator !=(BoundTypeOrValueData a, BoundTypeOrValueData b)
{
return !(a == b);
}

public override bool Equals(object? obj)
{
return obj is BoundTypeOrValueData && (BoundTypeOrValueData)obj == this;
}

public override int GetHashCode()
{
return Hash.Combine(ValueSymbol.GetHashCode(),
Hash.Combine(ValueExpression.GetHashCode(),
Hash.Combine(ValueDiagnostics.GetHashCode(),
Hash.Combine(TypeExpression.GetHashCode(), TypeDiagnostics.GetHashCode()))));
}

bool System.IEquatable<BoundTypeOrValueData>.Equals(BoundTypeOrValueData b)
{
return b == this;
}
}

internal partial class BoundTupleExpression
{
/// <summary>
Expand Down
17 changes: 17 additions & 0 deletions src/Compilers/CSharp/Portable/BoundTree/BoundNameOfOperator.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Diagnostics;

namespace Microsoft.CodeAnalysis.CSharp
{
internal partial class BoundNameOfOperator
{
private partial void Validate()
{
Debug.Assert(!Binder.IsTypeOrValueExpression(Argument));
Debug.Assert(!Binder.IsMethodGroupWithTypeOrValueReceiver(Argument));
}
}
}
Loading
Loading