Skip to content

Commit

Permalink
Refactor IsAutoPropertyWithGetAccessor usage in MethodCompiler (#58515)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Youssef1313
Youssef1313 authored Jan 4, 2022
1 parent c413257 commit 6671725
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 62 deletions.
9 changes: 8 additions & 1 deletion src/Compilers/CSharp/Portable/Compiler/MethodBodySynthesizer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ private static void MakeSubmissionInitialization(
}

/// <summary>
/// Construct a body for an auto-property accessor (updating or returning the backing field).
/// Construct a body for an auto-property accessor (updating or returning the backing field). This can return null on error scenarios.
/// </summary>
internal static BoundBlock ConstructAutoPropertyAccessorBody(SourceMemberMethodSymbol accessor)
{
Expand All @@ -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.GetMethod is null);
return null;
}

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
View file
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 { BodyShouldBeSynthesized: true })
{
return MethodBodySynthesizer.ConstructAutoPropertyAccessorBody(sourceMethod);
}
Expand Down
66 changes: 27 additions & 39 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs
View file
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 _bodyShouldBeSynthesized;
private readonly bool _isExpressionBodied;
private readonly bool _containsFieldKeyword;
private readonly bool _usesInit;
Expand All @@ -32,7 +32,7 @@ public static SourcePropertyAccessorSymbol CreateAccessorSymbol(
SourcePropertySymbol property,
DeclarationModifiers propertyModifiers,
AccessorDeclarationSyntax syntax,
bool isAutoPropertyAccessor,
bool bodyShouldBeSynthesizedForSemicolonOnly,
BindingDiagnosticBag diagnostics)
{
Debug.Assert(syntax.Kind() == SyntaxKind.GetAccessorDeclaration ||
Expand All @@ -42,7 +42,7 @@ public static SourcePropertyAccessorSymbol CreateAccessorSymbol(
bool isGetMethod = (syntax.Kind() == SyntaxKind.GetAccessorDeclaration);
var methodKind = isGetMethod ? MethodKind.PropertyGet : MethodKind.PropertySet;

bool hasBody = syntax.Body is object;
bool hasBlockBody = syntax.Body is object;
bool hasExpressionBody = syntax.ExpressionBody is object;
bool isNullableAnalysisEnabled = containingType.DeclaringCompilation.IsNullableAnalysisEnabledIn(syntax);
CheckForBlockAndExpressionBody(syntax.Body, syntax.ExpressionBody, syntax, diagnostics);
Expand All @@ -52,13 +52,13 @@ public static SourcePropertyAccessorSymbol CreateAccessorSymbol(
propertyModifiers,
syntax.Keyword.GetLocation(),
syntax,
hasBody,
hasBlockBody,
hasExpressionBody,
isIterator: SyntaxFacts.HasYieldOperations(syntax.Body),
syntax.Modifiers,
methodKind,
syntax.Keyword.IsKind(SyntaxKind.InitKeyword),
isAutoPropertyAccessor,
bodyShouldBeSynthesized: bodyShouldBeSynthesizedForSemicolonOnly && !hasBlockBody && !hasExpressionBody,
isNullableAnalysisEnabled: isNullableAnalysisEnabled,
diagnostics);
}
Expand Down Expand Up @@ -99,34 +99,17 @@ public static SourcePropertyAccessorSymbol CreateAccessorSymbol(
propertyModifiers,
location,
syntax,
hasBody: false,
hasBlockBody: false,
hasExpressionBody: false,
isIterator: false,
modifiers: new SyntaxTokenList(),
methodKind,
usesInit,
isAutoPropertyAccessor: true,
bodyShouldBeSynthesized: true,
isNullableAnalysisEnabled: false,
diagnostics);
}

public static SourcePropertyAccessorSymbol CreateAccessorSymbol(
NamedTypeSymbol containingType,
SynthesizedRecordEqualityContractProperty property,
DeclarationModifiers propertyModifiers,
Location location,
CSharpSyntaxNode syntax,
BindingDiagnosticBag diagnostics)
{
return new SynthesizedRecordEqualityContractProperty.GetAccessorSymbol(
containingType,
property,
propertyModifiers,
location,
syntax,
diagnostics);
}

// PROTOTYPE(semi-auto-props): Figure out what is going to be more efficient, to go after tokens and then
// checking their parent, or to go after nodes (IdentifierNameSyntax) first and then checking the underlying token.
// PROTOTYPE(semi-auto-props): Filter out identifiers that syntactically cannot be keywords. For example those that follow a ., a -> or a :: in names. Something else?
Expand Down Expand Up @@ -166,7 +149,7 @@ private SourcePropertyAccessorSymbol(
base(containingType, syntax.GetReference(), location, isIterator: false)
{
_property = property;
_isAutoPropertyAccessor = false;
_bodyShouldBeSynthesized = false;
_isExpressionBodied = true;
_containsFieldKeyword = property.IsIndexer ? false : NodeContainsFieldKeyword(syntax);

Expand All @@ -188,7 +171,7 @@ private SourcePropertyAccessorSymbol(
diagnostics.Add(info, location);
}

this.CheckModifiers(location, hasBody: true, isAutoPropertyOrExpressionBodied: true, diagnostics: diagnostics);
this.CheckModifiers(location, isBodyMissing: false, diagnostics: diagnostics);
}

#nullable enable
Expand All @@ -198,13 +181,13 @@ protected SourcePropertyAccessorSymbol(
DeclarationModifiers propertyModifiers,
Location location,
CSharpSyntaxNode syntax,
bool hasBody,
bool hasBlockBody,
bool hasExpressionBody,
bool isIterator,
SyntaxTokenList modifiers,
MethodKind methodKind,
bool usesInit,
bool isAutoPropertyAccessor,
bool bodyShouldBeSynthesized,
bool isNullableAnalysisEnabled,
BindingDiagnosticBag diagnostics)
: base(containingType,
Expand All @@ -213,18 +196,18 @@ protected SourcePropertyAccessorSymbol(
isIterator)
{
_property = property;
_isAutoPropertyAccessor = isAutoPropertyAccessor;
_bodyShouldBeSynthesized = bodyShouldBeSynthesized;
_containsFieldKeyword = property.IsIndexer ? false : NodeContainsFieldKeyword(getAccessorSyntax(syntax));
Debug.Assert(!_property.IsExpressionBodied, "Cannot have accessors in expression bodied lightweight properties");
_isExpressionBodied = !hasBody && hasExpressionBody;
_isExpressionBodied = !hasBlockBody && hasExpressionBody;
_usesInit = usesInit;
if (_usesInit)
{
Binder.CheckFeatureAvailability(syntax, MessageID.IDS_FeatureInitOnlySetters, diagnostics, location);
}

bool modifierErrors;
var declarationModifiers = this.MakeModifiers(modifiers, property.IsExplicitInterfaceImplementation, hasBody || hasExpressionBody, location, diagnostics, out modifierErrors);
var declarationModifiers = this.MakeModifiers(modifiers, property.IsExplicitInterfaceImplementation, hasBlockBody || hasExpressionBody, location, diagnostics, out modifierErrors);

// Include some modifiers from the containing property, but not the accessibility modifiers.
declarationModifiers |= GetAccessorModifiers(propertyModifiers) & ~DeclarationModifiers.AccessibilityMask;
Expand All @@ -239,9 +222,9 @@ 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: hasBlockBody || hasExpressionBody || _bodyShouldBeSynthesized, diagnostics);

if (hasBody || hasExpressionBody)
if (hasBlockBody || hasExpressionBody)
{
CheckModifiersForBody(location, diagnostics);
}
Expand All @@ -254,7 +237,7 @@ protected SourcePropertyAccessorSymbol(

if (!modifierErrors)
{
this.CheckModifiers(location, hasBody || hasExpressionBody, isAutoPropertyAccessor, diagnostics);
this.CheckModifiers(location, isBodyMissing: !hasBlockBody && !hasExpressionBody && !_bodyShouldBeSynthesized, diagnostics);
}

static CSharpSyntaxNode? getAccessorSyntax(CSharpSyntaxNode node)
Expand Down Expand Up @@ -478,6 +461,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 BodyShouldBeSynthesized => _bodyShouldBeSynthesized;

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

return ContainingType.IsStructType() &&
!_property.IsStatic &&
_isAutoPropertyAccessor &&
_bodyShouldBeSynthesized &&
MethodKind == MethodKind.PropertyGet;
}
}
Expand Down Expand Up @@ -564,7 +552,7 @@ private DeclarationModifiers MakeModifiers(SyntaxTokenList modifiers, bool isExp
return mods;
}

private void CheckModifiers(Location location, bool hasBody, bool isAutoPropertyOrExpressionBodied, BindingDiagnosticBag diagnostics)
private void CheckModifiers(Location location, bool isBodyMissing, BindingDiagnosticBag diagnostics)
{
// Check accessibility against the accessibility declared on the accessor not the property.
var localAccessibility = this.LocalAccessibility;
Expand All @@ -579,7 +567,7 @@ private void CheckModifiers(Location location, bool hasBody, bool isAutoProperty
// '{0}' is a new virtual member in sealed type '{1}'
diagnostics.Add(ErrorCode.ERR_NewVirtualInSealed, location, this, ContainingType);
}
else if (!hasBody && !IsExtern && !IsAbstract && !isAutoPropertyOrExpressionBodied)
else if (isBodyMissing && !IsExtern && !IsAbstract)
{
diagnostics.Add(ErrorCode.ERR_ConcreteMissingBody, location, this);
}
Expand All @@ -602,7 +590,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 && _bodyShouldBeSynthesized && MethodKind == MethodKind.PropertySet)
{
// Auto-implemented accessor '{0}' cannot be marked 'readonly'.
diagnostics.Add(ErrorCode.ERR_AutoSetterCantBeReadOnly, location, this);
Expand Down Expand Up @@ -835,7 +823,7 @@ internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, r
{
base.AddSynthesizedAttributes(moduleBuilder, ref attributes);

if (_isAutoPropertyAccessor)
if (_bodyShouldBeSynthesized)
{
var compilation = this.DeclaringCompilation;
AddSynthesizedAttribute(ref attributes, compilation.TrySynthesizeAttribute(WellKnownMember.System_Runtime_CompilerServices_CompilerGeneratedAttribute__ctor));
Expand Down
12 changes: 6 additions & 6 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs
View file
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(bool bodyShouldBeSynthesizedForSemicolonOnly, BindingDiagnosticBag diagnostics)
{
var syntax = (BasePropertyDeclarationSyntax)CSharpSyntaxNode;
ArrowExpressionClauseSyntax? arrowExpression = GetArrowExpression(syntax);
Expand All @@ -366,29 +366,29 @@ protected override SourcePropertyAccessorSymbol CreateGetAccessorSymbol(bool isA
}
else
{
return CreateAccessorSymbol(GetGetAccessorDeclaration(syntax), isAutoPropertyAccessor, diagnostics);
return CreateAccessorSymbol(GetGetAccessorDeclaration(syntax), bodyShouldBeSynthesizedForSemicolonOnly, diagnostics);
}
}

protected override SourcePropertyAccessorSymbol CreateSetAccessorSymbol(bool isAutoPropertyAccessor, BindingDiagnosticBag diagnostics)
protected override SourcePropertyAccessorSymbol CreateSetAccessorSymbol(bool bodyShouldBeSynthesizedForSemicolonOnly, 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), bodyShouldBeSynthesizedForSemicolonOnly, diagnostics);
}

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

Expand Down
8 changes: 4 additions & 4 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,11 @@ protected SourcePropertySymbolBase(

if (hasGetAccessor)
{
_getMethod = CreateGetAccessorSymbol(isAutoPropertyAccessor: isAutoProperty, diagnostics);
_getMethod = CreateGetAccessorSymbol(bodyShouldBeSynthesizedForSemicolonOnly: isAutoProperty, diagnostics);
}
if (hasSetAccessor)
{
_setMethod = CreateSetAccessorSymbol(isAutoPropertyAccessor: isAutoProperty, diagnostics);
_setMethod = CreateSetAccessorSymbol(bodyShouldBeSynthesizedForSemicolonOnly: isAutoProperty, diagnostics);
}
}

Expand Down Expand Up @@ -643,15 +643,15 @@ internal bool IsNew
/// The implementation may depend only on information available from the <see cref="SourcePropertySymbolBase"/> type.
/// </summary>
protected abstract SourcePropertyAccessorSymbol CreateGetAccessorSymbol(
bool isAutoPropertyAccessor,
bool bodyShouldBeSynthesizedForSemicolonOnly,
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,
bool bodyShouldBeSynthesizedForSemicolonOnly,
BindingDiagnosticBag diagnostics);

public sealed override MethodSymbol? GetMethod
Expand Down
12 changes: 6 additions & 6 deletions .../CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordEqualityContractProperty.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ public override SyntaxList<AttributeListSyntax> AttributeDeclarationSyntaxList
protected override Location TypeLocation
=> ContainingType.Locations[0];

protected override SourcePropertyAccessorSymbol CreateGetAccessorSymbol(bool isAutoPropertyAccessor, BindingDiagnosticBag diagnostics)
protected override SourcePropertyAccessorSymbol CreateGetAccessorSymbol(bool bodyShouldBeSynthesizedForSemicolonOnly, BindingDiagnosticBag diagnostics)
{
return SourcePropertyAccessorSymbol.CreateAccessorSymbol(
return new GetAccessorSymbol(
ContainingType,
this,
_modifiers,
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(bool bodyShouldBeSynthesizedForSemicolonOnly, BindingDiagnosticBag diagnostics)
{
throw ExceptionUtilities.Unreachable;
}
Expand Down Expand Up @@ -115,7 +115,7 @@ internal static void VerifyOverridesEqualityContractFromBase(PropertySymbol over
}
}

internal sealed class GetAccessorSymbol : SourcePropertyAccessorSymbol
private sealed class GetAccessorSymbol : SourcePropertyAccessorSymbol
{
internal GetAccessorSymbol(
NamedTypeSymbol containingType,
Expand All @@ -130,13 +130,13 @@ internal GetAccessorSymbol(
propertyModifiers,
location,
syntax,
hasBody: true,
hasBlockBody: true,
hasExpressionBody: false,
isIterator: false,
modifiers: new SyntaxTokenList(),
MethodKind.PropertyGet,
usesInit: false,
isAutoPropertyAccessor: false,
bodyShouldBeSynthesized: false,
isNullableAnalysisEnabled: false,
diagnostics)
{
Expand Down
Loading

0 comments on commit 6671725

Please sign in to comment.