Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support ref field assignment in object initializers #62584

Merged
merged 9 commits into from
Jul 21, 2022
Merged

Support ref field assignment in object initializers #62584

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2c7d809
Allow ref initialization
jcouv Jul 12, 2022
6980e20
fix a test
jcouv Jul 15, 2022
64cb0af
Address feedback
jcouv Jul 19, 2022
74ad78e
Localize
jcouv Jul 19, 2022
73150a0
tweak
jcouv Jul 19, 2022
7c5136d
Address feedback
jcouv Jul 20, 2022
53fc72c
Test execution
jcouv Jul 21, 2022
984528f
Merge remote-tracking branch 'dotnet/main' into ref-init
jcouv Jul 21, 2022
bf99c5d
Merge conflict
jcouv Jul 21, 2022
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
13 changes: 11 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@ internal bool CheckValueKind(SyntaxNode node, BoundExpression expr, BindValueKin
// dynamic expressions are readwrite, and can even be passed by ref (which is implemented via a temp)
case BoundKind.DynamicMemberAccess:
case BoundKind.DynamicIndexerAccess:
case BoundKind.DynamicObjectInitializerMember:
{
if (RequiresRefAssignableVariable(valueKind))
{
Expand Down Expand Up @@ -3144,7 +3145,11 @@ private uint GetValEscapeOfObjectInitializer(BoundObjectInitializerExpression in
if (expression.Kind == BoundKind.AssignmentOperator)
{
var assignment = (BoundAssignmentOperator)expression;
result = Math.Max(result, GetValEscape(assignment.Right, scopeOfTheContainingExpression));
var rightValEscape = assignment.IsRef
? GetRefEscape(assignment.Right, scopeOfTheContainingExpression)
: GetValEscape(assignment.Right, scopeOfTheContainingExpression);

result = Math.Max(result, rightValEscape);

var left = (BoundObjectInitializerMember)assignment.Left;
result = Math.Max(result, GetValEscape(left.Arguments, scopeOfTheContainingExpression));
Expand Down Expand Up @@ -3773,7 +3778,11 @@ private bool CheckValEscapeOfObjectInitializer(BoundObjectInitializerExpression
if (expression.Kind == BoundKind.AssignmentOperator)
{
var assignment = (BoundAssignmentOperator)expression;
if (!CheckValEscape(expression.Syntax, assignment.Right, escapeFrom, escapeTo, checkingReceiver: false, diagnostics: diagnostics))
bool valid = assignment.IsRef
? CheckRefEscape(expression.Syntax, assignment.Right, escapeFrom, escapeTo, checkingReceiver: false, diagnostics: diagnostics)
: CheckValEscape(expression.Syntax, assignment.Right, escapeFrom, escapeTo, checkingReceiver: false, diagnostics: diagnostics);

if (!valid)
{
return false;
}
Expand Down
146 changes: 78 additions & 68 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4588,6 +4588,7 @@ private BoundObjectInitializerExpressionBase BindInitializerExpression(
private BoundExpression BindInitializerExpressionOrValue(
ExpressionSyntax syntax,
TypeSymbol type,
BindValueKind rhsValueKind,
SyntaxNode typeSyntax,
BindingDiagnosticBag diagnostics)
{
Expand All @@ -4601,7 +4602,7 @@ private BoundExpression BindInitializerExpressionOrValue(
Debug.Assert(syntax.Parent.Parent.Kind() != SyntaxKind.WithInitializerExpression);
333fred marked this conversation as resolved.
Show resolved Hide resolved
return BindInitializerExpression((InitializerExpressionSyntax)syntax, type, typeSyntax, isForNewInstance: false, diagnostics);
default:
return BindValue(syntax, diagnostics, BindValueKind.RValue);
return BindValue(syntax, diagnostics, rhsValueKind);
}
}

Expand Down Expand Up @@ -4666,44 +4667,37 @@ private BoundExpression BindInitializerMemberAssignment(
{
var initializer = (AssignmentExpressionSyntax)memberInitializer;

// Bind member initializer identifier, i.e. left part of assignment
BoundExpression boundLeft = null;
var leftSyntax = initializer.Left;
// We use a location specific binder for binding object initializer field/property access to generate object initializer specific diagnostics:
// 1) CS1914 (ERR_StaticMemberInObjectInitializer)
// 2) CS1917 (ERR_ReadonlyValueTypeInObjectInitializer)
// 3) CS1918 (ERR_ValueTypePropertyInObjectInitializer)
// See comments in BindObjectInitializerExpression for more details.

if (initializerType.IsDynamic() && leftSyntax.Kind() == SyntaxKind.IdentifierName)
{
{
// D = { ..., <identifier> = <expr>, ... }, where D : dynamic
var memberName = ((IdentifierNameSyntax)leftSyntax).Identifier.Text;
boundLeft = new BoundDynamicObjectInitializerMember(leftSyntax, memberName, implicitReceiver.Type, initializerType, hasErrors: false);
Copy link
Member Author

@jcouv jcouv Jul 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

📝 The binding of dynamic LHS was moved into BindObjectInitializerMember which handled other bindings of LHS. #Resolved

}
}
else
{
// We use a location specific binder for binding object initializer field/property access to generate object initializer specific diagnostics:
// 1) CS1914 (ERR_StaticMemberInObjectInitializer)
// 2) CS1917 (ERR_ReadonlyValueTypeInObjectInitializer)
// 3) CS1918 (ERR_ValueTypePropertyInObjectInitializer)
// See comments in BindObjectInitializerExpression for more details.
Debug.Assert(objectInitializerMemberBinder != null);

Debug.Assert(objectInitializerMemberBinder != null);

boundLeft = objectInitializerMemberBinder.BindObjectInitializerMember(initializer, implicitReceiver, diagnostics);
}
BoundExpression boundLeft = objectInitializerMemberBinder.BindObjectInitializerMember(initializer, implicitReceiver, diagnostics);

if (boundLeft != null)
{
Debug.Assert((object)boundLeft.Type != null);

var rhsExpr = initializer.Right.CheckAndUnwrapRefExpression(diagnostics, out RefKind refKind);
bool isRef = refKind == RefKind.Ref;
var rhsKind = isRef ? GetRequiredRHSValueKindForRefAssignment(boundLeft) : BindValueKind.RValue;

// Bind member initializer value, i.e. right part of assignment
BoundExpression boundRight = BindInitializerExpressionOrValue(
syntax: initializer.Right,
syntax: rhsExpr,
type: boundLeft.Type,
rhsKind,
typeSyntax: boundLeft.Syntax,
diagnostics: diagnostics);

// Bind member initializer assignment expression
return BindAssignment(initializer, boundLeft, boundRight, isRef: false, diagnostics);
// We don't verify escape safety of initializers against the instance because the initializers
// get factored in when determining the safe-to-escape of the instance (the initializers contribute
// like constructor arguments).
return BindAssignment(initializer, boundLeft, boundRight, isRef, verifyEscapeSafety: false, diagnostics);
}
}

Expand All @@ -4722,43 +4716,60 @@ private BoundExpression BindObjectInitializerMember(
LookupResultKind resultKind;
bool hasErrors;

if (namedAssignment.Left.Kind() == SyntaxKind.IdentifierName)
{
var memberName = (IdentifierNameSyntax)namedAssignment.Left;
var leftSyntax = namedAssignment.Left;
var initializerType = implicitReceiver.Type;
SyntaxKind rhsKind = namedAssignment.Right.Kind();
bool isRef = rhsKind is SyntaxKind.RefExpression;
bool isRhsNestedInitializer = rhsKind is SyntaxKind.ObjectInitializerExpression or SyntaxKind.CollectionInitializerExpression;
BindValueKind valueKind = isRhsNestedInitializer ? BindValueKind.RValue : (isRef ? BindValueKind.RefAssignable : BindValueKind.Assignable);

// SPEC: Each member initializer must name an accessible field or property of the object being initialized, followed by an equals sign and
// SPEC: an expression or an object initializer or collection initializer.
// SPEC: A member initializer that specifies an expression after the equals sign is processed in the same way as an assignment (7.17.1) to the field or property.
if (leftSyntax.Kind() == SyntaxKind.IdentifierName)
{
var memberName = (IdentifierNameSyntax)leftSyntax;

// SPEC VIOLATION: Native compiler also allows initialization of field-like events in object initializers, so we allow it as well.

boundMember = BindInstanceMemberAccess(
node: memberName,
right: memberName,
boundLeft: implicitReceiver,
rightName: memberName.Identifier.ValueText,
rightArity: 0,
typeArgumentsSyntax: default(SeparatedSyntaxList<TypeSyntax>),
typeArgumentsWithAnnotations: default(ImmutableArray<TypeWithAnnotations>),
invoked: false,
indexed: false,
diagnostics: diagnostics);
if (initializerType.IsDynamic())
{
// D = { ..., <identifier> = <expr>, ... }, where D : dynamic
boundMember = new BoundDynamicObjectInitializerMember(leftSyntax, memberName.Identifier.Text, implicitReceiver.Type, initializerType, hasErrors: false);
return CheckValue(boundMember, valueKind, diagnostics);
}
else
{
// SPEC: Each member initializer must name an accessible field or property of the object being initialized, followed by an equals sign and
// SPEC: an expression or an object initializer or collection initializer.
// SPEC: A member initializer that specifies an expression after the equals sign is processed in the same way as an assignment (7.17.1) to the field or property.

// SPEC VIOLATION: Native compiler also allows initialization of field-like events in object initializers, so we allow it as well.

boundMember = BindInstanceMemberAccess(
node: memberName,
right: memberName,
boundLeft: implicitReceiver,
rightName: memberName.Identifier.ValueText,
rightArity: 0,
typeArgumentsSyntax: default,
typeArgumentsWithAnnotations: default,
invoked: false,
indexed: false,
diagnostics: diagnostics);

resultKind = boundMember.ResultKind;
hasErrors = boundMember.HasAnyErrors || implicitReceiver.HasAnyErrors;
hasErrors = boundMember.HasAnyErrors || implicitReceiver.HasAnyErrors;

if (boundMember.Kind == BoundKind.PropertyGroup)
{
boundMember = BindIndexedPropertyAccess((BoundPropertyGroup)boundMember, mustHaveAllOptionalParameters: true, diagnostics: diagnostics);
if (boundMember.HasAnyErrors)
if (boundMember.Kind == BoundKind.PropertyGroup)
{
hasErrors = true;
boundMember = BindIndexedPropertyAccess((BoundPropertyGroup)boundMember, mustHaveAllOptionalParameters: true, diagnostics: diagnostics);
if (boundMember.HasAnyErrors)
{
hasErrors = true;
}
}
}

resultKind = boundMember.ResultKind;
}
else if (namedAssignment.Left.Kind() == SyntaxKind.ImplicitElementAccess)
else if (leftSyntax.Kind() == SyntaxKind.ImplicitElementAccess)
{
var implicitIndexing = (ImplicitElementAccessSyntax)namedAssignment.Left;
var implicitIndexing = (ImplicitElementAccessSyntax)leftSyntax;
boundMember = BindElementAccess(implicitIndexing, implicitReceiver, implicitIndexing.ArgumentList, diagnostics);

resultKind = boundMember.ResultKind;
Expand All @@ -4784,15 +4795,12 @@ private BoundExpression BindObjectInitializerMember(
// TODO: If/when we have a way to version warnings, we should add a warning for this.

BoundKind boundMemberKind = boundMember.Kind;
SyntaxKind rhsKind = namedAssignment.Right.Kind();
bool isRhsNestedInitializer = rhsKind == SyntaxKind.ObjectInitializerExpression || rhsKind == SyntaxKind.CollectionInitializerExpression;
BindValueKind valueKind = isRhsNestedInitializer ? BindValueKind.RValue : BindValueKind.Assignable;

ImmutableArray<BoundExpression> arguments = ImmutableArray<BoundExpression>.Empty;
ImmutableArray<string> argumentNamesOpt = default(ImmutableArray<string>);
ImmutableArray<int> argsToParamsOpt = default(ImmutableArray<int>);
ImmutableArray<RefKind> argumentRefKindsOpt = default(ImmutableArray<RefKind>);
BitVector defaultArguments = default(BitVector);
ImmutableArray<string> argumentNamesOpt = default;
ImmutableArray<int> argsToParamsOpt = default;
ImmutableArray<RefKind> argumentRefKindsOpt = default;
BitVector defaultArguments = default;
bool expanded = false;

switch (boundMemberKind)
Expand All @@ -4805,7 +4813,7 @@ private BoundExpression BindObjectInitializerMember(
if (!hasErrors)
{
// TODO: distinct error code for collection initializers? (Dev11 doesn't have one.)
Error(diagnostics, ErrorCode.ERR_ReadonlyValueTypeInObjectInitializer, namedAssignment.Left, fieldSymbol, fieldSymbol.Type);
Error(diagnostics, ErrorCode.ERR_ReadonlyValueTypeInObjectInitializer, leftSyntax, fieldSymbol, fieldSymbol.Type);
hasErrors = true;
}

Expand All @@ -4818,14 +4826,14 @@ private BoundExpression BindObjectInitializerMember(
break;

case BoundKind.PropertyAccess:
hasErrors |= isRhsNestedInitializer && !CheckNestedObjectInitializerPropertySymbol(((BoundPropertyAccess)boundMember).PropertySymbol, namedAssignment.Left, diagnostics, hasErrors, ref resultKind);
hasErrors |= isRhsNestedInitializer && !CheckNestedObjectInitializerPropertySymbol(((BoundPropertyAccess)boundMember).PropertySymbol, leftSyntax, diagnostics, hasErrors, ref resultKind);
break;

case BoundKind.IndexerAccess:
{
var indexer = BindIndexerDefaultArguments((BoundIndexerAccess)boundMember, valueKind, diagnostics);
boundMember = indexer;
hasErrors |= isRhsNestedInitializer && !CheckNestedObjectInitializerPropertySymbol(indexer.Indexer, namedAssignment.Left, diagnostics, hasErrors, ref resultKind);
hasErrors |= isRhsNestedInitializer && !CheckNestedObjectInitializerPropertySymbol(indexer.Indexer, leftSyntax, diagnostics, hasErrors, ref resultKind);
arguments = indexer.Arguments;
argumentNamesOpt = indexer.ArgumentNamesOpt;
argsToParamsOpt = indexer.ArgsToParamsOpt;
Expand Down Expand Up @@ -4854,6 +4862,9 @@ private BoundExpression BindObjectInitializerMember(
break;
}

case BoundKind.DynamicObjectInitializerMember:
break;

case BoundKind.DynamicIndexerAccess:
{
var indexer = (BoundDynamicIndexerAccess)boundMember;
Expand All @@ -4866,10 +4877,10 @@ private BoundExpression BindObjectInitializerMember(

case BoundKind.ArrayAccess:
case BoundKind.PointerElementAccess:
return boundMember;
return CheckValue(boundMember, valueKind, diagnostics);
Copy link
Member

@cston cston Jul 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

CheckValue

Was this a missing case previously? #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

Right. We were returning so bypassing the CheckValueKind check below.
Without this fix, we'd have no error in RefInitializer_OnArray or RefInitializer_OnPointer


default:
return BadObjectInitializerMemberAccess(boundMember, implicitReceiver, namedAssignment.Left, diagnostics, valueKind, hasErrors);
return BadObjectInitializerMemberAccess(boundMember, implicitReceiver, leftSyntax, diagnostics, valueKind, hasErrors);
}

if (!hasErrors)
Expand All @@ -4885,7 +4896,7 @@ private BoundExpression BindObjectInitializerMember(
}

return new BoundObjectInitializerMember(
namedAssignment.Left,
leftSyntax,
boundMember.ExpressionSymbol,
arguments,
argumentNamesOpt,
Expand Down Expand Up @@ -5200,7 +5211,7 @@ private BoundExpression BindCollectionInitializerElement(
Error(diagnostics, ErrorCode.ERR_InvalidInitializerElementInitializer, elementInitializer);
}

var boundElementInitializer = BindInitializerExpressionOrValue(elementInitializer, initializerType, implicitReceiver.Syntax, diagnostics);
var boundElementInitializer = BindInitializerExpressionOrValue(elementInitializer, initializerType, BindValueKind.RValue, implicitReceiver.Syntax, diagnostics);

BoundExpression result = BindCollectionInitializerElementAddMethod(
elementInitializer,
Expand Down Expand Up @@ -5422,7 +5433,6 @@ protected BoundExpression BindClassCreationExpression(
TypeSymbol initializerTypeOpt = null,
bool wasTargetTyped = false)
{

BoundExpression result = null;
bool hasErrors = type.IsErrorType();
if (type.IsAbstract)
Expand Down
Loading