Skip to content

Commit

Permalink
Differentiate l-value and r-value property use in ctor
Browse files Browse the repository at this point in the history
  • Loading branch information
cston committed Sep 18, 2024
1 parent 1443e54 commit a1ecdc9
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 87 deletions.
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1694,7 +1694,7 @@ private bool CheckPropertyValueKind(SyntaxNode node, BoundExpression expr, BindV
if (setMethod is null)
{
var containing = this.ContainingMemberOrLambda;
if (!AccessingAutoPropertyFromConstructor(receiver, propertySymbol, containing)
if (!AccessingAutoPropertyFromConstructor(receiver, propertySymbol, containing, useAsLvalue: true)
&& !isAllowedDespiteReadonly(receiver))
{
Error(diagnostics, ErrorCode.ERR_AssgReadonlyProp, node, propertySymbol);
Expand Down
12 changes: 5 additions & 7 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1749,15 +1749,13 @@ private DiagnosticInfo GetBadEventUsageDiagnosticInfo(EventSymbol eventSymbol)
new CSDiagnosticInfo(ErrorCode.ERR_BadEventUsageNoField, leastOverridden);
}

internal static bool AccessingAutoPropertyFromConstructor(BoundPropertyAccess propertyAccess, Symbol fromMember)
internal static bool AccessingAutoPropertyFromConstructor(BoundPropertyAccess propertyAccess, Symbol fromMember, bool useAsLvalue = false)
{
return AccessingAutoPropertyFromConstructor(propertyAccess.ReceiverOpt, propertyAccess.PropertySymbol, fromMember);
return AccessingAutoPropertyFromConstructor(propertyAccess.ReceiverOpt, propertyAccess.PropertySymbol, fromMember, useAsLvalue);
}

// PROTOTYPE: Is this method used only when writing to the backing field, or also when reading?
// If it's used for reading as well, then we need to check that the getter is auto-implemented in
// that case, rather than checking CanAssignBackingFieldDirectlyInConstructor.
private static bool AccessingAutoPropertyFromConstructor(BoundExpression receiver, PropertySymbol propertySymbol, Symbol fromMember)
// PROTOTYPE: Is useAsLvalue set correctly in all callers?
private static bool AccessingAutoPropertyFromConstructor(BoundExpression receiver, PropertySymbol propertySymbol, Symbol fromMember, bool useAsLvalue)
{
if (!propertySymbol.IsDefinition && propertySymbol.ContainingType.Equals(propertySymbol.ContainingType.OriginalDefinition, TypeCompareKind.IgnoreNullableModifiersForReferenceTypes))
{
Expand All @@ -1768,7 +1766,7 @@ private static bool AccessingAutoPropertyFromConstructor(BoundExpression receive
var propertyIsStatic = propertySymbol.IsStatic;

return (object)sourceProperty != null &&
sourceProperty.CanAssignBackingFieldDirectlyInConstructor &&
sourceProperty.CanUseBackingFieldDirectlyInConstructor(useAsLvalue) &&
TypeSymbol.Equals(sourceProperty.ContainingType, fromMember.ContainingType, TypeCompareKind.AllIgnoreOptions) &&
IsConstructorOrField(fromMember, isStatic: propertyIsStatic) &&
(propertyIsStatic || receiver.Kind == BoundKind.ThisReference);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ protected void VisitLvalue(BoundExpression node)
{
var access = (BoundPropertyAccess)node;

if (Binder.AccessingAutoPropertyFromConstructor(access, _symbol))
if (Binder.AccessingAutoPropertyFromConstructor(access, _symbol, useAsLvalue: true))
{
var backingField = (access.PropertySymbol as SourcePropertySymbolBase)?.BackingField;
if (backingField != null)
Expand Down Expand Up @@ -2026,6 +2026,7 @@ protected virtual void PropertySetter(BoundExpression node, BoundExpression rece
VisitReceiverAfterCall(receiver, setter);
}

// PROTOTYPE: Test all uses of this method.
// returns false if expression is not a property access
// or if the property has a backing field
// and accessed in a corresponding constructor
Expand All @@ -2036,7 +2037,7 @@ private bool RegularPropertyAccess(BoundExpression expr)
return false;
}

return !Binder.AccessingAutoPropertyFromConstructor((BoundPropertyAccess)expr, _symbol);
return !Binder.AccessingAutoPropertyFromConstructor((BoundPropertyAccess)expr, _symbol, useAsLvalue: true);
}

public override BoundNode VisitAssignmentOperator(BoundAssignmentOperator node)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1060,7 +1060,7 @@ protected override void Normalize(ref LocalState state)
}
}

protected override bool TryGetReceiverAndMember(BoundExpression expr, out BoundExpression receiver, out Symbol member)
protected override bool TryGetReceiverAndMember(BoundExpression expr, out BoundExpression receiver, out Symbol member, bool useAsLvalue)
{
receiver = null;
member = null;
Expand Down Expand Up @@ -1099,7 +1099,7 @@ protected override bool TryGetReceiverAndMember(BoundExpression expr, out BoundE
{
var propAccess = (BoundPropertyAccess)expr;

if (Binder.AccessingAutoPropertyFromConstructor(propAccess, this.CurrentSymbol))
if (Binder.AccessingAutoPropertyFromConstructor(propAccess, this.CurrentSymbol, useAsLvalue))
{
var propSymbol = propAccess.PropertySymbol;
member = (propSymbol as SourcePropertySymbolBase)?.BackingField;
Expand Down Expand Up @@ -1640,7 +1640,7 @@ protected virtual void AssignImpl(BoundNode node, BoundExpression value, bool is
case BoundKind.PropertyAccess:
{
var expression = (BoundExpression)node;
int slot = MakeSlot(expression);
int slot = MakeSlot(expression, useAsLvalue: true);
SetSlotState(slot, written);
if (written) NoteWrite(expression, value, read);
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,14 +201,14 @@ private int DescendThroughTupleRestFields(ref Symbol symbol, int containingSlot,
return containingSlot;
}

protected abstract bool TryGetReceiverAndMember(BoundExpression expr, out BoundExpression? receiver, [NotNullWhen(true)] out Symbol? member);
protected abstract bool TryGetReceiverAndMember(BoundExpression expr, out BoundExpression? receiver, [NotNullWhen(true)] out Symbol? member, bool useAsLvalue = false);

/// <summary>
/// Return the slot for a variable, or -1 if it is not tracked (because, for example, it is an empty struct).
/// </summary>
/// <param name="node"></param>
/// <returns></returns>
protected virtual int MakeSlot(BoundExpression node)
protected virtual int MakeSlot(BoundExpression node, bool useAsLvalue = false)
{
switch (node.Kind)
{
Expand All @@ -224,7 +224,7 @@ protected virtual int MakeSlot(BoundExpression node)
case BoundKind.FieldAccess:
case BoundKind.EventAccess:
case BoundKind.PropertyAccess:
if (TryGetReceiverAndMember(node, out BoundExpression? receiver, out Symbol? member))
if (TryGetReceiverAndMember(node, out BoundExpression? receiver, out Symbol? member, useAsLvalue))
{
Debug.Assert((receiver is null) != member.RequiresInstanceReceiver());
return MakeMemberSlot(receiver, member);
Expand Down
9 changes: 4 additions & 5 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1927,7 +1927,7 @@ private NullableFlowState GetDefaultState(ref LocalState state, int slot)
}
}

protected override bool TryGetReceiverAndMember(BoundExpression expr, out BoundExpression? receiver, [NotNullWhen(true)] out Symbol? member)
protected override bool TryGetReceiverAndMember(BoundExpression expr, out BoundExpression? receiver, [NotNullWhen(true)] out Symbol? member, bool useAsLvalue)
{
receiver = null;
member = null;
Expand Down Expand Up @@ -1968,6 +1968,8 @@ protected override bool TryGetReceiverAndMember(BoundExpression expr, out BoundE
var propAccess = (BoundPropertyAccess)expr;
var propSymbol = propAccess.PropertySymbol;
member = propSymbol;
// PROTOTYPE: Why don't we call Binder.AccessingAutoPropertyFromConstructor
// to match the DefiniteAssignment implementation?
if (propSymbol.IsStatic)
{
return true;
Expand All @@ -1985,7 +1987,7 @@ receiver is object &&
receiver.Type is object;
}

protected override int MakeSlot(BoundExpression node)
protected override int MakeSlot(BoundExpression node, bool useAsLvalue = false)
{
int result = makeSlot(node);
#if DEBUG
Expand Down Expand Up @@ -9754,9 +9756,6 @@ private void VisitThisOrBaseReference(BoundExpression node)
Debug.Assert(!IsConditionalState);

var left = node.Left;
// PROTOTYPE: We cannot simply use the associated property if the field is a backing field
// if the property uses `field`, because the relationship between property and field may
// be something other than { get => field; set { field = value; } }.
switch (left)
{
// when binding initializers, we treat assignments to auto-properties or field-like events as direct assignments to the underlying field.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -668,10 +668,23 @@ internal bool HasAutoPropertySet

/// <summary>
/// True if the property has a synthesized backing field, and
/// either no setter or the setter is auto-implemented.
/// either no accessor or the accessor is auto-implemented.
/// </summary>
internal bool CanAssignBackingFieldDirectlyInConstructor
=> BackingField is { } && (SetMethod is null || HasAutoPropertySet);
internal bool CanUseBackingFieldDirectlyInConstructor(bool useAsLvalue)
{
if (BackingField is null)
{
return false;
}
if (useAsLvalue)
{
return SetMethod is null || HasAutoPropertySet;
}
else
{
return GetMethod is null || HasAutoPropertyGet;
}
}

private bool IsSetOnEitherPart(Flags flags)
{
Expand Down
Loading

0 comments on commit a1ecdc9

Please sign in to comment.