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

[semi-auto-props]: Rename ContainsFieldKeyword, revise cases that should handle FieldKeywordBackingField #61310

Merged
merged 16 commits into from
May 24, 2022
Merged
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ private void CompileNamedType(NamedTypeSymbol containingType)
var member = members[memberOrdinal];

//When a filter is supplied, limit the compilation of members passing the filter.
if (member is not SourcePropertyAccessorSymbol { ContainsFieldKeyword: true } accessor ||
if (member is not SourcePropertyAccessorSymbol { ContainsFieldIdentifier: true } accessor ||
!PassesFilter(_filterOpt, member))
{
continue;
Expand Down Expand Up @@ -579,7 +579,7 @@ private void CompileNamedType(NamedTypeSymbol containingType)
Debug.Assert(!method.IsPartialDefinition());
}

if (member is SourcePropertyAccessorSymbol { ContainsFieldKeyword: true })
if (member is SourcePropertyAccessorSymbol { ContainsFieldIdentifier: true })
{
// We already compiled these accessors in the loop above.
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,12 @@ private static void GetDocumentsForMethodsAndNestedTypes(PooledHashSet<Cci.Debug

case SymbolKind.Property:
AddSymbolLocation(result, member);
FieldSymbol fieldKeywordBackingField = (member as SourcePropertySymbolBase)?.FieldKeywordBackingField;
if (fieldKeywordBackingField is not null)
{
AddSymbolLocation(result, fieldKeywordBackingField);
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
}

break;
case SymbolKind.Field:
if (member is TupleErrorFieldSymbol)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ private FieldSymbol GetActualField(Symbol member, NamedTypeSymbol type)
return (!eventSymbol.HasAssociatedField || ShouldIgnoreStructField(eventSymbol, eventSymbol.Type)) ? null : eventSymbol.AssociatedField.AsMember(type);
case SymbolKind.Property:
// PROTOTYPE(semi-auto-props): Review other event associated field callers and see if we have to do anything special for properties.
// Everything is reviewed except FlowAnalysis.
Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer this to be carefully reviewed to make sure I'm not missing anything, as we later will rely on the prototype update that only FlowAnalysis is left.

333fred marked this conversation as resolved.
Show resolved Hide resolved

// Backing field for semi auto props are not included in GetMembers.
if (member is SourcePropertySymbol property)
Expand Down
6 changes: 5 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/BaseTypeAnalysis.cs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ private static (bool definitelyManaged, bool hasGenerics) DependsOnDefinitelyMan
var hasGenerics = false;
if (partialClosure.Add(type))
{
foreach (var member in type.GetInstanceFieldsAndEvents())
foreach (var member in type.GetInstanceFieldsAndEventsAndProperties())
{
// Only instance fields (including field-like events) affect the outcome.
FieldSymbol field;
Expand All @@ -190,10 +190,14 @@ private static (bool definitelyManaged, bool hasGenerics) DependsOnDefinitelyMan
field = (FieldSymbol)member;
Debug.Assert((object)(field.AssociatedSymbol as EventSymbol) == null,
"Didn't expect to find a field-like event backing field in the member list.");
Debug.Assert(field is not SynthesizedBackingFieldSymbol { IsCreatedForFieldKeyword: true });
break;
case SymbolKind.Event:
field = ((EventSymbol)member).AssociatedField;
break;
case SymbolKind.Property:
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
field = (member as SourcePropertySymbolBase)?.FieldKeywordBackingField;
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
break;
default:
throw ExceptionUtilities.UnexpectedValue(member.Kind);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,7 @@ internal static bool IsFieldOrFieldLikeEvent(this Symbol member, out FieldSymbol
case SymbolKind.Event:
field = ((EventSymbol)member).AssociatedField;
return (object)field != null;
// PROTOTYPE(semi-auto-props): Revise if we need to do something for FieldKeywordBackingfield.
default:
field = null;
return false;
Expand Down
7 changes: 4 additions & 3 deletions src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -550,19 +550,20 @@ internal virtual ImmutableArray<Symbol> GetSimpleNonTypeMembers(string name)
/// For source symbols may be called while calculating
/// <see cref="NamespaceOrTypeSymbol.GetMembersUnordered"/>.
/// </remarks>
internal virtual IEnumerable<Symbol> GetInstanceFieldsAndEvents()
internal virtual IEnumerable<Symbol> GetInstanceFieldsAndEventsAndProperties()
{
return GetMembersUnordered().Where(IsInstanceFieldOrEvent);
return GetMembersUnordered().Where(IsInstanceFieldOrEventOrProperty);
}

protected static Func<Symbol, bool> IsInstanceFieldOrEvent = symbol =>
protected static Func<Symbol, bool> IsInstanceFieldOrEventOrProperty = symbol =>
{
if (!symbol.IsStatic)
{
switch (symbol.Kind)
{
case SymbolKind.Field:
case SymbolKind.Event:
case SymbolKind.Property:
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1597,10 +1597,10 @@ private Dictionary<string, ImmutableArray<Symbol>> GetMembersByNameSlow()
return _lazyMembersDictionary;
}

internal override IEnumerable<Symbol> GetInstanceFieldsAndEvents()
internal override IEnumerable<Symbol> GetInstanceFieldsAndEventsAndProperties()
{
var membersAndInitializers = this.GetMembersAndInitializers();
return membersAndInitializers.NonTypeMembers.Where(IsInstanceFieldOrEvent);
return membersAndInitializers.NonTypeMembers.Where(IsInstanceFieldOrEventOrProperty);
}

protected void AfterMembersChecks(BindingDiagnosticBag diagnostics)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ internal class SourcePropertyAccessorSymbol : SourceMemberMethodSymbol
private string _lazyName;
private readonly bool _bodyShouldBeSynthesized;
private readonly bool _isExpressionBodied;
private readonly bool _containsFieldKeyword;
private readonly bool _containsFieldIdentifier;
private readonly bool _usesInit;

public static SourcePropertyAccessorSymbol CreateAccessorSymbol(
Expand Down Expand Up @@ -113,7 +113,7 @@ public static SourcePropertyAccessorSymbol CreateAccessorSymbol(
// 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?
private static bool NodeContainsFieldKeyword(CSharpSyntaxNode? node)
private static bool NodeContainsFieldIdentifier(CSharpSyntaxNode? node)
{
if (node is null)
{
Expand Down Expand Up @@ -151,7 +151,7 @@ private SourcePropertyAccessorSymbol(
_property = property;
_bodyShouldBeSynthesized = false;
_isExpressionBodied = true;
_containsFieldKeyword = property.IsIndexer ? false : NodeContainsFieldKeyword(syntax);
_containsFieldIdentifier = !property.IsIndexer && NodeContainsFieldIdentifier(syntax);

// The modifiers for the accessor are the same as the modifiers for the property,
// minus the indexer and readonly bit
Expand Down Expand Up @@ -197,7 +197,7 @@ protected SourcePropertyAccessorSymbol(
{
_property = property;
_bodyShouldBeSynthesized = bodyShouldBeSynthesized;
_containsFieldKeyword = property.IsIndexer ? false : NodeContainsFieldKeyword(getAccessorSyntax(syntax));
_containsFieldIdentifier = !property.IsIndexer && NodeContainsFieldIdentifier(getAccessorSyntax(syntax));
Debug.Assert(!_property.IsExpressionBodied, "Cannot have accessors in expression bodied lightweight properties");
_isExpressionBodied = !hasBlockBody && hasExpressionBody;
_usesInit = usesInit;
Expand Down Expand Up @@ -459,8 +459,7 @@ internal Accessibility LocalAccessibility
/// This is only calculated from syntax, so we don't know if it
/// will bind to something or will create a backing field.
/// </remarks>
// PROTOTYPE(semi-auto-props): Rename to ContainsFieldKeywordSyntactically or similar.
internal bool ContainsFieldKeyword => _containsFieldKeyword;
internal bool ContainsFieldIdentifier => _containsFieldIdentifier;

/// <summary>
/// Indicates whether this accessor has no body in source and a body will be synthesized by the compiler.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ private void EnsureBackingFieldIsSynthesized()

if (this is SourcePropertySymbol propertySymbol)
{
if (propertySymbol.GetMethod is SourcePropertyAccessorSymbol { ContainsFieldKeyword: true } getMethod)
if (propertySymbol.GetMethod is SourcePropertyAccessorSymbol { ContainsFieldIdentifier: true } getMethod)
{
noteAccessorBinding();
var binder = getMethod.TryGetBodyBinder();
Expand All @@ -456,7 +456,7 @@ private void EnsureBackingFieldIsSynthesized()

// If we still don't have a backing field after binding the getter, try binding the setter.
if (_lazyBackingFieldSymbol == _lazyBackingFieldSymbolSentinel &&
propertySymbol.SetMethod is SourcePropertyAccessorSymbol { ContainsFieldKeyword: true } setMethod)
propertySymbol.SetMethod is SourcePropertyAccessorSymbol { ContainsFieldIdentifier: true } setMethod)
{
noteAccessorBinding();
setMethod.TryGetBodyBinder()?.BindMethodBody(setMethod.SyntaxNode, BindingDiagnosticBag.Discarded);
Expand Down Expand Up @@ -813,7 +813,7 @@ private bool AllowFieldAttributeTarget
{
get
{
// PROTOTYPE(semi-auto-props): Fix implementation for semi auto properties.
// PROTOTYPE(semi-auto-props): Fix implementation for semi auto properties. Consider also the BackingField?.GetAttributes() call in GetAttributesBag. Should it handle FieldKeywordBackingField? Add a test.
return _getMethod?.BodyShouldBeSynthesized == true ||
_setMethod?.BodyShouldBeSynthesized == true;
}
Expand Down Expand Up @@ -1626,6 +1626,7 @@ internal override void ForceComplete(SourceLocation locationOpt, CancellationTok
{
var diagnostics = BindingDiagnosticBag.GetInstance();
var conversions = new TypeConversions(this.ContainingAssembly.CorLibrary);

foreach (var parameter in this.Parameters)
{
parameter.ForceComplete(locationOpt, cancellationToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1081,6 +1081,7 @@ SmallDictionary<Symbol, Symbol> computeDefinitionToMemberMap()

case SymbolKind.Event:
var underlyingEvent = (EventSymbol)member;
// PROTOTYPE(semi-auto-props): Do we need to do something for properties with FieldKeywordBackingField?
var underlyingAssociatedField = underlyingEvent.AssociatedField;
// The field is not part of the members list
if (underlyingAssociatedField is object)
Expand Down
28 changes: 28 additions & 0 deletions src/Compilers/CSharp/Test/Emit/PDB/PDBWinMdExpTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,34 @@ public int this[int a]
AssertXml.Equal(expected, actual);
}

[ConditionalFact(typeof(WindowsOnly), Reason = ConditionalSkipReason.NativePdbRequiresDesktop)]
public void TestWinMdExpData_SemiAutoProperty()
{
var text = @"
public class C
{
public int P { get => field; }
}
";

string expected = @"<?xml version=""1.0"" encoding=""utf-16""?>
<token-map>
<token-location token=""0x02xxxxxx"" file=""source.cs"" start-line=""2"" start-column=""14"" end-line=""2"" end-column=""15""/>
<token-location token=""0x06xxxxxx"" file=""source.cs"" start-line=""2"" start-column=""14"" end-line=""2"" end-column=""15""/>
<token-location token=""0x04xxxxxx"" file=""source.cs"" start-line=""4"" start-column=""16"" end-line=""4"" end-column=""17""/>
<token-location token=""0x17xxxxxx"" file=""source.cs"" start-line=""4"" start-column=""16"" end-line=""4"" end-column=""17""/>
<token-location token=""0x06xxxxxx"" file=""source.cs"" start-line=""4"" start-column=""20"" end-line=""4"" end-column=""23""/>
</token-map>";

var compilation = CreateCompilationWithMscorlib45(
text,
options: TestOptions.ReleaseWinMD,
sourceFileName: "source.cs").VerifyDiagnostics();

string actual = PdbTestUtilities.GetTokenToLocationMap(compilation, true);
AssertXml.Equal(expected, actual);
}

[ConditionalFact(typeof(WindowsOnly), Reason = ConditionalSkipReason.NativePdbRequiresDesktop)]
public void TestWinMdExpData_AnonymousTypes()
{
Expand Down
Loading