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

Handle various cases for field-backed properties #74641

Merged
merged 19 commits into from
Aug 12, 2024
Merged

Handle various cases for field-backed properties #74641

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
d5595b3
Support attributes targeting synthesized backing field
cston Aug 2, 2024
0c437a6
Update test
cston Aug 3, 2024
14072de
Ref like types
cston Aug 3, 2024
46ab842
Generate implicit accessor bodies
cston Aug 3, 2024
879c246
Remove IsAutoPropertyWithGetAccessor
cston Aug 4, 2024
1fce652
Cleanup
cston Aug 4, 2024
fcc6b42
Allow mixing auto- and implemented accessors
cston Aug 4, 2024
2df0393
Revert
cston Aug 4, 2024
04d5b98
Fix tests
cston Aug 4, 2024
7faf125
Address feedback
cston Aug 8, 2024
1b37433
Fix tests
cston Aug 8, 2024
e1b3b1f
Move checks
cston Aug 9, 2024
cf7bf22
Check language version
cston Aug 9, 2024
ee34eca
Check members
cston Aug 9, 2024
32e30da
Fix test
cston Aug 9, 2024
5547645
Combine feature checks
cston Aug 11, 2024
d0cc45f
Fix tests
cston Aug 11, 2024
5bffcfb
Cleanup
cston Aug 11, 2024
8102cfc
Fix language version check
cston Aug 12, 2024
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
18 changes: 11 additions & 7 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ private static SourcePropertySymbol Create(
out var getSyntax,
out var setSyntax);

bool accessorsHaveImplementation = hasGetAccessorImplementation || hasSetAccessorImplementation; // PROTOTYPE: Remove if not needed.
bool accessorsHaveImplementation = hasGetAccessorImplementation || hasSetAccessorImplementation;

var explicitInterfaceSpecifier = GetExplicitInterfaceSpecifier(syntax);
SyntaxTokenList modifiersTokenList = GetModifierTokensSyntax(syntax);
Expand All @@ -63,8 +63,10 @@ private static SourcePropertySymbol Create(
diagnostics,
out _);

bool hasAutoPropertyGet = (modifiers & DeclarationModifiers.Partial) == 0 && getSyntax != null && !hasGetAccessorImplementation;
bool hasAutoPropertySet = (modifiers & DeclarationModifiers.Partial) == 0 && setSyntax != null && !hasSetAccessorImplementation;
bool allowAutoPropertyAccessors = (modifiers & (DeclarationModifiers.Partial | DeclarationModifiers.Abstract | DeclarationModifiers.Extern | DeclarationModifiers.Indexer)) == 0 &&
(!containingType.IsInterface || (modifiers & DeclarationModifiers.Static) != 0);
bool hasAutoPropertyGet = allowAutoPropertyAccessors && getSyntax != null && !hasGetAccessorImplementation;
bool hasAutoPropertySet = allowAutoPropertyAccessors && setSyntax != null && !hasSetAccessorImplementation;

binder = binder.SetOrClearUnsafeRegionIfNecessary(modifiersTokenList);
TypeSymbol? explicitInterfaceType;
Expand Down Expand Up @@ -140,7 +142,9 @@ private SourcePropertySymbol(
{
Binder.CheckFeatureAvailability(
syntax,
(hasGetAccessor && !hasSetAccessor) ? MessageID.IDS_FeatureReadonlyAutoImplementedProperties : MessageID.IDS_FeatureAutoImplementedProperties,
hasGetAccessor && hasSetAccessor ?
Copy link
Contributor

@RikkiGibson RikkiGibson Aug 12, 2024 •
edited by cston
Loading

Choose a reason for hiding this comment

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

nit: I think it would be easier to grasp what scenario we are in, in each branch, if the above IsAutoProperty were replaced with hasAutoPropertyGet || hasAutoPropertySet. #Resolved

(hasAutoPropertyGet && hasAutoPropertySet ? MessageID.IDS_FeatureAutoImplementedProperties : MessageID.IDS_FeatureFieldKeyword) :
MessageID.IDS_FeatureReadonlyAutoImplementedProperties,
Copy link
Contributor

@RikkiGibson RikkiGibson Aug 12, 2024 •
edited by cston
Loading

Choose a reason for hiding this comment

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

nit: It seems like the new factoring is going to diagnose a setter-only auto property (error case) as requiring the readonly auto-properties feature. #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, thanks.

diagnostics,
location);
}
Expand Down Expand Up @@ -213,8 +217,8 @@ private static void GetAccessorDeclarations(
out bool hasSetAccessorImplementation,
out bool usesFieldKeyword,
out bool isInitOnly,
out CSharpSyntaxNode? getSyntax,
out CSharpSyntaxNode? setSyntax)
out AccessorDeclarationSyntax? getSyntax,
out AccessorDeclarationSyntax? setSyntax)
{
var syntax = (BasePropertyDeclarationSyntax)syntaxNode;
isExpressionBodied = syntax.AccessorList is null;
Expand Down Expand Up @@ -335,7 +339,7 @@ private static (DeclarationModifiers modifiers, bool hasExplicitAccessMod) MakeM
SyntaxTokenList modifiers,
bool isExplicitInterfaceImplementation,
bool isIndexer,
bool accessorsHaveImplementation, // PROTOTYPE: How is this used?
bool accessorsHaveImplementation,
Location location,
BindingDiagnosticBag diagnostics,
out bool modifierErrors)
Expand Down
12 changes: 1 addition & 11 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,16 +115,6 @@ protected SourcePropertySymbolBase(
_lazyExplicitInterfaceImplementations = ImmutableArray<PropertySymbol>.Empty;
}

bool isIndexer = IsIndexer;
if ((!hasGetAccessor || hasAutoPropertyGet) && (!hasSetAccessor || hasAutoPropertySet))
{
if (!(!(containingType.IsInterface && !IsStatic) && !IsAbstract && !IsExtern && !isIndexer))
{
hasAutoPropertyGet = false;
hasAutoPropertySet = false;
}
}

if (hasExplicitAccessMod)
{
_propertyFlags |= Flags.HasExplicitAccessModifier;
Expand Down Expand Up @@ -160,7 +150,7 @@ protected SourcePropertySymbolBase(
_propertyFlags |= Flags.AccessorsHaveImplementation;
}

if (isIndexer)
if (IsIndexer)
{
if (indexerNameAttributeLists.Count == 0 || isExplicitInterfaceImplementation)
{
Expand Down
Loading