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

Refactor IsAutoPropertyWithGetAccessor usage in MethodCompiler #58515

Merged
merged 12 commits into from
Jan 4, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,13 @@ internal static BoundBlock ConstructAutoPropertyAccessorBody(SourceMemberMethodS
}

var field = property.BackingField;
if (field is null)
{
// This happens for public int { set; } where we produce ERR_AutoPropertyMustHaveGetAccessor
Debug.Assert(!property.HasGetAccessor);
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
return null;
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
}

Debug.Assert(!field.IsCreatedForFieldKeyword);
var fieldAccess = new BoundFieldAccess(syntax, thisReference, field, ConstantValue.NotAvailable) { WasCompilerGenerated = true };
BoundStatement statement;
Expand Down
3 changes: 1 addition & 2 deletions src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1869,8 +1869,7 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax &&
}
else
{
var property = sourceMethod.AssociatedSymbol as SourcePropertySymbolBase;
if ((object)property != null && property.IsAutoPropertyWithGetAccessor) // PROTOTYPE: Just IsAutoProperty, or _isAutoPropertyAccessor?
if (sourceMethod is SourcePropertyAccessorSymbol { AccessorBodyShouldBeSynthesized: true })
{
return MethodBodySynthesizer.ConstructAutoPropertyAccessorBody(sourceMethod);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ internal class SourcePropertyAccessorSymbol : SourceMemberMethodSymbol
private ImmutableArray<CustomModifier> _lazyRefCustomModifiers;
private ImmutableArray<MethodSymbol> _lazyExplicitInterfaceImplementations;
private string _lazyName;
private readonly bool _isAutoPropertyAccessor;
private readonly bool _accessorBodyShouldBeSynthesized;
private readonly bool _isExpressionBodied;
private readonly bool _containsFieldKeyword;
private readonly bool _usesInit;
Expand All @@ -32,7 +32,6 @@ public static SourcePropertyAccessorSymbol CreateAccessorSymbol(
SourcePropertySymbol property,
DeclarationModifiers propertyModifiers,
AccessorDeclarationSyntax syntax,
bool isAutoPropertyAccessor,
BindingDiagnosticBag diagnostics)
{
Debug.Assert(syntax.Kind() == SyntaxKind.GetAccessorDeclaration ||
Expand All @@ -58,7 +57,6 @@ public static SourcePropertyAccessorSymbol CreateAccessorSymbol(
syntax.Modifiers,
methodKind,
syntax.Keyword.IsKind(SyntaxKind.InitKeyword),
isAutoPropertyAccessor,
isNullableAnalysisEnabled: isNullableAnalysisEnabled,
diagnostics);
}
Expand Down Expand Up @@ -105,7 +103,6 @@ public static SourcePropertyAccessorSymbol CreateAccessorSymbol(
modifiers: new SyntaxTokenList(),
methodKind,
usesInit,
isAutoPropertyAccessor: true,
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
isNullableAnalysisEnabled: false,
diagnostics);
}
Expand Down Expand Up @@ -166,7 +163,7 @@ private SourcePropertyAccessorSymbol(
base(containingType, syntax.GetReference(), location, isIterator: false)
{
_property = property;
_isAutoPropertyAccessor = false;
_accessorBodyShouldBeSynthesized = false;
_isExpressionBodied = true;
_containsFieldKeyword = property.IsIndexer ? false : NodeContainsFieldKeyword(syntax);

Expand Down Expand Up @@ -204,7 +201,6 @@ protected SourcePropertyAccessorSymbol(
SyntaxTokenList modifiers,
MethodKind methodKind,
bool usesInit,
bool isAutoPropertyAccessor,
bool isNullableAnalysisEnabled,
BindingDiagnosticBag diagnostics)
: base(containingType,
Expand All @@ -213,7 +209,8 @@ protected SourcePropertyAccessorSymbol(
isIterator)
{
_property = property;
_isAutoPropertyAccessor = isAutoPropertyAccessor;
_accessorBodyShouldBeSynthesized = (property.IsAutoProperty && syntax is AccessorDeclarationSyntax { SemicolonToken.RawKind: (int)SyntaxKind.SemicolonToken, ExpressionBody: null, Body: null }) ||
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
property is SynthesizedRecordPropertySymbol;
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
_containsFieldKeyword = property.IsIndexer ? false : NodeContainsFieldKeyword(getAccessorSyntax(syntax));
Debug.Assert(!_property.IsExpressionBodied, "Cannot have accessors in expression bodied lightweight properties");
_isExpressionBodied = !hasBody && hasExpressionBody;
Expand All @@ -239,7 +236,7 @@ protected SourcePropertyAccessorSymbol(
this.MakeFlags(methodKind, declarationModifiers, returnsVoid: false, isExtensionMethod: false, isNullableAnalysisEnabled: isNullableAnalysisEnabled,
isMetadataVirtualIgnoringModifiers: property.IsExplicitInterfaceImplementation && (declarationModifiers & DeclarationModifiers.Static) == 0);

CheckFeatureAvailabilityAndRuntimeSupport(syntax, location, hasBody: hasBody || hasExpressionBody || isAutoPropertyAccessor, diagnostics);
CheckFeatureAvailabilityAndRuntimeSupport(syntax, location, hasBody: hasBody || hasExpressionBody || _accessorBodyShouldBeSynthesized, diagnostics);

if (hasBody || hasExpressionBody)
{
Expand All @@ -254,7 +251,7 @@ protected SourcePropertyAccessorSymbol(

if (!modifierErrors)
{
this.CheckModifiers(location, hasBody || hasExpressionBody, isAutoPropertyAccessor, diagnostics);
this.CheckModifiers(location, hasBody || hasExpressionBody, _accessorBodyShouldBeSynthesized, diagnostics);
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
}

static CSharpSyntaxNode? getAccessorSyntax(CSharpSyntaxNode node)
Expand Down Expand Up @@ -478,6 +475,11 @@ internal Accessibility LocalAccessibility
/// </remarks>
internal bool ContainsFieldKeyword => _containsFieldKeyword;

/// <summary>
/// Indicates whether this accessor has no body in source and a body will be synthesized by the compiler.
/// </summary>
internal bool AccessorBodyShouldBeSynthesized => _accessorBodyShouldBeSynthesized;
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Indicates whether this accessor is readonly due to reasons scoped to itself and its containing property.
/// </summary>
Expand Down Expand Up @@ -527,7 +529,7 @@ internal sealed override bool IsDeclaredReadOnly

return ContainingType.IsStructType() &&
!_property.IsStatic &&
_isAutoPropertyAccessor &&
_accessorBodyShouldBeSynthesized &&
MethodKind == MethodKind.PropertyGet;
}
}
Expand Down Expand Up @@ -602,7 +604,7 @@ private void CheckModifiers(Location location, bool hasBody, bool isAutoProperty
// 'init' accessors cannot be marked 'readonly'. Mark '{0}' readonly instead.
diagnostics.Add(ErrorCode.ERR_InitCannotBeReadonly, location, _property);
}
else if (LocalDeclaredReadOnly && _isAutoPropertyAccessor && MethodKind == MethodKind.PropertySet)
else if (LocalDeclaredReadOnly && _accessorBodyShouldBeSynthesized && MethodKind == MethodKind.PropertySet)
{
// Auto-implemented accessor '{0}' cannot be marked 'readonly'.
diagnostics.Add(ErrorCode.ERR_AutoSetterCantBeReadOnly, location, this);
Expand Down Expand Up @@ -835,7 +837,7 @@ internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, r
{
base.AddSynthesizedAttributes(moduleBuilder, ref attributes);

if (_isAutoPropertyAccessor)
if (_accessorBodyShouldBeSynthesized)
{
var compilation = this.DeclaringCompilation;
AddSynthesizedAttribute(ref attributes, compilation.TrySynthesizeAttribute(WellKnownMember.System_Runtime_CompilerServices_CompilerGeneratedAttribute__ctor));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ private static DeclarationModifiers MakeModifiers(
return mods;
}

protected override SourcePropertyAccessorSymbol CreateGetAccessorSymbol(bool isAutoPropertyAccessor, BindingDiagnosticBag diagnostics)
protected override SourcePropertyAccessorSymbol CreateGetAccessorSymbol(BindingDiagnosticBag diagnostics)
{
var syntax = (BasePropertyDeclarationSyntax)CSharpSyntaxNode;
ArrowExpressionClauseSyntax? arrowExpression = GetArrowExpression(syntax);
Expand All @@ -366,29 +366,27 @@ protected override SourcePropertyAccessorSymbol CreateGetAccessorSymbol(bool isA
}
else
{
return CreateAccessorSymbol(GetGetAccessorDeclaration(syntax), isAutoPropertyAccessor, diagnostics);
return CreateAccessorSymbol(GetGetAccessorDeclaration(syntax), diagnostics);
}
}

protected override SourcePropertyAccessorSymbol CreateSetAccessorSymbol(bool isAutoPropertyAccessor, BindingDiagnosticBag diagnostics)
protected override SourcePropertyAccessorSymbol CreateSetAccessorSymbol(BindingDiagnosticBag diagnostics)
{
var syntax = (BasePropertyDeclarationSyntax)CSharpSyntaxNode;
Debug.Assert(!(syntax.AccessorList is null && GetArrowExpression(syntax) != null));

return CreateAccessorSymbol(GetSetAccessorDeclaration(syntax), isAutoPropertyAccessor, diagnostics);
return CreateAccessorSymbol(GetSetAccessorDeclaration(syntax), diagnostics);
}

private SourcePropertyAccessorSymbol CreateAccessorSymbol(
AccessorDeclarationSyntax syntax,
bool isAutoPropertyAccessor,
BindingDiagnosticBag diagnostics)
{
return SourcePropertyAccessorSymbol.CreateAccessorSymbol(
ContainingType,
this,
_modifiers,
syntax,
isAutoPropertyAccessor,
diagnostics);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,11 @@ protected SourcePropertySymbolBase(

if (hasGetAccessor)
{
_getMethod = CreateGetAccessorSymbol(isAutoPropertyAccessor: isAutoProperty, diagnostics);
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
_getMethod = CreateGetAccessorSymbol(diagnostics);
}
if (hasSetAccessor)
{
_setMethod = CreateSetAccessorSymbol(isAutoPropertyAccessor: isAutoProperty, diagnostics);
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
_setMethod = CreateSetAccessorSymbol(diagnostics);
}
}

Expand Down Expand Up @@ -643,17 +643,13 @@ internal bool IsNew
/// The method is called at the end of <see cref="SourcePropertySymbolBase"/> constructor.
/// The implementation may depend only on information available from the <see cref="SourcePropertySymbolBase"/> type.
/// </summary>
protected abstract SourcePropertyAccessorSymbol CreateGetAccessorSymbol(
bool isAutoPropertyAccessor,
BindingDiagnosticBag diagnostics);
protected abstract SourcePropertyAccessorSymbol CreateGetAccessorSymbol(BindingDiagnosticBag diagnostics);

/// <summary>
/// The method is called at the end of <see cref="SourcePropertySymbolBase"/> constructor.
/// The implementation may depend only on information available from the <see cref="SourcePropertySymbolBase"/> type.
/// </summary>
protected abstract SourcePropertyAccessorSymbol CreateSetAccessorSymbol(
bool isAutoPropertyAccessor,
BindingDiagnosticBag diagnostics);
protected abstract SourcePropertyAccessorSymbol CreateSetAccessorSymbol(BindingDiagnosticBag diagnostics);

public sealed override MethodSymbol? GetMethod
{
Expand Down Expand Up @@ -690,7 +686,7 @@ public sealed override ImmutableArray<ParameterSymbol> Parameters
internal override bool IsExplicitInterfaceImplementation
=> (_propertyFlags & Flags.IsExplicitInterfaceImplementation) != 0;

private bool HasGetAccessor => (_propertyFlags & Flags.HasGetAccessor) != 0;
internal bool HasGetAccessor => (_propertyFlags & Flags.HasGetAccessor) != 0;
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved

private bool HasSetAccessor => (_propertyFlags & Flags.HasSetAccessor) != 0;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public override SyntaxList<AttributeListSyntax> AttributeDeclarationSyntaxList
protected override Location TypeLocation
=> ContainingType.Locations[0];

protected override SourcePropertyAccessorSymbol CreateGetAccessorSymbol(bool isAutoPropertyAccessor, BindingDiagnosticBag diagnostics)
protected override SourcePropertyAccessorSymbol CreateGetAccessorSymbol(BindingDiagnosticBag diagnostics)
{
return SourcePropertyAccessorSymbol.CreateAccessorSymbol(
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
ContainingType,
Expand All @@ -66,7 +66,7 @@ protected override SourcePropertyAccessorSymbol CreateGetAccessorSymbol(bool isA
diagnostics);
}

protected override SourcePropertyAccessorSymbol CreateSetAccessorSymbol(bool isAutoPropertyAccessor, BindingDiagnosticBag diagnostics)
protected override SourcePropertyAccessorSymbol CreateSetAccessorSymbol(BindingDiagnosticBag diagnostics)
{
throw ExceptionUtilities.Unreachable;
}
Expand Down Expand Up @@ -136,7 +136,6 @@ internal GetAccessorSymbol(
modifiers: new SyntaxTokenList(),
MethodKind.PropertyGet,
usesInit: false,
isAutoPropertyAccessor: false,
isNullableAnalysisEnabled: false,
diagnostics)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,13 @@ protected override Location TypeLocation
public override SyntaxList<AttributeListSyntax> AttributeDeclarationSyntaxList
=> BackingParameter.AttributeDeclarationList;

protected override SourcePropertyAccessorSymbol CreateGetAccessorSymbol(bool isAutoPropertyAccessor, BindingDiagnosticBag diagnostics)
protected override SourcePropertyAccessorSymbol CreateGetAccessorSymbol(BindingDiagnosticBag diagnostics)
{
Debug.Assert(isAutoPropertyAccessor);
return CreateAccessorSymbol(isGet: true, CSharpSyntaxNode, diagnostics);
}

protected override SourcePropertyAccessorSymbol CreateSetAccessorSymbol(bool isAutoPropertyAccessor, BindingDiagnosticBag diagnostics)
protected override SourcePropertyAccessorSymbol CreateSetAccessorSymbol(BindingDiagnosticBag diagnostics)
{
Debug.Assert(isAutoPropertyAccessor);
return CreateAccessorSymbol(isGet: false, CSharpSyntaxNode, diagnostics);
}

Expand Down