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.

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

Use init-only for records and adjust binding #44582

Merged
merged 9 commits into from
May 30, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
97 changes: 31 additions & 66 deletions src/Compilers/CSharp/Portable/Binder/Binder_WithExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ internal partial class Binder
{
private BoundExpression BindWithExpression(WithExpressionSyntax syntax, DiagnosticBag diagnostics)
{
// PROTOTYPE: this entire method is likely to change
var receiver = BindRValueWithoutTargetType(syntax.Receiver, diagnostics);
var receiverType = receiver.Type;

Expand Down Expand Up @@ -95,85 +94,51 @@ private BoundExpression BindWithExpression(WithExpressionSyntax syntax, Diagnost
}

var cloneReturnType = cloneMethod?.ReturnType;

var args = ArrayBuilder<(Symbol?, BoundExpression)>.GetInstance();
// PROTOTYPE: Handle dynamic
var implicitReceiver = new BoundObjectOrCollectionValuePlaceholder(
agocke marked this conversation as resolved.
Show resolved Hide resolved
syntax.Receiver,
// formatting
#pragma warning disable IDE0055
agocke marked this conversation as resolved.
Show resolved Hide resolved
cloneReturnType ?? receiverType) { WasCompilerGenerated = true };
#pragma warning restore IDE0055

var args = ArrayBuilder<BoundExpression>.GetInstance();
// Bind with expression arguments
foreach (var expr in syntax.Initializer.Expressions)
{
Symbol? member = null;
BoundExpression boundRight;
BoundExpression boundExpr;
// We're expecting a simple assignment only, with an ID on the left
if (!(expr is AssignmentExpressionSyntax assignment) ||
agocke marked this conversation as resolved.
Show resolved Hide resolved
!(assignment.Left is IdentifierNameSyntax left))
{
boundRight = BindExpression(expr, diagnostics);
boundExpr = BindExpression(expr, diagnostics);
hasErrors = true;
diagnostics.Add(ErrorCode.ERR_BadWithExpressionArgument, expr.Location);
}
else
{
var propName = left.Identifier.Text;
agocke marked this conversation as resolved.
Show resolved Hide resolved
if (!(cloneReturnType is null))
{
var location = left.Location;
this.LookupMembersInType(
lookupResult,
cloneReturnType,
propName,
arity: 0,
basesBeingResolved: null,
options: LookupOptions.Default,
originalBinder: this,
diagnose: false,
useSiteDiagnostics: ref useSiteDiagnostics);
// PROTOTYPE: Should handle hiding/overriding and bind like regular accesses
if (lookupResult.IsSingleViable &&
lookupResult.SingleSymbolOrDefault is var sym)
{
switch (sym.Kind)
{
case SymbolKind.Property:
member = sym;
// PROTOTYPE: this should check for init-only, but that isn't a separate feature yet
// It also will not work in metadata.
if (!(sym is SynthesizedRecordPropertySymbol))
{
goto default;
}
break;

default:
hasErrors = true;
diagnostics.Add(
ErrorCode.ERR_WithMemberIsNotRecordProperty,
location);
break;
}
}

if (!hasErrors && member is null)
{
hasErrors = true;
Error(
diagnostics,
ErrorCode.ERR_NoSuchMemberOrExtension,
location,
cloneReturnType,
propName);
}
}

boundRight = BindValue(assignment.Right, diagnostics, BindValueKind.RValue);
if (!(member is null))
{
boundRight = GenerateConversionForAssignment(
member.GetTypeOrReturnType().Type,
boundRight,
diagnostics);
}
lookupResult.Clear();
BoundExpression? boundMember = null;
agocke marked this conversation as resolved.
Show resolved Hide resolved
boundMember = BindInstanceMemberAccess(
node: left,
right: left,
boundLeft: implicitReceiver,
rightName: propName,
rightArity: 0,
typeArgumentsSyntax: default(SeparatedSyntaxList<TypeSyntax>),
typeArgumentsWithAnnotations: default(ImmutableArray<TypeWithAnnotations>),
invoked: false,
indexed: false,
diagnostics: diagnostics);

hasErrors |= boundMember.HasAnyErrors || implicitReceiver.HasAnyErrors;
agocke marked this conversation as resolved.
Show resolved Hide resolved
agocke marked this conversation as resolved.
Show resolved Hide resolved

boundMember = CheckValue(boundMember, BindValueKind.Assignable, diagnostics);
agocke marked this conversation as resolved.
Show resolved Hide resolved

var boundRight = BindValue(assignment.Right, diagnostics, BindValueKind.RValue);
boundExpr = BindAssignment(expr, boundMember, boundRight, isRef: false, diagnostics);
agocke marked this conversation as resolved.
Show resolved Hide resolved
agocke marked this conversation as resolved.
Show resolved Hide resolved
}
args.Add((member, boundRight));
args.Add(boundExpr);
}

lookupResult.Free();
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2131,7 +2131,7 @@
<!-- CloneMethod may be null in error scenarios-->
<Field Name="CloneMethod" Type="MethodSymbol?" />
<!-- Members and expressions passed as arguments to the With expression. -->
<Field Name="Arguments" Type="ImmutableArray&lt;(Symbol? Member, BoundExpression Expression)&gt;" />
<Field Name="Arguments" Type="ImmutableArray&lt;BoundExpression&gt;" />
agocke marked this conversation as resolved.
Show resolved Hide resolved
</Node>

</Tree>

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,37 @@ public override BoundNode VisitWithExpression(BoundWithExpression withExpr)
// tmp.Pn = En;
foreach (var arg in withExpr.Arguments)
{
RoslynDebug.AssertNotNull(arg.Member);
// PROTOTYPE: only works for source symbols
var prop = (SynthesizedRecordPropertySymbol)arg.Member;
stores.Add(F.AssignmentExpression(
(BoundExpression)F.Field((BoundExpression)receiverLocal, (FieldSymbol)prop.BackingField),
(BoundExpression)VisitExpression((BoundExpression)arg.Expression)
));
// If we got here without errors we should have a simple assignment with either a
// field or property on the left and a value expression on the right
var assignment = (BoundAssignmentOperator)arg;
// We need to construct the assignment LHS manually because swapping out the
// placeholder receiver could change the ref assignability of the expression, which
// the rest of lowering doesn't like
var left = assignment.Left.ExpressionSymbol switch
agocke marked this conversation as resolved.
Show resolved Hide resolved
{
PropertySymbol p => MakePropertyAccess(
withExpr.Syntax,
receiverLocal,
p,
receiverLocal.ResultKind,
p.Type,
isLeftOfAssignment: true),
FieldSymbol f => MakeFieldAccess(
withExpr.Syntax,
receiverLocal,
f,
constantValueOpt: null,
receiverLocal.ResultKind,
f.Type),
_ => throw ExceptionUtilities.UnexpectedValue(assignment.Left.Kind)
};
stores.Add(MakeStaticAssignmentOperator(
assignment.Syntax,
left,
VisitExpression(assignment.Right),
isRef: false,
left.Type!,
used: false));
}

return new BoundSequence(
Expand Down
5 changes: 3 additions & 2 deletions src/Compilers/CSharp/Portable/Symbols/LexicalSortKey.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,16 @@ public int Position
public static readonly LexicalSortKey NotInitialized = new LexicalSortKey() { _treeOrdinal = -1, _position = -1 };

// Put Record Equals right before synthesized constructors.
public static LexicalSortKey SynthesizedRecordEquals => new LexicalSortKey() { _treeOrdinal = int.MaxValue, _position = int.MaxValue - 4 };
public static LexicalSortKey SynthesizedRecordObjEquals => new LexicalSortKey() { _treeOrdinal = int.MaxValue, _position = int.MaxValue - 3 };
public static LexicalSortKey SynthesizedRecordEquals => new LexicalSortKey() { _treeOrdinal = int.MaxValue, _position = int.MaxValue - 5 };
public static LexicalSortKey SynthesizedRecordObjEquals => new LexicalSortKey() { _treeOrdinal = int.MaxValue, _position = int.MaxValue - 4 };


// Dev12 compiler adds synthetic constructors to the child list after adding all other members.
// Methods are emitted in the children order, but synthetic cctors would be deferred
// until later when it is known if they can be optimized or not.
// As a result the last emitted method tokens are synthetic ctor and then synthetic cctor (if not optimized)
// Since it is not too hard, we will try keeping the same order just to be easy on metadata diffing tools and such.
public static readonly LexicalSortKey SynthesizedRecordCopyCtor = new LexicalSortKey() { _treeOrdinal = int.MaxValue, _position = int.MaxValue - 3 };
public static readonly LexicalSortKey SynthesizedRecordCtor = new LexicalSortKey() { _treeOrdinal = int.MaxValue, _position = int.MaxValue - 2 };
public static readonly LexicalSortKey SynthesizedCtor = new LexicalSortKey() { _treeOrdinal = int.MaxValue, _position = int.MaxValue - 1 };
public static readonly LexicalSortKey SynthesizedCCtor = new LexicalSortKey() { _treeOrdinal = int.MaxValue, _position = int.MaxValue };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2946,13 +2946,19 @@ private void AddSynthesizedRecordMembersIfNecessary(MembersAndInitializersBuilde
BinderFactory binderFactory = this.DeclaringCompilation.GetBinderFactory(paramList.SyntaxTree);
var binder = binderFactory.GetBinder(paramList);

// PROTOTYPE: need to check base members as well
var memberSignatures = s_duplicateMemberSignatureDictionary.Allocate();
foreach (var member in members)
{
memberSignatures.Add(member, member);
}

var ctor = addCtor(paramList);
if (!this.IsStructType())
Copy link
Member

@gafter gafter May 27, 2020

Choose a reason for hiding this comment

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

if [](start = 12, length = 2)

This condition is not supported by the specification. #Resolved

{
addCopyCtor();
}
addCloneMethod();
addProperties(ctor.Parameters);
var thisEquals = addThisEquals();
addObjectEquals(thisEquals);
Expand All @@ -2977,15 +2983,34 @@ SynthesizedRecordConstructor addCtor(ParameterListSyntax paramList)
return ctor;
}

void addCopyCtor()
{
var ctor = new SynthesizedRecordCopyCtor(this, diagnostics);
if (!memberSignatures.ContainsKey(ctor))
{
members.Add(ctor);
}
}

void addCloneMethod()
{
var clone = new SynthesizedRecordClone(this);
if (!memberSignatures.ContainsKey(clone))
{
members.Add(clone);
}
}

void addProperties(ImmutableArray<ParameterSymbol> recordParameters)
{
foreach (ParameterSymbol param in ctor.Parameters)
{
var property = new SynthesizedRecordPropertySymbol(this, param);
var property = new SynthesizedRecordPropertySymbol(this, param, diagnostics);
if (!memberSignatures.ContainsKey(property))
{
members.Add(property);
members.Add(property.GetMethod);
members.Add(property.SetMethod);
members.Add(property.BackingField);
}
}
Expand Down
Loading