Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 33 additions & 1 deletion sources/ClangSharp.PInvokeGenerator/PInvokeGenerator.Naming.cs
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,11 @@ private static string GetNamespaceQualifiedNativeTypeName(Type type, string nati
return GetNamespaceQualifiedNativeTypeName(decl, nativeTypeName);
}

// Tokens the clang type printer can spell before a type's nested-name-specifier: cv-qualifiers,
// elaborated tag keywords, and the MS `__unaligned` type qualifier. A restored `Namespace::`
// qualifier belongs on the type name, after any leading run of these.
private static readonly string[] s_leadingTypeQualifiers = ["const ", "volatile ", "struct ", "class ", "union ", "enum ", "__unaligned "];

private static string GetNamespaceQualifiedNativeTypeName(Decl decl, string nativeTypeName)
{
// clang 22's type printer omits the enclosing C++ namespace from a reference when a
Expand All @@ -213,7 +218,34 @@ private static string GetNamespaceQualifiedNativeTypeName(Decl decl, string nati
}

var qualifier = qualifierBuilder.ToString();
return nativeTypeName.StartsWith(qualifier, StringComparison.Ordinal) ? nativeTypeName : qualifier + nativeTypeName;

// The qualifier belongs on the type name, after any leading cv-qualifiers, elaborated tag
// keywords, or `__unaligned`, so e.g. a `const struct` pointer stays `const struct Ns::Point *`
// rather than the malformed `Ns::const struct Point *`.
var offset = 0;

while (true)
{
var rest = nativeTypeName.AsSpan(offset);
var matched = false;

foreach (var prefix in s_leadingTypeQualifiers)
{
if (rest.StartsWith(prefix, StringComparison.Ordinal))
{
offset += prefix.Length;
matched = true;
break;
}
}

if (!matched)
{
break;
}
}

return nativeTypeName.AsSpan(offset).StartsWith(qualifier, StringComparison.Ordinal) ? nativeTypeName : nativeTypeName.Insert(offset, qualifier);
}

private string GetCursorQualifiedName(NamedDecl namedDecl, bool truncateParameters = false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -385,12 +385,24 @@ private string GetTypeName(Cursor? cursor, Cursor? context, Type rootType, Type
result.typeName = GetRemappedCursorName(tagType.Decl, out _, skipUsing: true);

// A nested type needs to be qualified by its containing type(s) so it resolves
// when referenced from another scope (e.g. `A::Inner` -> `A.Inner`). Namespaces
// are flattened away, so only walk the enclosing record decls.
// when referenced from another scope (e.g. `A::Inner` -> `A.Inner`). A reference
// from within a containing type sees the nested type directly, so only qualify
// up to the scope shared with the reference site. Namespaces are flattened away,
// so only walk the enclosing record decls.

var referenceScope = new HashSet<Decl>();

for (var refContext = (cursor as Decl)?.DeclContext ?? (context as Decl)?.DeclContext; refContext is Decl refParent; refContext = refParent.DeclContext)
{
if (refParent is RecordDecl)
{
_ = referenceScope.Add(refParent);
}
}

var qualificationBuilder = new StringBuilder();

for (var declContext = tagType.Decl.DeclContext; declContext is RecordDecl parentRecordDecl; declContext = parentRecordDecl.DeclContext)
for (var declContext = tagType.Decl.DeclContext; declContext is RecordDecl parentRecordDecl && !referenceScope.Contains(parentRecordDecl); declContext = parentRecordDecl.DeclContext)
{
var parentRecordDeclName = GetRemappedCursorName(parentRecordDecl, out _, skipUsing: true);
_ = qualificationBuilder.Insert(0, '.').Insert(0, EscapeName(parentRecordDeclName));
Expand Down
37 changes: 24 additions & 13 deletions sources/ClangSharp.PInvokeGenerator/PInvokeGenerator.VisitStmt.cs
Original file line number Diff line number Diff line change
Expand Up @@ -749,27 +749,38 @@ private void VisitCXXConstructExpr(CXXConstructExpr cxxConstructExpr)

if (args.Count != 0)
{
if (isUnmanagedConstant)
if (isUnmanagedConstant
&& IsStmtAsWritten<UnaryOperator>(args[0], out var derefOperator, removeParens: true)
&& (derefOperator.Opcode == CXUnaryOperator_Deref))
{
outputBuilder.Write("ref ");
// A computed-pointer GUID macro (e.g. `MAKEDIPROP(n)` -> `*(const GUID*)(n)`) is exposed
// as the pointer itself, so emit its operand rather than a byref to arbitrary memory.
Visit(derefOperator.SubExpr);
}

var needsComma = false;

for (var i = 0; i < args.Count; i++)
else
{
var arg = args[i];

if (needsComma && (arg is not CXXDefaultArgExpr))
if (isUnmanagedConstant)
{
outputBuilder.Write(", ");
outputBuilder.Write("ref ");
}

Visit(arg);
var needsComma = false;

if (arg is not CXXDefaultArgExpr)
for (var i = 0; i < args.Count; i++)
{
needsComma = true;
var arg = args[i];

if (needsComma && (arg is not CXXDefaultArgExpr))
{
outputBuilder.Write(", ");
}

Visit(arg);

if (arg is not CXXDefaultArgExpr)
{
needsComma = true;
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,13 +204,22 @@ private void VisitVarDecl(VarDecl varDecl)
return;
}

// clang wraps an alias to another constant (e.g. `#define IID_X IID_Y`) in a
// copy-constructor. The referenced constant has backing storage, so keep the
// `ref readonly` alias rather than emitting a by-value copy.
// clang wraps an alias to another constant (e.g. `#define IID_X IID_Y`) or a
// pointer dereference (e.g. `#define X MAKEDIPROP(n)` -> `*(const GUID*)(n)`) in a
// copy-constructor.
if (IsStmtAsWritten<DeclRefExpr>(cxxConstructExpr.Args[0], out _, removeParens: true))
{
// An alias references the other constant's backing storage, so keep the
// `ref readonly` alias rather than emitting a by-value copy.
flags |= ValueFlags.Reference;
}
else if (IsStmtAsWritten<UnaryOperator>(cxxConstructExpr.Args[0], out var unaryOperator, removeParens: true) && (unaryOperator.Opcode == CXUnaryOperator_Deref))
{
// The dereference targets an arbitrary address (typically illegal to read),
// so expose the pointer itself; a managed byref to it would not be valid.
typeName += '*';
_topLevelClassIsUnsafe[className] = true;
}

flags |= ValueFlags.Copy;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,21 @@ public unsafe partial struct Holder
[NativeTypeName("Ns::Point *")]
public Point* pointPtr;

[NativeTypeName("const Ns::Point *")]
public Point* constPointPtr;

[NativeTypeName("const Ns::Real *")]
public float* constRealPtr;

[NativeTypeName("struct Ns::Point *")]
public Point* elabPointPtr;

[NativeTypeName("const struct Ns::Point *")]
public Point* constElabPointPtr;

[NativeTypeName("enum Ns::Status *")]
public Status* elabStatusPtr;

[NativeTypeName("Ns::Real")]
public float r;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,21 @@
<field name="pointPtr" access="public">
<type native="Ns::Point *">Point*</type>
</field>
<field name="constPointPtr" access="public">
<type native="const Ns::Point *">Point*</type>
</field>
<field name="constRealPtr" access="public">
<type native="const Ns::Real *">float*</type>
</field>
<field name="elabPointPtr" access="public">
<type native="struct Ns::Point *">Point*</type>
</field>
<field name="constElabPointPtr" access="public">
<type native="const struct Ns::Point *">Point*</type>
</field>
<field name="elabStatusPtr" access="public">
<type native="enum Ns::Status *">Status*</type>
</field>
<field name="r" access="public">
<type native="Ns::Real">float</type>
</field>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
namespace ClangSharp.Test
{
public unsafe partial struct Outer
{
public Inner field;

public Inner* fieldPtr;

public partial struct Inner
{
public int value;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,14 @@ public partial struct IInternet
public int x;
}

public static partial class Methods
public static unsafe partial class Methods
{
[NativeTypeName("#define IID_IOInet IID_IInternet")]
public static ref readonly Guid IID_IOInet => ref IID_IInternet;

[NativeTypeName("#define DIPROP_BUFFERSIZE (*(const GUID *)(1))")]
public static Guid* DIPROP_BUFFERSIZE => unchecked((Guid*)(1));

public static ref readonly Guid IID_IInternet
{
get
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,14 @@ public partial struct IInternet
public int x;
}

public static partial class Methods
public static unsafe partial class Methods
{
[NativeTypeName("#define IID_IOInet IID_IInternet")]
public static ref readonly Guid IID_IOInet => ref IID_IInternet;

[NativeTypeName("#define DIPROP_BUFFERSIZE (*(const GUID *)(1))")]
public static Guid* DIPROP_BUFFERSIZE => unchecked((Guid*)(1));

public static ref readonly Guid IID_IInternet
{
get
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,20 @@
<type>int</type>
</field>
</struct>
<class name="Methods" access="public" static="true">
<class name="Methods" access="public" static="true" unsafe="true">
<constant name="IID_IOInet" access="public">
<type primitive="False">Guid</type>
<value>
<code>ref IID_IInternet</code>
</value>
</constant>
<constant name="DIPROP_BUFFERSIZE" access="public">
<type primitive="False">Guid*</type>
<value>
<code>
<unchecked></unchecked>(<value>(Guid*)(1)</value>)</code>
</value>
</constant>
<iid name="IID_IInternet" value="0x79EAC9E0, 0xBAF9, 0x11CE, 0x8C, 0x82, 0x00, 0xAA, 0x00, 0x4B, 0xA9, 0x0B" />
</class>
</namespace>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,20 @@
<type>int</type>
</field>
</struct>
<class name="Methods" access="public" static="true">
<class name="Methods" access="public" static="true" unsafe="true">
<constant name="IID_IOInet" access="public">
<type primitive="False">Guid</type>
<value>
<code>ref IID_IInternet</code>
</value>
</constant>
<constant name="DIPROP_BUFFERSIZE" access="public">
<type primitive="False">Guid*</type>
<value>
<code>
<unchecked></unchecked>(<value>(Guid*)(1)</value>)</code>
</value>
</constant>
<iid name="IID_IInternet" value="0x79EAC9E0, 0xBAF9, 0x11CE, 0x8C, 0x82, 0x00, 0xAA, 0x00, 0x4B, 0xA9, 0x0B" />
</class>
</namespace>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ public NamespaceNativeTypeNameTest(BaselineVariant variant) : base(variant)
// clang 22's type printer omits the enclosing C++ namespace from a reference spelled from within that
// same namespace, where-as older releases always spelled it. The generator restores the dropped
// `Namespace::` prefix from the decl so the emitted `NativeTypeName` keeps the fully qualified source
// spelling for typedef, record (including by-pointer), and enum references and stays version-stable.
// spelling for typedef, record (including by-pointer, const-qualified, and elaborated `struct`/`enum`
// specifiers), and enum references, placing the qualifier after any leading cv-qualifiers or tag
// keywords, and stays version-stable.
[Test]
public Task NamespaceQualifiedNativeTypeNameIsPreservedTest()
{
Expand All @@ -41,6 +43,11 @@ struct Holder
{
Point point;
Point* pointPtr;
const Point* constPointPtr;
const Real* constRealPtr;
struct Point* elabPointPtr;
const struct Point* constElabPointPtr;
enum Status* elabStatusPtr;
Real r;
Status status;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -604,8 +604,9 @@ public Task GuidDefineAliasRefReadonlyTest()
return Task.CompletedTask;
}

// A `#define IID_X IID_Y` alias over a `ref readonly Guid` IID must keep the `ref readonly` return so it
// stays a zero-copy alias; dropping it leaves a by-value `Guid` return paired with a `=> ref` body.
// A `#define IID_X IID_Y` alias references another constant's storage, so it stays a `ref readonly Guid`
// alias. A `#define X (*(const GUID*)(n))` computed-pointer deref (as `MAKEDIPROP` expands to) targets an
// arbitrary address, so it is exposed as a `Guid*` pointer rather than an invalid managed byref.
var inputContents = @"#define DECLSPEC_UUID(x) __declspec(uuid(x))
#define EXTERN_C extern ""C""

Expand All @@ -628,6 +629,7 @@ struct DECLSPEC_UUID(""79eac9e0-baf9-11ce-8c82-00aa004ba90b"") IInternet
EXTERN_C const IID IID_IInternet;

#define IID_IOInet IID_IInternet
#define DIPROP_BUFFERSIZE (*(const GUID *)(1))
";

var remappedNames = new Dictionary<string, string> { ["_GUID"] = "Guid", ["GUID"] = "Guid" };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,24 @@ struct B
{
A::Inner inner;
};
";

return ValidateGeneratedCSharpLatestWindowsBaselineAsync(inputContents);
}

[Test]
public Task NestedTypeIsNotQualifiedWithinContainer()
{
var inputContents = @"struct Outer
{
struct Inner
{
int value;
};

Inner field;
Inner* fieldPtr;
};
";

return ValidateGeneratedCSharpLatestWindowsBaselineAsync(inputContents);
Expand Down
Loading