Merge pull request #587 from dpaoliello/deeplynested

Fixes name generation for nested anonymous types

Author: tannergooding

sources/ClangSharp.PInvokeGenerator/PInvokeGenerator.VisitDecl.cs

@@ -865,7 +865,7 @@ private void VisitIndirectFieldDecl(IndirectFieldDecl indirectFieldDecl)
             return;
         }
 
-        if (IsPrevContextDecl<RecordDecl>(out var prevContext, out _) && prevContext.IsAnonymousStructOrUnion)
+        if (IsPrevContextDecl<RecordDecl>(out var prevContext, out _) && prevContext.IsAnonymous)
         {
             // We shouldn't process indirect fields where the prev context is an anonymous record decl
             return;
@@ -880,28 +880,15 @@ private void VisitIndirectFieldDecl(IndirectFieldDecl indirectFieldDecl)
         var contextNameParts = new Stack<string>();
         var contextTypeParts = new Stack<string>();
 
-        while (rootRecordDecl.IsAnonymousStructOrUnion && (rootRecordDecl.Parent is RecordDecl parentRecordDecl))
+        while (rootRecordDecl.IsAnonymous && (rootRecordDecl.Parent is RecordDecl parentRecordDecl))
         {
+            // The name of a field of an anonymous type should be same as the type's name minus the
+            // type kind tag at the end and the leading `_`.
             var contextNamePart = GetRemappedCursorName(rootRecordDecl);
-
-            if (contextNamePart.StartsWith('_'))
-            {
-                var suffixLength = 0;
-
-                if (contextNamePart.EndsWith("_e__Union", StringComparison.Ordinal))
-                {
-                    suffixLength = 10;
-                }
-                else if (contextNamePart.EndsWith("_e__Struct", StringComparison.Ordinal))
-                {
-                    suffixLength = 11;
-                }
-
-                if (suffixLength != 0)
-                {
-                    contextNamePart = contextNamePart.Substring(1, contextNamePart.Length - suffixLength);
-                }
-            }
+            var tagIndex = contextNamePart.LastIndexOf("_e__", StringComparison.Ordinal);
+            Debug.Assert(contextNamePart[0] == '_');
+            Debug.Assert(tagIndex >= 0);
+            contextNamePart = contextNamePart.Substring(1, tagIndex - 1);
 
             contextNameParts.Push(EscapeName(contextNamePart));
 
@@ -1397,7 +1384,7 @@ private void VisitRecordDecl(RecordDecl recordDecl)
             var maxAlignm = recordDecl.Fields.Any() ? recordDecl.Fields.Max((fieldDecl) => Math.Max(fieldDecl.Type.Handle.AlignOf, 1)) : alignment;
 
             var isTopLevelStruct = _config.WithTypes.TryGetValue(name, out var withType) && withType.Equals("struct", StringComparison.Ordinal);
-            var generateTestsClass = !recordDecl.IsAnonymousStructOrUnion && recordDecl.DeclContext is not RecordDecl;
+            var generateTestsClass = !recordDecl.IsAnonymous && recordDecl.DeclContext is not RecordDecl;
             var testOutputStarted = false;
 
             var nullableUuid = (Guid?)null;

sources/ClangSharp.PInvokeGenerator/PInvokeGenerator.cs

@@ -3166,9 +3166,19 @@ private string GetRemappedCursorName(NamedDecl namedDecl, out string nativeTypeN
         {
             remappedName = "Dispose";
         }
-        else if (namedDecl is FieldDecl fieldDecl)
+        else if ((namedDecl is FieldDecl fieldDecl) && name.StartsWith("__AnonymousFieldDecl_", StringComparison.Ordinal))
         {
-            if (name.StartsWith("__AnonymousFieldDecl_", StringComparison.Ordinal))
+            if (fieldDecl.Type.AsCXXRecordDecl?.IsAnonymous == true)
+            {
+                // For fields of anonymous types, use the name of the type but clean off the type
+                // kind tag at the end.
+                var typeName = GetRemappedNameForAnonymousRecord(fieldDecl.Type.AsCXXRecordDecl);
+                var tagIndex = typeName.LastIndexOf("_e__", StringComparison.Ordinal);
+                Debug.Assert(typeName[0] == '_');
+                Debug.Assert(tagIndex >= 0);
+                remappedName = typeName.Substring(1, tagIndex - 1);
+            }
+            else
             {
                 remappedName = "Anonymous";
 
@@ -3184,28 +3194,62 @@ private string GetRemappedCursorName(NamedDecl namedDecl, out string nativeTypeN
         }
         else if ((namedDecl is RecordDecl recordDecl) && name.StartsWith("__AnonymousRecord_", StringComparison.Ordinal))
         {
-            if (recordDecl.Parent is RecordDecl parentRecordDecl)
-            {
-                remappedName = "_Anonymous";
+            remappedName = GetRemappedNameForAnonymousRecord(recordDecl);
+        }
 
-                var matchingField = parentRecordDecl.Fields.Where((fieldDecl) => fieldDecl.Type.CanonicalType == recordDecl.TypeForDecl.CanonicalType).FirstOrDefault();
+        return remappedName;
+    }
 
-                if (matchingField is not null)
-                {
-                    remappedName = "_";
-                    remappedName += GetRemappedCursorName(matchingField);
-                }
-                else if (parentRecordDecl.AnonymousRecords.Count > 1)
+    private string GetRemappedNameForAnonymousRecord(RecordDecl recordDecl)
+    {
+        if (recordDecl.Parent is RecordDecl parentRecordDecl)
+        {
+            var remappedNameBuilder = new StringBuilder();
+            var matchingField = parentRecordDecl.Fields.Where((fieldDecl) => fieldDecl.Type.CanonicalType == recordDecl.TypeForDecl.CanonicalType).FirstOrDefault();
+
+            if ((matchingField is not null) && !matchingField.IsAnonymousField)
+            {
+                _ = remappedNameBuilder.Append('_');
+                _ = remappedNameBuilder.Append(GetRemappedCursorName(matchingField));
+            }
+            else
+            {
+                _ = remappedNameBuilder.Append("_Anonymous");
+
+                // If there is more than one anonymous type, then add a numeral to differentiate.
+                if (parentRecordDecl.AnonymousRecords.Count > 1)
                 {
                     var index = parentRecordDecl.AnonymousRecords.IndexOf(recordDecl) + 1;
-                    remappedName += index.ToString(CultureInfo.InvariantCulture);
+                    Debug.Assert(index > 0);
+                    _ = remappedNameBuilder.Append(index);
                 }
 
-                remappedName += $"_e__{(recordDecl.IsUnion ? "Union" : "Struct")}";
+                // C# doesn't allow a nested type to have the same name as the parent, so if the
+                // parent is also anonymous, add the nesting depth as a way to avoid conflicts with
+                // the parent's name.
+                if (parentRecordDecl.IsAnonymous)
+                {
+                    var depth = 1;
+                    var currentParent = parentRecordDecl.Parent;
+                    while ((currentParent is RecordDecl currentParentRecordDecl) && currentParentRecordDecl.IsAnonymous)
+                    {
+                        depth++;
+                        currentParent = currentParentRecordDecl.Parent;
+                    }
+                    _ = remappedNameBuilder.Append('_');
+                    _ = remappedNameBuilder.Append(depth);
+                }
             }
-        }
 
-        return remappedName;
+            // Add the type kind tag.
+            _ = remappedNameBuilder.Append("_e__");
+            _ = remappedNameBuilder.Append(recordDecl.IsUnion ? "Union" : "Struct");
+            return remappedNameBuilder.ToString();
+        }
+        else
+        {
+            return $"_Anonymous_e__{(recordDecl.IsUnion ? "Union" : "Struct")}";
+        }
     }
 
     private string GetRemappedName(string name, Cursor? cursor, bool tryRemapOperatorName, out bool wasRemapped, bool skipUsing = false)
@@ -3304,48 +3348,7 @@ private string GetRemappedTypeName(Cursor? cursor, Cursor? context, Type type, o
                     if (IsType<RecordType>(cursor, type, out var recordType) && remappedName.StartsWith("__AnonymousRecord_", StringComparison.Ordinal))
                     {
                         var recordDecl = recordType.Decl;
-                        remappedName = "_Anonymous";
-
-                        if (recordDecl.Parent is RecordDecl parentRecordDecl)
-                        {
-                            var matchingField = parentRecordDecl.Fields.Where((fieldDecl) => fieldDecl.Type.CanonicalType == recordType).FirstOrDefault();
-
-                            if (matchingField is not null)
-                            {
-                                remappedName = "_";
-                                remappedName += GetRemappedCursorName(matchingField);
-                            }
-                            else
-                            {
-                                var index = 0;
-
-                                if (parentRecordDecl.AnonymousRecords.Count > 1)
-                                {
-                                    index = parentRecordDecl.AnonymousRecords.IndexOf(cursor) + 1;
-                                }
-
-                                while (parentRecordDecl.IsAnonymousStructOrUnion && (parentRecordDecl.IsUnion == recordType.Decl.IsUnion))
-                                {
-                                    index += 1;
-
-                                    if (parentRecordDecl.Parent is RecordDecl parentRecordDeclParent)
-                                    {
-                                        if (parentRecordDeclParent.AnonymousRecords.Count > 0)
-                                        {
-                                            index += parentRecordDeclParent.AnonymousRecords.Count - 1;
-                                        }
-                                        parentRecordDecl = parentRecordDeclParent;
-                                    }
-                                }
-
-                                if (index != 0)
-                                {
-                                    remappedName += index.ToString(CultureInfo.InvariantCulture);
-                                }
-                            }
-                        }
-
-                        remappedName += $"_e__{(recordDecl.IsUnion ? "Union" : "Struct")}";
+                        remappedName = GetRemappedNameForAnonymousRecord(recordDecl);
                     }
                     else if (IsType<EnumType>(cursor, type, out var enumType) && remappedName.StartsWith("__AnonymousEnum_", StringComparison.Ordinal))
                     {
@@ -4572,7 +4575,7 @@ private bool HasBaseField(CXXRecordDecl cxxRecordDecl)
 
     private bool HasField(RecordDecl recordDecl)
     {
-        var hasField = recordDecl.Fields.Any() || recordDecl.Decls.Any((decl) => (decl is RecordDecl nestedRecordDecl) && nestedRecordDecl.IsAnonymousStructOrUnion && HasField(nestedRecordDecl));
+        var hasField = recordDecl.Fields.Any() || recordDecl.Decls.Any((decl) => (decl is RecordDecl nestedRecordDecl) && nestedRecordDecl.IsAnonymous && HasField(nestedRecordDecl));
 
         if (!hasField && (recordDecl is CXXRecordDecl cxxRecordDecl))
         {
@@ -5123,7 +5126,7 @@ bool IsEmptyRecord(RecordDecl recordDecl)
 
             foreach (var decl in recordDecl.Decls)
             {
-                if ((decl is RecordDecl nestedRecordDecl) && nestedRecordDecl.IsAnonymousStructOrUnion && !IsEmptyRecord(nestedRecordDecl))
+                if ((decl is RecordDecl nestedRecordDecl) && nestedRecordDecl.IsAnonymous && !IsEmptyRecord(nestedRecordDecl))
                 {
                     return false;
                 }
@@ -6150,7 +6153,7 @@ private bool IsUnsafe(RecordDecl recordDecl)
             {
                 return true;
             }
-            else if ((decl is RecordDecl nestedRecordDecl) && nestedRecordDecl.IsAnonymousStructOrUnion && (IsUnsafe(nestedRecordDecl) || Config.GenerateCompatibleCode))
+            else if ((decl is RecordDecl nestedRecordDecl) && nestedRecordDecl.IsAnonymous && (IsUnsafe(nestedRecordDecl) || Config.GenerateCompatibleCode))
             {
                 return true;
             }

sources/ClangSharp/Cursors/Decls/RecordDecl.cs

@@ -48,12 +48,12 @@ private protected RecordDecl(CXCursor handle, CXCursorKind expectedCursorKind, C
         });
 
         _anonymousFields = new Lazy<IReadOnlyList<FieldDecl>>(() => Decls.OfType<FieldDecl>().Where(decl => decl.IsAnonymousField).ToList());
-        _anonymousRecords = new Lazy<IReadOnlyList<RecordDecl>>(() => Decls.OfType<RecordDecl>().Where(decl => decl.IsAnonymousStructOrUnion && !decl.IsInjectedClassName).ToList());
+        _anonymousRecords = new Lazy<IReadOnlyList<RecordDecl>>(() => Decls.OfType<RecordDecl>().Where(decl => decl.IsAnonymous && !decl.IsInjectedClassName).ToList());
         _indirectFields = new Lazy<IReadOnlyList<IndirectFieldDecl>>(() => Decls.OfType<IndirectFieldDecl>().ToList());
         _injectedClassName = new Lazy<RecordDecl?>(() => Decls.OfType<RecordDecl>().Where(decl => decl.IsInjectedClassName).SingleOrDefault());
     }
 
-    public bool IsAnonymousStructOrUnion => Handle.IsAnonymousStructOrUnion;
+    public bool IsAnonymous => Handle.IsAnonymous;
 
     public IReadOnlyList<FieldDecl> AnonymousFields => _anonymousFields.Value;
 

tests/ClangSharp.PInvokeGenerator.UnitTests/Base/StructDeclarationTest.cs

@@ -215,6 +215,21 @@ public abstract class StructDeclarationTest : PInvokeGeneratorTest
     [Test]
     public Task SourceLocationAttributeTest() => SourceLocationAttributeTestImpl();
 
+    // Regression test: the code that generated the name for anonymous types had a couple of
+    // bugs:
+    // * It would only append a numeral if the parent had more than one anonymous records,
+    //   but an anonymous typed array didn't count as a record resulting in multiple structs
+    //   named `_Anonymous_e__Struct`.
+    // * The code that remapped the name of anonymous types was different between the code that
+    //   generated the type definition and the code that defined fields in a FixedBuffer.
+    [Test]
+    public Task AnonStructAndAnonStructArray() => AnonStructAndAnonStructArrayImpl();
+
+    // Regression test: nested anonymous structs would result in:
+    // error CS0542: '_Anonymous_e__Struct': member names cannot be the same as their enclosing type
+    [Test]
+    public Task DeeplyNestedAnonStructs() => DeeplyNestedAnonStructsImpl();
+
     protected abstract Task IncompleteArraySizeTestImpl(string nativeType, string expectedManagedType);
 
     protected abstract Task BasicTestImpl(string nativeType, string expectedManagedType);
@@ -290,4 +305,8 @@ public abstract class StructDeclarationTest : PInvokeGeneratorTest
     protected abstract Task WithPackingTestImpl();
 
     protected abstract Task SourceLocationAttributeTestImpl();
+
+    protected abstract Task AnonStructAndAnonStructArrayImpl();
+
+    protected abstract Task DeeplyNestedAnonStructsImpl();
 }