generation/WinSDK/manual/Backup.cs could be removed

[xtqqczze]

dotnet/ClangSharp#366 is closed so it appears this file could be removed.

win32metadata/generation/WinSDK/manual/Backup.cs

Lines 5 to 7 in 56ad3d0

	// Because of ClangSharp: https://github.com/dotnet/ClangSharp/issues/366
	// If this gets fixed we could remove this class
	public unsafe partial struct ACTIVATION_CONTEXT_COMPATIBILITY_INFORMATION

[riverar]

Without this, metadata generates a struct that appears as such:

public partial struct ACTIVATION_CONTEXT_COMPATIBILITY_INFORMATION
{
    [NativeTypeName("DWORD")]
    public uint ElementCount;

    [NativeTypeName("COMPATIBILITY_CONTEXT_ELEMENT[]")]
    public _Elements_e__FixedBuffer Elements;

    public partial struct _Elements_e__FixedBuffer
    {
        public COMPATIBILITY_CONTEXT_ELEMENT* e0;

        public unsafe ref COMPATIBILITY_CONTEXT_ELEMENT* this[int index]
        {
            get
            {
                fixed (COMPATIBILITY_CONTEXT_ELEMENT** pThis = &e0)
                {
                    return ref pThis[index];
                }
            }
        }
    }
}

This fails compilation with error CS0214: Pointers and fixed size buffers may only be used in an unsafe context. The linked issue shows a commit to 14.0.0-rc1 but doesn't seem to work for this case. Am testing with 15.0.0 now.

cc: @tannergooding

[riverar]

I pushed the scrape headers task to ClangSharp 15.0.2 and saw no difference.

[tannergooding]

@riverar could you share what options the codegen is being generated with?

TerraFX also uses ClangSharp and it is being generated correctly there: https://source.terrafx.dev/#TerraFX.Interop.Windows/Windows/um/winnt/ACTIVATION_CONTEXT_COMPATIBILITY_INFORMATION.cs,001635dc53802eb8 (actual source is here: https://github.com/terrafx/terrafx.interop.windows/blob/main/sources/Interop/Windows/Windows/um/winnt/ACTIVATION_CONTEXT_COMPATIBILITY_INFORMATION.cs)

There is also correspondingly a test covering this for the "Windows compatible codegen" that I believe microsoft/win32metadata is using: https://github.com/dotnet/ClangSharp/blob/main/tests/ClangSharp.PInvokeGenerator.UnitTests/CSharpCompatibleWindows/StructDeclarationTest.cs#L13-L32

It's notably covering primitive types, but at a glance I don't see any difference for other struct types.

[riverar]

@tannergooding Here's the base set of options

--additional
-Wno-expansion-to-defined
-Wno-ignored-attributes
-Wno-ignored-pragma-intrinsic
-Wno-nonportable-include-path
-Wno-pragma-pack
-Wno-comment
-Wno-extra-tokens
-Wno-unused-value
-Wno-#pragma-messages
-Wno-deprecated-declarations
-Wno-extern-c-compat
-Wno-extern-initializer
-D_USE_DECLSPECS_FOR_SAL=1
-D_PREFAST_=1
-DWINAPI_FAMILY=WINAPI_FAMILY_DESKTOP_APP
-DMICROSOFT_WINDOWS_WINBASE_H_DEFINE_INTERLOCKED_CPLUSPLUS_OVERLOADS=0
-D_ALLOW_COMPILER_AND_STL_VERSION_MISMATCH=1
-DUCHAR_TYPE=wchar_t
--config
compatible-codegen
log-visited-files
log-exclusions
log-potential-typedef-remappings
exclude-funcs-with-body
generate-cpp-attributes
generate-native-inheritance-attribute
dont-use-using-statics-for-enums
exclude-anonymous-field-helpers
--methodClassName
Apis

[tannergooding]

Looks like a post-processing artifact of the win32metadata repo or maybe some other config knob or difference getting in the way...

I just tested with a GenerateLocal.rsp

--additional
-Wno-expansion-to-defined
-Wno-ignored-attributes
-Wno-ignored-pragma-intrinsic
-Wno-nonportable-include-path
-Wno-pragma-pack
-Wno-comment
-Wno-extra-tokens
-Wno-unused-value
-Wno-#pragma-messages
-Wno-deprecated-declarations
-Wno-extern-c-compat
-Wno-extern-initializer
-D_USE_DECLSPECS_FOR_SAL=1
-D_PREFAST_=1
-DWINAPI_FAMILY=WINAPI_FAMILY_DESKTOP_APP
-DMICROSOFT_WINDOWS_WINBASE_H_DEFINE_INTERLOCKED_CPLUSPLUS_OVERLOADS=0
-D_ALLOW_COMPILER_AND_STL_VERSION_MISMATCH=1
-DUCHAR_TYPE=wchar_t
--config
compatible-codegen
log-visited-files
log-exclusions
log-potential-typedef-remappings
exclude-funcs-with-body
generate-cpp-attributes
generate-native-inheritance-attribute
dont-use-using-statics-for-enums
exclude-anonymous-field-helpers
--methodClassName
Apis
--namespace
Test
--include-directory
C:/Program Files (x86)/Windows Kits/10/Include/10.0.22621.0/shared
C:/Program Files (x86)/Windows Kits/10/Include/10.0.22621.0/um
C:/Program Files (x86)/Windows Kits/10/Include/10.0.22621.0/winrt
--output
./gen/um/winnt.cs
--traverse
"C:/Program Files (x86)/Windows Kits/10/Include/10.0.22621.0/um/winnt.h"
--file
"C:/Program Files (x86)/Windows Kits/10/Include/10.0.22621.0/um/windows.h"

And the command line ClangSharpPInvokeGenerator.exe @GenerateLocal.rsp

That generates one file winnt.cs with the following definition on L13872:

    public partial struct _ACTIVATION_CONTEXT_COMPATIBILITY_INFORMATION
    {
        [NativeTypeName("DWORD")]
        public uint ElementCount;

        [NativeTypeName("COMPATIBILITY_CONTEXT_ELEMENT[]")]
        public _Elements_e__FixedBuffer Elements;

        public partial struct _Elements_e__FixedBuffer
        {
            public _COMPATIBILITY_CONTEXT_ELEMENT e0;

            public unsafe ref _COMPATIBILITY_CONTEXT_ELEMENT this[int index]
            {
                get
                {
                    fixed (_COMPATIBILITY_CONTEXT_ELEMENT* pThis = &e0)
                    {
                        return ref pThis[index];
                    }
                }
            }
        }
    }

[riverar]

@tannergooding Note the struct you pasted is missing unsafe

[tannergooding]

@riverar, yes. It also contains no unsafe code. Note that the two fields are uint and _Elements_e__FixedBuffer, which is a struct. The unsafe correctly exists on the public unsafe ref _COMPATIBILITY_CONTEXT_ELEMENT this[int index] which contains a fixed statement and thus requires it.

The win32metadata definition of ACTIVATION_CONTEXT_COMPATIBILITY_INFORMATION is incorrect, because it has the second field as a COMPATIBILITY_CONTEXT_ELEMENT*. It is not defined as a pointerin C/C++, but rather an inline buffer of type COMPATIBILITY_CONTEXT_ELEMENT with "length == 0" (technically "no length", but that's the same as "length == 0").

This struct in general is not exactly representable in C#/.NET since we don't have a concept of "incomplete arrays" or "zero length" arrays. In C/C++, sizeof(ACTIVATION_CONTEXT_COMPATIBILITY_INFORMATION) is reported as 8 (sizeof(uint) + padding to account for "proper" alignment of COMPATIBILITY_CONTEXT_ELEMENT): godbolt

But when there is a non-zero amount of Elements, they appear inline and as trailing data which would not be possible with the win32metadata definition.

[riverar]

Ah I missed that inner unsafe, thanks! Will dig deeper, my snippet was output before any major transformation I believe.

[mikebattista]

The below remap is changing COMPATIBILITY_CONTEXT_ELEMENT[].

win32metadata/generation/WinSDK/scraper.settings.rsp

Line 157 in dc15d90

COMPATIBILITY_CONTEXT_ELEMENT[]=COMPATIBILITY_CONTEXT_ELEMENT*

Do we just need to remove this?

[mikebattista]

I've got win32metadata generating the same definition now as @tannergooding. Is that what we want here?

[tannergooding]

Is that what we want here?

Should be. ClangSharp was doing the right thing, ABI wise. Not aware of any actual bugs here.