Finish removing backing fields from GetMembers #44932
Conversation
| @@ -90,7 +90,7 @@ private static void StructDependsClosure(NamedTypeSymbol type, HashSet<Symbol> p | |||
| { | |||
| foreach (var member in type.GetMembersUnordered()) | |||
| { | |||
| var field = member as FieldSymbol; | |||
| var field = member as FieldSymbol ?? (member as PropertySymbol)?.AssociatedField; | |||
There was a problem hiding this comment.
Why weren't event backing fields already considered here?
| @@ -17,6 +17,8 @@ public SourceOrRecordPropertySymbol(Location location) | |||
|
|
|||
| internal abstract SynthesizedBackingFieldSymbol BackingField { get; } | |||
|
|
|||
| internal sealed override FieldSymbol AssociatedField => BackingField; | |||
There was a problem hiding this comment.
A new BackingField property was added in master in the meantime. Perhaps IPropertySymbol.BackingField should be the name instead of AssociatedField?
| // TODO: event backing fields | ||
| return member as IFieldSymbol ?? (member as IPropertySymbol)?.AssociatedField; |
There was a problem hiding this comment.
It looks like there's a test hole here as well that would require IEventSymbol.AssociatedField to be exposed.
| // TODO: This probably indicates a problem because event backing fields have not been in GetMembers() for a | ||
| // long time. Compare to the changes that were necessary in PropertySymbolReferenceFinder. |
There was a problem hiding this comment.
I think there is an existing 'find all references' test hole here that would require IEventSymbol.AssociatedField to be exposed.
| // TODO: Events? | ||
| #if CODE_STYLE | ||
| var fieldSymbol = member as IFieldSymbol; | ||
| #else | ||
| var fieldSymbol = member as IFieldSymbol ?? (member as IPropertySymbol)?.AssociatedField; | ||
| #endif |
There was a problem hiding this comment.
It looks like there's an existing test hole again that would require IEventSymbol.AssociatedField to be exposed.
jnm2
force-pushed
the
finish_removing_backing_fields_from_getmembers
branch
from
b51cc92 to
a4665c3
Compare
…just like event backing fields
…ent features now that the field is not in GetMembers()
jnm2
force-pushed
the
finish_removing_backing_fields_from_getmembers
branch
from
a4665c3 to
a44d7f8
Compare
|
Can you rerun the failing CI jobs? There is a single VB test in Microsoft.VisualStudio.LanguageServices failing in only one of the runs and it doesn't repro locally on x86 or x64. |
jinujoseph
added
Area-Compilers
Community
|
@CyrusNajmabadi asked me to split this PR and just add the API first since removing backing fields from GetMembers() is a behavioral breaking change. Take a look at a44d7f8 for a clear view of the impact of the change. |
Fixes #42355.
This is also an essential building block in the prototype for the
fieldbacking field access keyword being discussed for potential inclusion in a future version of C#. master...jnm2:prototypes/backing_field_accessFor previous context, also see #40572 (comment). This is work that the Roslyn team had wanted to do at some point.
Asked for by @CyrusNajmabadi