Clash when `union` field has the same name and "`struct` tag"
pix3.h from https://www.nuget.org/packages/WinPixEventRuntime/1.0.230302001 defines a union as follows:
union PIXCaptureParameters
{
enum PIXCaptureStorage
{
Memory = 0,
};
struct GpuCaptureParameters
{
PCWSTR FileName;
} GpuCaptureParameters;
struct TimingCaptureParameters
{
PCWSTR FileName;
UINT32 MaximumToolingMemorySizeMb;
PIXCaptureStorage CaptureStorage;
BOOL CaptureGpuTiming;
BOOL CaptureCallstacks;
BOOL CaptureCpuSamples;
UINT32 CpuSamplesPerSecond;
BOOL CaptureFileIO;
BOOL CaptureVirtualAllocEvents;
BOOL CaptureHeapAllocEvents;
BOOL CaptureXMemEvents; // Xbox only
BOOL CapturePixMemEvents; // Xbox only
} TimingCaptureParameters;
};
This generates the following CSharp code:
[StructLayout(LayoutKind.Explicit)]
public partial struct PIXCaptureParameters
{
[FieldOffset(0)]
[NativeTypeName("struct GpuCaptureParameters")] public GpuCaptureParameters GpuCaptureParameters;
[FieldOffset(0)]
[NativeTypeName("struct TimingCaptureParameters")] public TimingCaptureParameters TimingCaptureParameters;
public enum PIXCaptureStorage
{
Memory = 0,
}
public unsafe partial struct GpuCaptureParameters
{
[NativeTypeName("PCWSTR"),Const] public ushort* FileName;
}
public unsafe partial struct TimingCaptureParameters
{
[NativeTypeName("PCWSTR"),Const] public ushort* FileName;
[NativeTypeName("UINT32")] public uint MaximumToolingMemorySizeMb;
[NativeTypeName("PIXCaptureParameters::PIXCaptureStorage")] public PIXCaptureStorage CaptureStorage;
[NativeTypeName("BOOL")] public int CaptureGpuTiming;
[NativeTypeName("BOOL")] public int CaptureCallstacks;
[NativeTypeName("BOOL")] public int CaptureCpuSamples;
[NativeTypeName("UINT32")] public uint CpuSamplesPerSecond;
[NativeTypeName("BOOL")] public int CaptureFileIO;
[NativeTypeName("BOOL")] public int CaptureVirtualAllocEvents;
[NativeTypeName("BOOL")] public int CaptureHeapAllocEvents;
[NativeTypeName("BOOL")] public int CaptureXMemEvents;
[NativeTypeName("BOOL")] public int CapturePixMemEvents;
}
}
Which fails to compile because the field name clashes the nested struct name:
windows-pix\.metadata\obj\generated\obj\crossarch\common\WinPixEventRuntime.modified.cs(272,84): error CS0102: The type 'PIXCaptureParameters' already contains a definition for 'GpuCaptureParameters'
windows-pix\.metadata\obj\generated\obj\crossarch\common\WinPixEventRuntime.modified.cs(275,90): error CS0102: The type 'PIXCaptureParameters' already contains a definition for 'TimingCaptureParameters'
What would be the right way to fix this? I tried using --remap PIXCaptureParameters::GpuCaptureParameters=GpuCaptureParameters_t which makes the struct- and field name change, but the field type does not (i.e. compile error because that type is no longer found).
When using --remap GpuCaptureParameters=GpuCaptureParameters_t, all three change, also defeating the purpose (field name, field type, and type name).
This is just an unhandled edge-case due to C allowing something C# doesn't.
The name should be modified in this scenario to ensure the struct is unique, potentially falling back to the Anonymous struct name selection logic.
Thanks @tannergooding, that makes sense. However, what is the way to rename the struct - for example making it anonymous like you proposed - in ClangSharp without modifying the original header? That's something I don't have access to, nor think Microsoft will change when I ask (maybe that's even a breaking API change).
The worst alternative I can think of is finding a way to modify my generate.proj (that references Sdk="Microsoft.Windows.WinmdGenerator) to find/replace this name in the header file before running it through, if that's even possible "as a pre-build step". Curious to hear your thoughts!
@MarijnS95, sorry for the delay. I got absorbed into some work with prepping the .NET 10 release and this fell off my radar.
Due to this just being name sensitive, there doesn't look to be a way to differentiate between these today.
I'd need to add something like --remap type:Name=name and --remap field:Name=name or --remap-type and --remap-field or similar, so we can disambiguate.
I think the latter is just a little clearer and avoids potential issues on misinterpreting the name.
This should be a relatively small tweak, so I'm happy if someone wants to submit a PR and otherwise I'll try to get to this over the weekend as part of prepping for the next release
@tannergooding thanks; no I haven't been able to submit a PR as I'm not up-to-speed with working on/in this project yet.
Even if this is fixed, we'd be blocked one step further down the line with some little windows-rs codegen for nested types too: https://github.com/microsoft/windows-rs/issues/3468#issuecomment-3450819834