Support [AssociatedEnum] on UINT* pointer ([Out]/return) types

[MarijnS95]

Summary

For microsoft/win32metadata#1757

DXGI has some UINT* return fields that should be typed/associated with the relevant enumeration type in win32metadata. Overwriting the UINT* type with a pointer to an enum type currently generates valid code, but it was recommended to use AssociatedEnum (for the same reason as it is used on UINT and other types). This however does not currently generate valid code.

Pulling the original report from microsoft/win32metadata#1757 (comment):

Having AssociatedEnum on these (out) pointer types breaks windows-rs completely:

Traverse-Research@ea1c327

Out values are no longer returned from functions but are passed by value to the function, where its value is cast to a pointer.

Fwiw:

Windows.Win32.Graphics.Dxgi.IDXGIOutput3.CheckOverlaySupport : pFlags : [Out] => [AssociatedEnum(DXGI_OVERLAY_SUPPORT_FLAG),Out]
Windows.Win32.Graphics.Dxgi.IDXGIOutput4.CheckOverlayColorSpaceSupport : pFlags : [Out] => [AssociatedEnum(DXGI_OVERLAY_COLOR_SPACE_SUPPORT_FLAG),Out]
Windows.Win32.Graphics.Dxgi.IDXGIOutput6.CheckHardwareCompositionSupport : pFlags : [Out] => [AssociatedEnum(DXGI_HARDWARE_COMPOSITION_SUPPORT_FLAGS),Out]
Windows.Win32.Graphics.Dxgi.IDXGISwapChain3.CheckColorSpaceSupport : pColorSpaceSupport : [Out] => [AssociatedEnum(DXGI_SWAP_CHAIN_COLOR_SPACE_SUPPORT_FLAG),Out]

Taking the following metadata of one of these functions:

unsafe HRESULT CheckColorSpaceSupport([In] DXGI_COLOR_SPACE_TYPE ColorSpace, [Out][AssociatedEnum("DXGI_SWAP_CHAIN_COLOR_SPACE_SUPPORT_FLAG")] uint* pColorSpaceSupport);

And running tool_windows over it, the signature is changed in a breaking way:

1. Now expected as argument instead;
2. Its raw value is cast to a pointer;
3. Nothing is returned to the caller:

-    pub unsafe fn CheckColorSpaceSupport(&self, colorspace: Common::DXGI_COLOR_SPACE_TYPE) -> ::windows_core::Result<DXGI_SWAP_CHAIN_COLOR_SPACE_SUPPORT_FLAG> {
-        let mut result__ = ::std::mem::zeroed();
-        (::windows_core::Interface::vtable(self).CheckColorSpaceSupport)(::windows_core::Interface::as_raw(self), colorspace, &mut result__).from_abi(result__)
+    pub unsafe fn CheckColorSpaceSupport(&self, colorspace: Common::DXGI_COLOR_SPACE_TYPE, pcolorspacesupport: DXGI_SWAP_CHAIN_COLOR_SPACE_SUPPORT_FLAG) -> ::windows_core::Result<()> {
+        (::windows_core::Interface::vtable(self).CheckColorSpaceSupport)(::windows_core::Interface::as_raw(self), colorspace, pcolorspacesupport.0 as _).ok()
    }

If the win32metadata project goes ahead with annotating UINT* types with [AssociatedEnum], windows-bindgen etc should be updated to support this.

[kennykerr]

It's not clear what the question or bug is here.

From what I can tell you're still discussing this over in the win32 metadata repo. If it's a metadata issue, then we can close this issue and focus on resolving it there.

[riverar]

@kennykerr I believe the question here is whether or not bindgen supports (or should support) metadata AssociatedEnum attributes on [out] pointers. I suspect this is not supported today, as pointed out in my comment over there.

Looks like Marijn's winmd contains:

unsafe HRESULT CheckOverlayColorSpaceSupport(
  [In] DXGI_FORMAT Format,
  [In] DXGI_COLOR_SPACE_TYPE ColorSpace,
  [In] IUnknown pConcernedDevice,
  [AssociatedEnum(DXGI_OVERLAY_SUPPORT_FLAG),Out] uint* pFlags);

And the expectation is that retval signature transform continues working but is AssociatedEnum-aware:

CheckOverlayColorSpaceSupport(...) -> Result<DXGI_OVERLAY_SUPPORT_FLAG>

It's not clear if this is something we should/want to support on the metadata side yet.

[MarijnS95]

The generated metadata for the example function is as follows:

[Documentation("https://learn.microsoft.com/windows/win32/api/dxgi1_4/nf-dxgi1_4-idxgiswapchain3-checkcolorspacesupport")]
unsafe HRESULT CheckColorSpaceSupport([In] DXGI_COLOR_SPACE_TYPE ColorSpace, [Out][AssociatedEnum("DXGI_SWAP_CHAIN_COLOR_SPACE_SUPPORT_FLAG")] uint* pColorSpaceSupport);

Seeing as the metadata is generating something apparently sensible, it's neither a question nor a bug report but a feature request for windows-rs to support such AssociatedEnum annotations on UINT*. Flying close to a bug report as it is silently generating code that does the wrong thing.

[riverar]

We should be clear though that metadata has not indicated this is something that'll get merged in, so this feature request/bug report is premature.

[MarijnS95]

@riverar your request to use AnnotatedEnum on UINT* (as I already suggested when opening that PR) sounded like that is the desired way to go, here I'm just pointing out that it isn't actually working in windows-rs.

[kennykerr]

For future reference. Win32 metadata already employs this attribute in this way in the case of IClientSecurity.

https://github.com/microsoft/windows-rs/compare/PrimitiveOrEnum?expand=1

[MarijnS95]

@kennykerr thanks, looks like you started fixing this and found some drive-by metadata changes that are also affected?

I was trying your changes out on top of a rebased metadata branch with the [AnnotatedEnum] changes from microsoft/win32metadata#1757 on e4603e1, but it fails with:

thread '<unnamed>' panicked at crates\libs\metadata\src\type.rs:126:18:
`deref` can only be called on pointer types: PrimitiveOrEnum(MutPtr(U32, 1), TypeDef(TypeDef(Row { file: 0x1fa3b009040, index: 8247 }), []))

[kennykerr]

Yes, deref support for PrimitiveOrEnum will be required for retval parameters.

[MarijnS95]

@kennykerr sounds good, I'll leave that to you but feel free to play with the .winmd from that branch if necessary.

[kennykerr]

Closing as duplicate of #2784 to keep the conversation in one place.

[MarijnS95]

@kennykerr that's strange. This issue was first, and #2784 mentions nothing about pointers to enum variants.

[kennykerr]

More folks are engaged on that thread. Feel free to bring up your particular scenario.

[MarijnS95]

Feel free to bring up your particular scenario.

Its not up to me to cross-post my report when you've deduced that the content is already represented there (== duplicate) and hence close this.

I'm afraid this issue otherwise gets forgotten about, while @riverar already stated that this is a change to go ahead with in the metadata: microsoft/win32metadata#1757 (comment). Once that lands, windows-rs will generate incorrect code when importing newer .winmd.

[kennykerr]

This should avoid code gen issues: #2797

[MarijnS95]

@kennykerr thanks! It looks like some things are still off:

https://github.com/Traverse-Research/windows-rs/compare/regen-agility-711-preview

I have a few stages of metadata, the latest one of the branch comes from microsoft/win32metadata#1757 and includes:

# Associate DXGI function parameters and struct fields with the corresponding enum type^M
Windows.Win32.Graphics.Dxgi.DXGI_ADAPTER_DESC1.Flags :  => [AssociatedEnum(DXGI_ADAPTER_FLAG)]^M
Windows.Win32.Graphics.Dxgi.DXGI_ADAPTER_DESC2.Flags :  => [AssociatedEnum(DXGI_ADAPTER_FLAG)]^M
Windows.Win32.Graphics.Dxgi.DXGI_DECODE_SWAP_CHAIN_DESC.Flags :  => [AssociatedEnum(DXGI_SWAP_CHAIN_FLAG)]^M
Windows.Win32.Graphics.Dxgi.DXGI_SWAP_CHAIN_DESC.Flags :  => [AssociatedEnum(DXGI_SWAP_CHAIN_FLAG)]^M
Windows.Win32.Graphics.Dxgi.IDXGIDevice4.OfferResources1 : Flags : [In] => [AssociatedEnum(DXGI_OFFER_RESOURCE_FLAGS),In]^M
Windows.Win32.Graphics.Dxgi.IDXGIOutput3.CheckOverlaySupport : pFlags : [Out] => [AssociatedEnum(DXGI_OVERLAY_SUPPORT_FLAG),Out]^M
Windows.Win32.Graphics.Dxgi.IDXGIOutput4.CheckOverlayColorSpaceSupport : pFlags : [Out] => [AssociatedEnum(DXGI_OVERLAY_COLOR_SPACE_SUPPORT_FLAG),Out]^M
Windows.Win32.Graphics.Dxgi.IDXGIOutput6.CheckHardwareCompositionSupport : pFlags : [Out] => [AssociatedEnum(DXGI_HARDWARE_COMPOSITION_SUPPORT_FLAGS),Out]^M
Windows.Win32.Graphics.Dxgi.IDXGISwapChain.ResizeBuffers : SwapChainFlags : [In] => [AssociatedEnum(DXGI_SWAP_CHAIN_FLAG),In]^M
Windows.Win32.Graphics.Dxgi.IDXGISwapChain3.CheckColorSpaceSupport : pColorSpaceSupport : [Out] => [AssociatedEnum(DXGI_SWAP_CHAIN_COLOR_SPACE_SUPPORT_FLAG),Out]^M
Windows.Win32.Graphics.Dxgi.IDXGISwapChain3.ResizeBuffers1 : SwapChainFlags : [In] => [AssociatedEnum(DXGI_SWAP_CHAIN_FLAG),In]^M

Produces:

1. A pointer/borrow where it should be a value:

   -fn ResizeBuffers(&self, buffercount: u32, width: u32, height: u32, newformat: Common::DXGI_FORMAT, swapchainflags: u32) -> ::windows_core::Result<()>;
   +fn ResizeBuffers(&self, buffercount: u32, width: u32, height: u32, newformat: Common::DXGI_FORMAT, swapchainflags: &DXGI_SWAP_CHAIN_FLAG) -> ::windows_core::Result<()>;

2. Functions that should return an enum still return u32:

   fn CheckColorSpaceSupport(&self, colorspace: Common::DXGI_COLOR_SPACE_TYPE) -> ::windows_core::Result<u32>;

   (If you look at the commit history, this was temporarily fixed on "bad" metadata that mapped the parameters to a DXGI_SWAP_CHAIN_COLOR_SPACE_SUPPORT_FLAG * type, rather than using AssociatedEnum.

Can you look into this?

[kennykerr]

As always, we'll address any issues when the new metadata is available.

[MarijnS95]

Just telling you up-front, but I'll have to wait for @mikebattista / @riverar to push forward on microsoft/win32metadata#1757.

[riverar]

Heads up, I filed a GitHub ticket against this thread so they may drop by and post a comment or two for testing. It appears emails from this thread are generating notification emails that are failing DKIM signature checks.

(GH ticket https://support.github.com/ticket/personal/0/2533535)

[MarijnS95]

Hey, just letting you all know that microsoft/win32metadata#1757 was merged and is part of the win32metadata v60.0.34-preview release. Looking forward to seeing that used in windows-rs!