[semi-auto-props]: Rename ContainsFieldKeyword, revise cases that sho…

…uld handle FieldKeywordBackingField (#61310)
dotnet · May 24, 2022 · b04c5c3 · b04c5c3
1 parent d04d096
commit b04c5c3
Show file tree

Hide file tree

Showing 12 changed files with 415 additions and 21 deletions.
diff --git a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
@@ -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;
@@ -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;

diff --git a/src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs b/src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs
@@ -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);
+                                        }
+
                                         break;
                                     case SymbolKind.Field:
                                         if (member is TupleErrorFieldSymbol)

diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/EmptyStructTypeCache.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/EmptyStructTypeCache.cs
@@ -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.
 
                     // Backing field for semi auto props are not included in GetMembers.
                     if (member is SourcePropertySymbol property)

diff --git a/src/Compilers/CSharp/Portable/Symbols/BaseTypeAnalysis.cs b/src/Compilers/CSharp/Portable/Symbols/BaseTypeAnalysis.cs
@@ -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;
@@ -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:
+                            field = (member as SourcePropertySymbolBase)?.FieldKeywordBackingField;
+                            break;
                         default:
                             throw ExceptionUtilities.UnexpectedValue(member.Kind);
                     }

diff --git a/src/Compilers/CSharp/Portable/Symbols/MemberSymbolExtensions.cs b/src/Compilers/CSharp/Portable/Symbols/MemberSymbolExtensions.cs
@@ -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;

diff --git a/src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs
@@ -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;
                 }
             }

diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs
@@ -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)

diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertyAccessorSymbol.cs
@@ -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(
@@ -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)
             {
@@ -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
@@ -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;
@@ -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.

diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs
@@ -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();
@@ -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);
@@ -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;
             }
@@ -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);

diff --git a/src/Compilers/CSharp/Portable/Symbols/Tuples/TupleTypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Tuples/TupleTypeSymbol.cs
@@ -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)

diff --git a/src/Compilers/CSharp/Test/Emit/PDB/PDBWinMdExpTests.cs b/src/Compilers/CSharp/Test/Emit/PDB/PDBWinMdExpTests.cs
@@ -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()
         {