Skip to content

Commit

Permalink
Fix name generation for nested types
Browse files Browse the repository at this point in the history
  • Loading branch information
@dpaoliello
dpaoliello committed Dec 16, 2024
1 parent 2d4e661 commit 0dd7c61
Show file tree
Hide file tree
Showing 37 changed files with 2,455 additions and 502 deletions.
31 changes: 9 additions & 22 deletions sources/ClangSharp.PInvokeGenerator/PInvokeGenerator.VisitDecl.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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));

Expand Down Expand Up @@ -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;
Expand Down
125 changes: 64 additions & 61 deletions sources/ClangSharp.PInvokeGenerator/PInvokeGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3160,9 +3160,19 @@ private string GetRemappedCursorName(NamedDecl namedDecl, out string nativeTypeN
Debug.Assert(parent is not null);
remappedName = GetRemappedCursorName(parent);
}
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";

Expand All @@ -3178,28 +3188,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)
Expand Down Expand Up @@ -3298,48 +3342,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))
{
Expand Down Expand Up @@ -4566,7 +4569,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))
{
Expand Down Expand Up @@ -5117,7 +5120,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;
}
Expand Down Expand Up @@ -6144,7 +6147,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;
}
Expand Down
4 changes: 2 additions & 2 deletions sources/ClangSharp/Cursors/Decls/RecordDecl.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
19 changes: 19 additions & 0 deletions tests/ClangSharp.PInvokeGenerator.UnitTests/Base/StructDeclarationTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
}
Loading

0 comments on commit 0dd7c61

Please sign in to comment.