Skip to content

Fixes name generation for nested anonymous types #587

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jan 10, 2025
Merged

Fixes name generation for nested anonymous types #587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0dd7c61
Fix name generation for nested types
dpaoliello Dec 16, 2024
ad49dcb
Merge branch 'main' into deeplynested
tannergooding Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 9 additions & 22 deletions sources/ClangSharp.PInvokeGenerator/PInvokeGenerator.VisitDecl.cs
View file
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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)
Expand Down Expand Up @@ -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))
{
Expand Down Expand Up @@ -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))
{
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions sources/ClangSharp/Cursors/Decls/RecordDecl.cs
View file
Open in desktop
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
Open in desktop
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
Loading