Skip to content

Don't emit duplicate members for structs with multiple vtbl bases #748

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
tannergooding merged 1 commit into from
Jul 13, 2026
Merged

Don't emit duplicate members for structs with multiple vtbl bases #748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
69 changes: 55 additions & 14 deletions sources/ClangSharp.PInvokeGenerator/PInvokeGenerator.VisitRecordDecl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ private void VisitRecordDecl(RecordDecl recordDecl)

if (hasVtbl || hasBaseVtbl)
{
OutputDelegateSignatures(cxxRecordDecl, cxxRecordDecl);
OutputDelegateSignatures(cxxRecordDecl, cxxRecordDecl, new HashSet<string>(StringComparer.Ordinal));
}
}

Expand Down Expand Up @@ -565,7 +565,7 @@ private void VisitRecordDecl(RecordDecl recordDecl)
_outputBuilder.EmitFnPtrSupport();
}

OutputVtblHelperMethods(cxxRecordDecl, cxxRecordDecl);
OutputVtblHelperMethods(cxxRecordDecl, cxxRecordDecl, new HashSet<string>(StringComparer.Ordinal));

if (_config.GenerateMarkerInterfaces)
{
Expand All @@ -587,7 +587,7 @@ private void VisitRecordDecl(RecordDecl recordDecl)
}

_outputBuilder.BeginExplicitVtbl();
OutputVtblEntries(cxxRecordDecl, cxxRecordDecl);
OutputVtblEntries(cxxRecordDecl, cxxRecordDecl, new HashSet<string>(StringComparer.Ordinal));
_outputBuilder.EndExplicitVtbl();
}
}
Expand Down Expand Up @@ -618,7 +618,7 @@ string FixupNameForMultipleHits(CXXMethodDecl cxxMethodDecl)
return remappedName;
}

void OutputDelegateSignatures(CXXRecordDecl rootCxxRecordDecl, CXXRecordDecl cxxRecordDecl)
void OutputDelegateSignatures(CXXRecordDecl rootCxxRecordDecl, CXXRecordDecl cxxRecordDecl, HashSet<string> emittedMemberNames)
{
if (!_config.ExcludeFnptrCodegen)
{
Expand All @@ -628,7 +628,7 @@ void OutputDelegateSignatures(CXXRecordDecl rootCxxRecordDecl, CXXRecordDecl cxx
foreach (var cxxBaseSpecifier in cxxRecordDecl.Bases)
{
var baseCxxRecordDecl = GetRecordDecl(cxxBaseSpecifier);
OutputDelegateSignatures(rootCxxRecordDecl, baseCxxRecordDecl);
OutputDelegateSignatures(rootCxxRecordDecl, baseCxxRecordDecl, emittedMemberNames);
}

var cxxMethodDecls = cxxRecordDecl.Methods;
Expand All @@ -647,9 +647,15 @@ void OutputDelegateSignatures(CXXRecordDecl rootCxxRecordDecl, CXXRecordDecl cxx
continue;
}

var remappedName = FixupNameForMultipleHits(cxxMethodDecl);

if (!emittedMemberNames.Add(GetVtblMemberDeduplicationKey(remappedName, cxxMethodDecl)))
{
continue;
}

_outputBuilder.WriteDivider();

var remappedName = FixupNameForMultipleHits(cxxMethodDecl);
Debug.Assert(CurrentContext.Cursor == rootCxxRecordDecl);
Visit(cxxMethodDecl);
}
Expand Down Expand Up @@ -772,12 +778,12 @@ void OutputMarkerInterfaces(CXXRecordDecl rootCxxRecordDecl, CXXRecordDecl cxxRe
}
}

void OutputVtblEntries(CXXRecordDecl rootCxxRecordDecl, CXXRecordDecl cxxRecordDecl)
void OutputVtblEntries(CXXRecordDecl rootCxxRecordDecl, CXXRecordDecl cxxRecordDecl, HashSet<string> emittedMemberNames)
{
foreach (var cxxBaseSpecifier in cxxRecordDecl.Bases)
{
var baseCxxRecordDecl = GetRecordDecl(cxxBaseSpecifier);
OutputVtblEntries(rootCxxRecordDecl, baseCxxRecordDecl);
OutputVtblEntries(rootCxxRecordDecl, baseCxxRecordDecl, emittedMemberNames);
}

var cxxMethodDecls = cxxRecordDecl.Methods;
Expand All @@ -786,12 +792,12 @@ void OutputVtblEntries(CXXRecordDecl rootCxxRecordDecl, CXXRecordDecl cxxRecordD
{
foreach (var cxxMethodDecl in cxxMethodDecls.OrderBy((cxxmd) => cxxmd.VtblIndex))
{
OutputVtblEntry(rootCxxRecordDecl, cxxMethodDecl);
OutputVtblEntry(rootCxxRecordDecl, cxxMethodDecl, emittedMemberNames);
}
}
}

void OutputVtblEntry(CXXRecordDecl cxxRecordDecl, CXXMethodDecl cxxMethodDecl)
void OutputVtblEntry(CXXRecordDecl cxxRecordDecl, CXXMethodDecl cxxMethodDecl, HashSet<string> emittedMemberNames)
{
if (!cxxMethodDecl.IsVirtual)
{
Expand All @@ -814,6 +820,11 @@ void OutputVtblEntry(CXXRecordDecl cxxRecordDecl, CXXMethodDecl cxxMethodDecl)
var remappedName = FixupNameForMultipleHits(cxxMethodDecl);
var escapedName = EscapeAndStripMethodName(remappedName);

if (!emittedMemberNames.Add(GetVtblMemberDeduplicationKey(escapedName, cxxMethodDecl)))
{
return;
}

var desc = new FieldDesc {
AccessSpecifier = AccessSpecifier.Public,
NativeTypeName = nativeTypeName,
Expand All @@ -837,7 +848,7 @@ void OutputVtblEntry(CXXRecordDecl cxxRecordDecl, CXXMethodDecl cxxMethodDecl)
_outputBuilder.WriteDivider();
}

void OutputVtblHelperMethod(CXXRecordDecl cxxRecordDecl, CXXMethodDecl cxxMethodDecl)
void OutputVtblHelperMethod(CXXRecordDecl cxxRecordDecl, CXXMethodDecl cxxMethodDecl, HashSet<string> emittedMemberNames)
{
if (!cxxMethodDecl.IsVirtual)
{
Expand All @@ -849,6 +860,11 @@ void OutputVtblHelperMethod(CXXRecordDecl cxxRecordDecl, CXXMethodDecl cxxMethod
return;
}

if (!emittedMemberNames.Add(GetVtblMemberDeduplicationKey(EscapeAndStripMethodName(GetRemappedCursorName(cxxMethodDecl)), cxxMethodDecl)))
{
return;
}

var currentContext = _context.AddLast((cxxMethodDecl, null));

var returnType = cxxMethodDecl.ReturnType;
Expand Down Expand Up @@ -1092,12 +1108,12 @@ void OutputVtblHelperMethod(CXXRecordDecl cxxRecordDecl, CXXMethodDecl cxxMethod
_context.RemoveLast();
}

void OutputVtblHelperMethods(CXXRecordDecl rootCxxRecordDecl, CXXRecordDecl cxxRecordDecl)
void OutputVtblHelperMethods(CXXRecordDecl rootCxxRecordDecl, CXXRecordDecl cxxRecordDecl, HashSet<string> emittedMemberNames)
{
foreach (var cxxBaseSpecifier in cxxRecordDecl.Bases)
{
var baseCxxRecordDecl = GetRecordDecl(cxxBaseSpecifier);
OutputVtblHelperMethods(rootCxxRecordDecl, baseCxxRecordDecl);
OutputVtblHelperMethods(rootCxxRecordDecl, baseCxxRecordDecl, emittedMemberNames);
}

var cxxMethodDecls = cxxRecordDecl.Methods;
Expand All @@ -1107,7 +1123,7 @@ void OutputVtblHelperMethods(CXXRecordDecl rootCxxRecordDecl, CXXRecordDecl cxxR
foreach (var cxxMethodDecl in cxxMethodDecls.OrderBy((cxxmd) => cxxmd.VtblIndex))
{
_outputBuilder.WriteDivider();
OutputVtblHelperMethod(rootCxxRecordDecl, cxxMethodDecl);
OutputVtblHelperMethod(rootCxxRecordDecl, cxxMethodDecl, emittedMemberNames);
}
}
}
Expand Down Expand Up @@ -2023,4 +2039,29 @@ void StartTestOutput(ref bool testOutputStarted, bool generateTestsClass, bool i
}
}
}

// Multiple base classes can each contribute a virtual member that maps to the same C# name and
// signature (most notably each base's virtual destructor becoming `Dispose`). The single `lpVtbl`
// model flattens every base into one vtable, so the same member would otherwise be emitted more
// than once, which is a compile error. This builds a key of the emitted name plus the canonical
// parameter types so that legitimate overloads remain distinct. See https://github.com/dotnet/ClangSharp/issues/592
private static string GetVtblMemberDeduplicationKey(string emittedName, CXXMethodDecl cxxMethodDecl)
{
var builder = new StringBuilder(emittedName);
_ = builder.Append('(');

var parameters = cxxMethodDecl.Parameters;

for (var index = 0; index < parameters.Count; index++)
{
if (index != 0)
{
_ = builder.Append(',');
}
_ = builder.Append(parameters[index].Type.CanonicalType.AsString);
}

_ = builder.Append(')');
return builder.ToString();
}
}
29 changes: 16 additions & 13 deletions sources/ClangSharp.PInvokeGenerator/PInvokeGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1430,25 +1430,28 @@ private bool HasVtbl(CXXRecordDecl cxxRecordDecl, out bool hasBaseVtbl)
var hasVtbl = cxxRecordDecl.Methods.Any((method) => method.IsVirtual && (method.OverriddenMethods.Count == 0));
hasBaseVtbl = false;

if (!hasVtbl)
var indirectVtblCount = 0;

foreach (var cxxBaseSpecifier in cxxRecordDecl.Bases)
{
var indirectVtblCount = 0;
var baseCxxRecordDecl = GetRecordDecl(cxxBaseSpecifier);

foreach (var cxxBaseSpecifier in cxxRecordDecl.Bases)
if ((HasVtbl(baseCxxRecordDecl, out var baseHasBaseVtbl) || baseHasBaseVtbl) && !HasField(baseCxxRecordDecl))
{
var baseCxxRecordDecl = GetRecordDecl(cxxBaseSpecifier);

if ((HasVtbl(baseCxxRecordDecl, out var baseHasBaseVtbl) || baseHasBaseVtbl) && !HasField(baseCxxRecordDecl))
{
indirectVtblCount++;
}
indirectVtblCount++;
}
}

if (indirectVtblCount > 1)
{
AddDiagnostic(DiagnosticLevel.Warning, "Unsupported cxx record declaration: 'multiple virtual bases'. Generated bindings may be incomplete.", cxxRecordDecl);
}
// Multiple virtual bases require a distinct vtable pointer per base subobject, but the
// generated bindings only model a single, flattened `lpVtbl`. This is true even when the
// derived type introduces its own virtuals (`hasVtbl`), so the warning must fire regardless.
if (indirectVtblCount > 1)
{
AddDiagnostic(DiagnosticLevel.Warning, "Unsupported cxx record declaration: 'multiple virtual bases'. Generated bindings may be incomplete.", cxxRecordDecl);
}

if (!hasVtbl)
{
hasBaseVtbl = indirectVtblCount != 0;
}

Expand Down
Loading
Loading