ClangSharp icon indicating copy to clipboard operation
ClangSharp copied to clipboard

Clash when `union` field has the same name and "`struct` tag"

Open MarijnS95 opened this issue 6 months ago • 5 comments

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).

MarijnS95 avatar Jun 17 '25 18:06 MarijnS95

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.

tannergooding avatar Jul 17 '25 17:07 tannergooding

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 avatar Jul 30 '25 20:07 MarijnS95

@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.

tannergooding avatar Sep 05 '25 19:09 tannergooding

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 avatar Sep 05 '25 19:09 tannergooding

@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

MarijnS95 avatar Oct 27 '25 11:10 MarijnS95