Skip to content

Commit

Permalink
Restore dynamic as result type of some operations involving `dynami…
Browse files Browse the repository at this point in the history 
…c` arguments (dotnet#72964)

Fixes dotnet#72750.

For C# 12 language version: behavior of the compiler in regards to deciding between whether binding should be static or dynamic is the same as behavior of C# 12 compiler. As a result, for affected scenarios, what used to have `dynamic` type in C# 12 compiler will have `dynamic` type when C# 12 language version is targeted.

For newer language versions: invocations statically bound in presence of dynamic arguments should have dynamic result if their dynamic binding succeeded in C# 12. Corresponding spec update - dotnet/csharplang#8027 at commit 8.

Related to dotnet#33011, dotnet#72906, dotnet#72912, dotnet#72913, dotnet#72914, dotnet#72916, dotnet#72931.
  • Loading branch information
@AlekseyTs
AlekseyTs committed Apr 16, 2024
1 parent 0b1fefc commit 6291b1c
Show file tree
Hide file tree
Showing 25 changed files with 7,696 additions and 274 deletions.
30 changes: 26 additions & 4 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,8 +310,19 @@ private static bool RequiresRefOrOut(BindValueKind kind)

#nullable enable

private BoundIndexerAccess BindIndexerDefaultArgumentsAndParamsCollection(BoundIndexerAccess indexerAccess, BindValueKind valueKind, BindingDiagnosticBag diagnostics)
/// <param name="dynamificationOfAssignmentResultIsHandled">
/// When an indexer is accessed with dynamic argument is resolved statically,
/// in some scenarios its result type is set to 'dynamic' type.
/// Assignments to such indexers should be bound statically as well, reverting back
/// to the indexer's type for the target and setting result type of the assignment to 'dynamic' type.
/// This flag and the assertion below help catch any new assignment scenarios and
/// make them aware of this subtlety.
/// The flag itself doesn't affect semantic analysis beyond the assertion.
/// </param>
private BoundIndexerAccess BindIndexerDefaultArgumentsAndParamsCollection(BoundIndexerAccess indexerAccess, BindValueKind valueKind, BindingDiagnosticBag diagnostics, bool dynamificationOfAssignmentResultIsHandled = false)
{
Debug.Assert((valueKind & BindValueKind.Assignable) == 0 || (valueKind & BindValueKind.RefersToLocation) != 0 || dynamificationOfAssignmentResultIsHandled);

var useSetAccessor = valueKind == BindValueKind.Assignable && !indexerAccess.Indexer.ReturnsByRef;
var accessorForDefaultArguments = useSetAccessor
? indexerAccess.Indexer.GetOwnOrInheritedSetMethod()
Expand Down Expand Up @@ -404,15 +415,26 @@ private BoundIndexerAccess BindIndexerDefaultArgumentsAndParamsCollection(BoundI
/// method returns a BoundBadExpression node. The method returns the original
/// expression without generating any error if the expression has errors.
/// </summary>
private BoundExpression CheckValue(BoundExpression expr, BindValueKind valueKind, BindingDiagnosticBag diagnostics)
/// <param name="dynamificationOfAssignmentResultIsHandled">
/// When an indexer is accessed with dynamic argument is resolved statically,
/// in some scenarios its result type is set to 'dynamic' type.
/// Assignments to such indexers should be bound statically as well, reverting back
/// to the indexer's type for the target and setting result type of the assignment to 'dynamic' type.
/// This flag and the assertion below help catch any new assignment scenarios and
/// make them aware of this subtlety.
/// The flag itself doesn't affect semantic analysis beyond the assertion.
/// </param>
private BoundExpression CheckValue(BoundExpression expr, BindValueKind valueKind, BindingDiagnosticBag diagnostics, bool dynamificationOfAssignmentResultIsHandled = false)
{
Debug.Assert((valueKind & BindValueKind.Assignable) == 0 || (valueKind & BindValueKind.RefersToLocation) != 0 || dynamificationOfAssignmentResultIsHandled);

switch (expr.Kind)
{
case BoundKind.PropertyGroup:
expr = BindIndexedPropertyAccess((BoundPropertyGroup)expr, mustHaveAllOptionalParameters: false, diagnostics: diagnostics);
if (expr is BoundIndexerAccess indexerAccess)
{
expr = BindIndexerDefaultArgumentsAndParamsCollection(indexerAccess, valueKind, diagnostics);
expr = BindIndexerDefaultArgumentsAndParamsCollection(indexerAccess, valueKind, diagnostics, dynamificationOfAssignmentResultIsHandled: dynamificationOfAssignmentResultIsHandled);
}
break;

Expand All @@ -430,7 +452,7 @@ private BoundExpression CheckValue(BoundExpression expr, BindValueKind valueKind
return expr;

case BoundKind.IndexerAccess:
expr = BindIndexerDefaultArgumentsAndParamsCollection((BoundIndexerAccess)expr, valueKind, diagnostics);
expr = BindIndexerDefaultArgumentsAndParamsCollection((BoundIndexerAccess)expr, valueKind, diagnostics, dynamificationOfAssignmentResultIsHandled: dynamificationOfAssignmentResultIsHandled);
break;

case BoundKind.UnconvertedObjectCreationExpression:
Expand Down
63 changes: 51 additions & 12 deletions src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ private BoundDeconstructionAssignmentOperator BindDeconstructionAssignment(
var type = boundRHS.Type ?? voidType;
return new BoundDeconstructionAssignmentOperator(
node,
DeconstructionVariablesAsTuple(left, checkedVariables, diagnostics, ignoreDiagnosticsFromTuple: true),
DeconstructionVariablesAsTuple(left, checkedVariables, assignmentResultTupleType: out _, diagnostics, ignoreDiagnosticsFromTuple: true),
new BoundConversion(boundRHS.Syntax, boundRHS, Conversion.Deconstruction, @checked: false, explicitCastInCode: false, conversionGroupOpt: null,
constantValueOpt: null, type: type, hasErrors: true),
resultIsUsed,
Expand All @@ -154,9 +154,9 @@ private BoundDeconstructionAssignmentOperator BindDeconstructionAssignment(

FailRemainingInferences(checkedVariables, diagnostics);

var lhsTuple = DeconstructionVariablesAsTuple(left, checkedVariables, diagnostics, ignoreDiagnosticsFromTuple: diagnostics.HasAnyErrors() || !resultIsUsed);
var lhsTuple = DeconstructionVariablesAsTuple(left, checkedVariables, out NamedTypeSymbol returnType, diagnostics, ignoreDiagnosticsFromTuple: diagnostics.HasAnyErrors() || !resultIsUsed);
Debug.Assert(hasErrors || lhsTuple.Type is object);
TypeSymbol returnType = hasErrors ? CreateErrorType() : lhsTuple.Type!;
returnType = hasErrors ? CreateErrorType() : returnType;

var boundConversion = new BoundConversion(
boundRHS.Syntax,
Expand Down Expand Up @@ -316,8 +316,8 @@ private bool MakeDeconstructionConversion(
}
else
{
var single = variable.Single;
Debug.Assert(single is object);
Debug.Assert(variable.Single is object);
var single = AdjustAssignmentTargetForDynamic(variable.Single, forceDynamicResult: out _);
Debug.Assert(single.Type is not null);
CompoundUseSiteInfo<AssemblySymbol> useSiteInfo = GetNewCompoundUseSiteInfo(diagnostics);
nestedConversion = this.Conversions.ClassifyConversionFromType(tupleOrDeconstructedTypes[i], single.Type, isChecked: CheckOverflowAtRuntime, ref useSiteInfo);
Expand Down Expand Up @@ -502,7 +502,7 @@ private string GetDebuggerDisplay()
if ((object?)variable.Single.Type != null)
{
// typed-variable on the left
mergedType = variable.Single.Type;
mergedType = AdjustAssignmentTargetForDynamic(variable.Single, forceDynamicResult: out _).Type;
}
}
}
Expand Down Expand Up @@ -542,30 +542,39 @@ private string GetDebuggerDisplay()
syntax: syntax);
}

private BoundTupleExpression DeconstructionVariablesAsTuple(CSharpSyntaxNode syntax, ArrayBuilder<DeconstructionVariable> variables,
private BoundTupleExpression DeconstructionVariablesAsTuple(
CSharpSyntaxNode syntax, ArrayBuilder<DeconstructionVariable> variables,
out NamedTypeSymbol assignmentResultTupleType,
BindingDiagnosticBag diagnostics, bool ignoreDiagnosticsFromTuple)
{
int count = variables.Count;
var valuesBuilder = ArrayBuilder<BoundExpression>.GetInstance(count);
var resultTypesWithAnnotationsBuilder = ArrayBuilder<TypeWithAnnotations>.GetInstance(count);
var typesWithAnnotationsBuilder = ArrayBuilder<TypeWithAnnotations>.GetInstance(count);
var locationsBuilder = ArrayBuilder<Location?>.GetInstance(count);
var namesBuilder = ArrayBuilder<string?>.GetInstance(count);

foreach (var variable in variables)
{
BoundExpression value;
TypeSymbol resultType;
if (variable.NestedVariables is object)
{
value = DeconstructionVariablesAsTuple(variable.Syntax, variable.NestedVariables, diagnostics, ignoreDiagnosticsFromTuple);
value = DeconstructionVariablesAsTuple(variable.Syntax, variable.NestedVariables, out NamedTypeSymbol nestedResultType, diagnostics, ignoreDiagnosticsFromTuple);
resultType = nestedResultType;
namesBuilder.Add(null);
}
else
{
Debug.Assert(variable.Single is object);
value = variable.Single;
Debug.Assert(value.Type is not null);
resultType = value.Type;
value = AdjustAssignmentTargetForDynamic(value, forceDynamicResult: out _);
namesBuilder.Add(ExtractDeconstructResultElementName(value));
}
valuesBuilder.Add(value);
resultTypesWithAnnotationsBuilder.Add(TypeWithAnnotations.Create(resultType));
typesWithAnnotationsBuilder.Add(TypeWithAnnotations.Create(value.Type));
locationsBuilder.Add(variable.Syntax.Location);
}
Expand All @@ -579,14 +588,39 @@ private BoundTupleExpression DeconstructionVariablesAsTuple(CSharpSyntaxNode syn
ImmutableArray<bool> inferredPositions = tupleNames.IsDefault ? default : tupleNames.SelectAsArray(n => n != null);
bool disallowInferredNames = this.Compilation.LanguageVersion.DisallowInferredTupleElementNames();

ImmutableArray<Location?> elementLocations = locationsBuilder.ToImmutableAndFree();
var createTupleDiagnostics = ignoreDiagnosticsFromTuple ? null : BindingDiagnosticBag.GetInstance(diagnostics);

var type = NamedTypeSymbol.CreateTuple(
syntax.Location,
typesWithAnnotationsBuilder.ToImmutableAndFree(), locationsBuilder.ToImmutableAndFree(),
typesWithAnnotationsBuilder.ToImmutableAndFree(), elementLocations,
tupleNames, this.Compilation,
shouldCheckConstraints: createTupleDiagnostics is not null,
includeNullability: false,
errorPositions: disallowInferredNames ? inferredPositions : default,
syntax: syntax, diagnostics: createTupleDiagnostics);

if (createTupleDiagnostics is { AccumulatesDiagnostics: true, DiagnosticBag: { } bag } &&
bag.HasAnyResolvedErrors())
{
diagnostics.AddRangeAndFree(createTupleDiagnostics);
createTupleDiagnostics = null; // Suppress possibly duplicate errors from CreateTuple call below.
}

// This type is the same as the 'type' above, or differs only by using 'dynamic' for some elements.
assignmentResultTupleType = NamedTypeSymbol.CreateTuple(
syntax.Location,
resultTypesWithAnnotationsBuilder.ToImmutableAndFree(), elementLocations,
tupleNames, this.Compilation,
shouldCheckConstraints: !ignoreDiagnosticsFromTuple,
shouldCheckConstraints: createTupleDiagnostics is not null,
includeNullability: false,
errorPositions: disallowInferredNames ? inferredPositions : default,
syntax: syntax, diagnostics: ignoreDiagnosticsFromTuple ? null : diagnostics);
syntax: syntax, diagnostics: createTupleDiagnostics);

if (createTupleDiagnostics is not null)
{
diagnostics.AddRangeAndFree(createTupleDiagnostics);
}

return (BoundTupleExpression)BindToNaturalType(new BoundTupleLiteral(syntax, arguments, tupleNames, inferredPositions, type), diagnostics);
}
Expand Down Expand Up @@ -788,12 +822,17 @@ private DeconstructionVariable BindDeconstructionVariables(
}
default:
var boundVariable = BindExpression(node, diagnostics, invoked: false, indexed: false);
var checkedVariable = CheckValue(boundVariable, BindValueKind.Assignable, diagnostics);
var checkedVariable = CheckValue(boundVariable, BindValueKind.Assignable, diagnostics, dynamificationOfAssignmentResultIsHandled: true);

if (expression == null && checkedVariable.Kind != BoundKind.DiscardExpression)
{
expression = node;
}

// This object doesn't escape BindDeconstruction method, we don't call AdjustAssignmentTargetForDynamic
// for checkedVariable here, instead we call it where binder accesses DeconstructionVariable.Single
// In some of the places we need to be able to detect the fact that the type used to be dynamic, and,
// if we erase the fact here, there will be no other place for us to look at.
return new DeconstructionVariable(checkedVariable, node);
}
}
Expand Down
Loading

0 comments on commit 6291b1c

Please sign in to comment.