[Bug 54226] New: widl: implement `cstruct_out` option to generate C bindings compatible with small struct return values

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54226

Bug ID: 54226
           Summary: widl: implement `cstruct_out` option to generate C
                    bindings compatible with small struct return values
           Product: Wine
           Version: unspecified
          Hardware: x86-64
                OS: Windows
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: tools
          Assignee: [e-mail]
          Reporter: [e-mail]

According to https://devblogs.microsoft.com/oldnewthing/20220113-00/?p=106152,
the previous C bindings are incompatible with the C++ COM ABI when the method
returns a small struct for MSVC. Their solution is to add the `/cstruct_out`
flag to the MIDL compiler
(https://learn.microsoft.com/en-us/windows/win32/midl/-cstruct-out, added in
the Win11 SDK according to comments) to generate new C bindings compatible with
the C++ COM ABI.

This is similar to the (long-standing) GCC issue with its mingw-w64 MS ABI
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64384), which has been worked
around in widl/mingw-w64 by having special wrapper functions in the C++
bindings, guarded by `WIDL_EXPLICIT_AGGREGATE_RETURNS`.

I think widl should follow suit to implement a `--cstruct_out` flag to do the
same thing for the C bindings. This does not need to be guarded by
`WIDL_EXPLICIT_AGGREGATE_RETURNS` because both GCC and MSVC has the same issue
and will require the same fix. (I do not know if we need some kind of guard for
the mingw-w64 headers to revert to the old function declarations, but given
that the old bindings are practically broken perhaps it doesn't matter.)

(I have not checked what `/cstruct_out` on MIDL actually does to the C
bindings.)

[Bug 54226] widl: implement `cstruct_out` option to generate C bindings compatible with small struct return values

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54226

Zeb Figura  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #1 from Zeb Figura  ---
(In reply to Alvin Wong from comment #0)
> (I have not checked what `/cstruct_out` on MIDL actually does to the C
> bindings.)

This, basically:

@@ -129,8 +129,9 @@ EXTERN_C const IID IID_ID2D1Bitmap;
             ID2D1Bitmap * This);

DECLSPEC_XFGVIRT(ID2D1Bitmap, b)
-        struct apple ( STDMETHODCALLTYPE *b )( 
-            ID2D1Bitmap * This);
+        struct apple *( STDMETHODCALLTYPE *b )( 
+            ID2D1Bitmap * This,
+            apple * RetVal);

END_INTERFACE
     } ID2D1BitmapVtbl;
@@ -158,8 +159,8 @@ EXTERN_C const IID IID_ID2D1Bitmap;
 #define ID2D1Bitmap_a(This)    \
     ( (This)->lpVtbl -> a(This) )

-#define ID2D1Bitmap_b(This)    \
-    ( (This)->lpVtbl -> b(This) ) 
+#define ID2D1Bitmap_b(This,RetVal)     \
+    ( (This)->lpVtbl -> b(This,RetVal) )

#endif /* COBJMACROS */

But we already emit the C type definition (the first hunk) with ABI
compatibility in all cases. For the macro we currently have e.g.

#define ID2D1Bitmap_a(This) (This)->lpVtbl->a(This)
#define ID2D1Bitmap_b(This)
ID2D1Bitmap_b_define_WIDL_C_INLINE_WRAPPERS_for_aggregate_return_support

but is there really any point to doing this? Do we care about API compatibility
if the API is just broken? Should we just implement the ABI-compatible
behaviour for the macros and not bother checking cstruct_out?

[Bug 54226] widl: implement `cstruct_out` option to generate C bindings compatible with small struct return values

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=54226

--- Comment #2 from Alvin Wong  ---
(In reply to Zeb Figura from comment #1)
> (In reply to Alvin Wong from comment #0)
> > (I have not checked what `/cstruct_out` on MIDL actually does to the C
> > bindings.)
>
> This, basically:
>
> [snip]

Thanks for checking.
> But we already emit the C type definition (the first hunk) with ABI
> compatibility in all cases.

Ah yes, so this part is fine.
> For the macro we currently have e.g.
>
> #define ID2D1Bitmap_a(This) (This)->lpVtbl->a(This)
> #define ID2D1Bitmap_b(This)
> ID2D1Bitmap_b_define_WIDL_C_INLINE_WRAPPERS_for_aggregate_return_support
>
> but is there really any point to doing this? Do we care about API
> compatibility if the API is just broken? Should we just implement the
> ABI-compatible behaviour for the macros and not bother checking cstruct_out?

That's true, a broken API is useless anyway. I don't mind for widl to just
generate the same form as `midl /cstruct_out`, so it would become

#define ID2D1Bitmap_b(This,__ret) (This)->lpVtbl->b(This,__ret)

and if we only consider this then we perhaps don't need a --cstruct_out option.
The existing "%s_%s_define_WIDL_C_INLINE_WRAPPERS_for_aggregate_return_support"
obviously doesn't work.

But what about the inline functions defined when `WIDL_C_INLINE_WRAPPERS` is
defined? Changing this bit may actually break existing code:
https://github.com/mingw-w64/mingw-w64/blob/a57cacad31f5766d45959b6c0f9b1229...

---

I _think_ the intention of the new flag in the case of MSVC would be to
maintain backward compatibility for when someone has implemented their own COM
objects with the vtable in C and only use them from C, in which case the
"broken" behaviour works fine and the new "correct" behaviour would actually
break things. This does not apply to widl and mingw-w64 because widl has
already been emitting the "correct" C vtable.