[Bug 54226] New: widl: implement `cstruct_out` option to generate C bindings compatible with small struct return values
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: wine-bugs(a)winehq.org Reporter: alvin(a)alvinhc.com 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.) -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=54226 Zeb Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |z.figura12(a)gmail.com --- Comment #1 from Zeb Figura <z.figura12(a)gmail.com> --- (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? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=54226 --- Comment #2 from Alvin Wong <alvin(a)alvinhc.com> --- (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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
WineHQ Bugzilla