Skip to content

Commit 78ee577

Browse files
dpaoliellodpaoliello
committed
Fix name generation for nested types
1 parent be0b852 commit 78ee577

37 files changed

+2455
-502
lines changed

‎sources/ClangSharp.PInvokeGenerator/PInvokeGenerator.VisitDecl.cs‎

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -865,7 +865,7 @@ private void VisitIndirectFieldDecl(IndirectFieldDecl indirectFieldDecl)
865865
return;
866866
}
867867

868-
if (IsPrevContextDecl<RecordDecl>(out var prevContext, out _) && prevContext.IsAnonymousStructOrUnion)
868+
if (IsPrevContextDecl<RecordDecl>(out var prevContext, out _) && prevContext.IsAnonymous)
869869
{
870870
// We shouldn't process indirect fields where the prev context is an anonymous record decl
871871
return;
@@ -880,28 +880,15 @@ private void VisitIndirectFieldDecl(IndirectFieldDecl indirectFieldDecl)
880880
var contextNameParts = new Stack<string>();
881881
var contextTypeParts = new Stack<string>();
882882

883-
while (rootRecordDecl.IsAnonymousStructOrUnion && (rootRecordDecl.Parent is RecordDecl parentRecordDecl))
883+
while (rootRecordDecl.IsAnonymous && (rootRecordDecl.Parent is RecordDecl parentRecordDecl))
884884
{
885+
// The name of a field of an anonymous type should be same as the type's name minus the
886+
// type kind tag at the end and the leading `_`.
885887
var contextNamePart = GetRemappedCursorName(rootRecordDecl);
886-
887-
if (contextNamePart.StartsWith('_'))
888-
{
889-
var suffixLength = 0;
890-
891-
if (contextNamePart.EndsWith("_e__Union", StringComparison.Ordinal))
892-
{
893-
suffixLength = 10;
894-
}
895-
else if (contextNamePart.EndsWith("_e__Struct", StringComparison.Ordinal))
896-
{
897-
suffixLength = 11;
898-
}
899-
900-
if (suffixLength != 0)
901-
{
902-
contextNamePart = contextNamePart.Substring(1, contextNamePart.Length - suffixLength);
903-
}
904-
}
888+
var tagIndex = contextNamePart.LastIndexOf("_e__", StringComparison.Ordinal);
889+
Debug.Assert(contextNamePart[0] == '_');
890+
Debug.Assert(tagIndex >= 0);
891+
contextNamePart = contextNamePart.Substring(1, tagIndex - 1);
905892

906893
contextNameParts.Push(EscapeName(contextNamePart));
907894

@@ -1397,7 +1384,7 @@ private void VisitRecordDecl(RecordDecl recordDecl)
13971384
var maxAlignm = recordDecl.Fields.Any() ? recordDecl.Fields.Max((fieldDecl) => Math.Max(fieldDecl.Type.Handle.AlignOf, 1)) : alignment;
13981385

13991386
var isTopLevelStruct = _config.WithTypes.TryGetValue(name, out var withType) && withType.Equals("struct", StringComparison.Ordinal);
1400-
var generateTestsClass = !recordDecl.IsAnonymousStructOrUnion && recordDecl.DeclContext is not RecordDecl;
1387+
var generateTestsClass = !recordDecl.IsAnonymous && recordDecl.DeclContext is not RecordDecl;
14011388
var testOutputStarted = false;
14021389

14031390
var nullableUuid = (Guid?)null;

‎sources/ClangSharp.PInvokeGenerator/PInvokeGenerator.cs‎

Lines changed: 64 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -3160,9 +3160,19 @@ private string GetRemappedCursorName(NamedDecl namedDecl, out string nativeTypeN
31603160
Debug.Assert(parent is not null);
31613161
remappedName = GetRemappedCursorName(parent);
31623162
}
3163-
else if (namedDecl is FieldDecl fieldDecl)
3163+
else if ((namedDecl is FieldDecl fieldDecl) && name.StartsWith("__AnonymousFieldDecl_", StringComparison.Ordinal))
31643164
{
3165-
if (name.StartsWith("__AnonymousFieldDecl_", StringComparison.Ordinal))
3165+
if (fieldDecl.Type.AsCXXRecordDecl?.IsAnonymous == true)
3166+
{
3167+
// For fields of anonymous types, use the name of the type but clean off the type
3168+
// kind tag at the end.
3169+
var typeName = GetRemappedNameForAnonymousRecord(fieldDecl.Type.AsCXXRecordDecl);
3170+
var tagIndex = typeName.LastIndexOf("_e__", StringComparison.Ordinal);
3171+
Debug.Assert(typeName[0] == '_');
3172+
Debug.Assert(tagIndex >= 0);
3173+
remappedName = typeName.Substring(1, tagIndex - 1);
3174+
}
3175+
else
31663176
{
31673177
remappedName = "Anonymous";
31683178

@@ -3178,28 +3188,62 @@ private string GetRemappedCursorName(NamedDecl namedDecl, out string nativeTypeN
31783188
}
31793189
else if ((namedDecl is RecordDecl recordDecl) && name.StartsWith("__AnonymousRecord_", StringComparison.Ordinal))
31803190
{
3181-
if (recordDecl.Parent is RecordDecl parentRecordDecl)
3182-
{
3183-
remappedName = "_Anonymous";
3191+
remappedName = GetRemappedNameForAnonymousRecord(recordDecl);
3192+
}
31843193

3185-
var matchingField = parentRecordDecl.Fields.Where((fieldDecl) => fieldDecl.Type.CanonicalType == recordDecl.TypeForDecl.CanonicalType).FirstOrDefault();
3194+
return remappedName;
3195+
}
31863196

3187-
if (matchingField is not null)
3188-
{
3189-
remappedName = "_";
3190-
remappedName += GetRemappedCursorName(matchingField);
3191-
}
3192-
else if (parentRecordDecl.AnonymousRecords.Count > 1)
3197+
private string GetRemappedNameForAnonymousRecord(RecordDecl recordDecl)
3198+
{
3199+
if (recordDecl.Parent is RecordDecl parentRecordDecl)
3200+
{
3201+
var remappedNameBuilder = new StringBuilder();
3202+
var matchingField = parentRecordDecl.Fields.Where((fieldDecl) => fieldDecl.Type.CanonicalType == recordDecl.TypeForDecl.CanonicalType).FirstOrDefault();
3203+
3204+
if ((matchingField is not null) && !matchingField.IsAnonymousField)
3205+
{
3206+
_ = remappedNameBuilder.Append('_');
3207+
_ = remappedNameBuilder.Append(GetRemappedCursorName(matchingField));
3208+
}
3209+
else
3210+
{
3211+
_ = remappedNameBuilder.Append("_Anonymous");
3212+
3213+
// If there is more than one anonymous type, then add a numeral to differentiate.
3214+
if (parentRecordDecl.AnonymousRecords.Count > 1)
31933215
{
31943216
var index = parentRecordDecl.AnonymousRecords.IndexOf(recordDecl) + 1;
3195-
remappedName += index.ToString(CultureInfo.InvariantCulture);
3217+
Debug.Assert(index > 0);
3218+
_ = remappedNameBuilder.Append(index);
31963219
}
31973220

3198-
remappedName += $"_e__{(recordDecl.IsUnion ? "Union" : "Struct")}";
3221+
// C# doesn't allow a nested type to have the same name as the parent, so if the
3222+
// parent is also anonymous, add the nesting depth as a way to avoid conflicts with
3223+
// the parent's name.
3224+
if (parentRecordDecl.IsAnonymous)
3225+
{
3226+
var depth = 1;
3227+
var currentParent = parentRecordDecl.Parent;
3228+
while ((currentParent is RecordDecl currentParentRecordDecl) && currentParentRecordDecl.IsAnonymous)
3229+
{
3230+
depth++;
3231+
currentParent = currentParentRecordDecl.Parent;
3232+
}
3233+
_ = remappedNameBuilder.Append('_');
3234+
_ = remappedNameBuilder.Append(depth);
3235+
}
31993236
}
3200-
}
32013237

3202-
return remappedName;
3238+
// Add the type kind tag.
3239+
_ = remappedNameBuilder.Append("_e__");
3240+
_ = remappedNameBuilder.Append(recordDecl.IsUnion ? "Union" : "Struct");
3241+
return remappedNameBuilder.ToString();
3242+
}
3243+
else
3244+
{
3245+
return $"_Anonymous_e__{(recordDecl.IsUnion ? "Union" : "Struct")}";
3246+
}
32033247
}
32043248

32053249
private string GetRemappedName(string name, Cursor? cursor, bool tryRemapOperatorName, out bool wasRemapped, bool skipUsing = false)
@@ -3298,48 +3342,7 @@ private string GetRemappedTypeName(Cursor? cursor, Cursor? context, Type type, o
32983342
if (IsType<RecordType>(cursor, type, out var recordType) && remappedName.StartsWith("__AnonymousRecord_", StringComparison.Ordinal))
32993343
{
33003344
var recordDecl = recordType.Decl;
3301-
remappedName = "_Anonymous";
3302-
3303-
if (recordDecl.Parent is RecordDecl parentRecordDecl)
3304-
{
3305-
var matchingField = parentRecordDecl.Fields.Where((fieldDecl) => fieldDecl.Type.CanonicalType == recordType).FirstOrDefault();
3306-
3307-
if (matchingField is not null)
3308-
{
3309-
remappedName = "_";
3310-
remappedName += GetRemappedCursorName(matchingField);
3311-
}
3312-
else
3313-
{
3314-
var index = 0;
3315-
3316-
if (parentRecordDecl.AnonymousRecords.Count > 1)
3317-
{
3318-
index = parentRecordDecl.AnonymousRecords.IndexOf(cursor) + 1;
3319-
}
3320-
3321-
while (parentRecordDecl.IsAnonymousStructOrUnion && (parentRecordDecl.IsUnion == recordType.Decl.IsUnion))
3322-
{
3323-
index += 1;
3324-
3325-
if (parentRecordDecl.Parent is RecordDecl parentRecordDeclParent)
3326-
{
3327-
if (parentRecordDeclParent.AnonymousRecords.Count > 0)
3328-
{
3329-
index += parentRecordDeclParent.AnonymousRecords.Count - 1;
3330-
}
3331-
parentRecordDecl = parentRecordDeclParent;
3332-
}
3333-
}
3334-
3335-
if (index != 0)
3336-
{
3337-
remappedName += index.ToString(CultureInfo.InvariantCulture);
3338-
}
3339-
}
3340-
}
3341-
3342-
remappedName += $"_e__{(recordDecl.IsUnion ? "Union" : "Struct")}";
3345+
remappedName = GetRemappedNameForAnonymousRecord(recordDecl);
33433346
}
33443347
else if (IsType<EnumType>(cursor, type, out var enumType) && remappedName.StartsWith("__AnonymousEnum_", StringComparison.Ordinal))
33453348
{
@@ -4566,7 +4569,7 @@ private bool HasBaseField(CXXRecordDecl cxxRecordDecl)
45664569

45674570
private bool HasField(RecordDecl recordDecl)
45684571
{
4569-
var hasField = recordDecl.Fields.Any() || recordDecl.Decls.Any((decl) => (decl is RecordDecl nestedRecordDecl) && nestedRecordDecl.IsAnonymousStructOrUnion && HasField(nestedRecordDecl));
4572+
var hasField = recordDecl.Fields.Any() || recordDecl.Decls.Any((decl) => (decl is RecordDecl nestedRecordDecl) && nestedRecordDecl.IsAnonymous && HasField(nestedRecordDecl));
45704573

45714574
if (!hasField && (recordDecl is CXXRecordDecl cxxRecordDecl))
45724575
{
@@ -5117,7 +5120,7 @@ bool IsEmptyRecord(RecordDecl recordDecl)
51175120

51185121
foreach (var decl in recordDecl.Decls)
51195122
{
5120-
if ((decl is RecordDecl nestedRecordDecl) && nestedRecordDecl.IsAnonymousStructOrUnion && !IsEmptyRecord(nestedRecordDecl))
5123+
if ((decl is RecordDecl nestedRecordDecl) && nestedRecordDecl.IsAnonymous && !IsEmptyRecord(nestedRecordDecl))
51215124
{
51225125
return false;
51235126
}
@@ -6144,7 +6147,7 @@ private bool IsUnsafe(RecordDecl recordDecl)
61446147
{
61456148
return true;
61466149
}
6147-
else if ((decl is RecordDecl nestedRecordDecl) && nestedRecordDecl.IsAnonymousStructOrUnion && (IsUnsafe(nestedRecordDecl) || Config.GenerateCompatibleCode))
6150+
else if ((decl is RecordDecl nestedRecordDecl) && nestedRecordDecl.IsAnonymous && (IsUnsafe(nestedRecordDecl) || Config.GenerateCompatibleCode))
61486151
{
61496152
return true;
61506153
}

‎sources/ClangSharp/Cursors/Decls/RecordDecl.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,12 @@ private protected RecordDecl(CXCursor handle, CXCursorKind expectedCursorKind, C
4848
});
4949

5050
_anonymousFields = new Lazy<IReadOnlyList<FieldDecl>>(() => Decls.OfType<FieldDecl>().Where(decl => decl.IsAnonymousField).ToList());
51-
_anonymousRecords = new Lazy<IReadOnlyList<RecordDecl>>(() => Decls.OfType<RecordDecl>().Where(decl => decl.IsAnonymousStructOrUnion && !decl.IsInjectedClassName).ToList());
51+
_anonymousRecords = new Lazy<IReadOnlyList<RecordDecl>>(() => Decls.OfType<RecordDecl>().Where(decl => decl.IsAnonymous && !decl.IsInjectedClassName).ToList());
5252
_indirectFields = new Lazy<IReadOnlyList<IndirectFieldDecl>>(() => Decls.OfType<IndirectFieldDecl>().ToList());
5353
_injectedClassName = new Lazy<RecordDecl?>(() => Decls.OfType<RecordDecl>().Where(decl => decl.IsInjectedClassName).SingleOrDefault());
5454
}
5555

56-
public bool IsAnonymousStructOrUnion => Handle.IsAnonymousStructOrUnion;
56+
public bool IsAnonymous => Handle.IsAnonymous;
5757

5858
public IReadOnlyList<FieldDecl> AnonymousFields => _anonymousFields.Value;
5959

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,21 @@ public abstract class StructDeclarationTest : PInvokeGeneratorTest
215215
[Test]
216216
public Task SourceLocationAttributeTest() => SourceLocationAttributeTestImpl();
217217

218+
// Regression test: the code that generated the name for anonymous types had a couple of
219+
// bugs:
220+
// * It would only append a numeral if the parent had more than one anonymous records,
221+
// but an anonymous typed array didn't count as a record resulting in multiple structs
222+
// named `_Anonymous_e__Struct`.
223+
// * The code that remapped the name of anonymous types was different between the code that
224+
// generated the type definition and the code that defined fields in a FixedBuffer.
225+
[Test]
226+
public Task AnonStructAndAnonStructArray() => AnonStructAndAnonStructArrayImpl();
227+
228+
// Regression test: nested anonymous structs would result in:
229+
// error CS0542: '_Anonymous_e__Struct': member names cannot be the same as their enclosing type
230+
[Test]
231+
public Task DeeplyNestedAnonStructs() => DeeplyNestedAnonStructsImpl();
232+
218233
protected abstract Task IncompleteArraySizeTestImpl(string nativeType, string expectedManagedType);
219234

220235
protected abstract Task BasicTestImpl(string nativeType, string expectedManagedType);
@@ -290,4 +305,8 @@ public abstract class StructDeclarationTest : PInvokeGeneratorTest
290305
protected abstract Task WithPackingTestImpl();
291306

292307
protected abstract Task SourceLocationAttributeTestImpl();
308+
309+
protected abstract Task AnonStructAndAnonStructArrayImpl();
310+
311+
protected abstract Task DeeplyNestedAnonStructsImpl();
293312
}

0 commit comments

Comments
 (0)