[PATCH v3 0/13] MR6207: include: Introduce new COM implementation helper macros.

Rémi Bernon

7 Aug 2024 7 Aug '24

10:17 a.m.

I think it would be very nice to have something like that to reduce the burden of implementing COM interfaces. This shows for instance on the windows.gaming.input module a 30% LoC reduction (from ~6800 to ~4700), even though the module already had a boilerplate helper macros already. The MR introduces macros to automatically implement each IUnknown method, as well as all of them at once. It also includes a helper to implement IInspectable methods at once, as well as macros to forward both interface methods to a base interface or an outer object. Last, it provides higher-level macros to implement a main interface and any number of sub interfaces and generate IUnknown, forwarding and vtables for all of them at once, with IInspectable support when needed. It uses widl to generate additional per-interface macros, for things like inheritance and vtable generation. The rest of the macros are otherwise shared in a Wine-specific header. The implementation is split to show individual macro being used, although they are later replaced by higher-level macros. The individual helpers are still useful in some corner cases where specific behavior needs to be implemented, or for aggregating classes. -- v3: include: Add a parameter to COM helper END to control QI traces. windows.gaming.input: Use the new COM macros everywhere possible. include: Introduce new macros for COM heritage implementation. widl: Generate some INTERFACE_IMPL/FWD helper macros. include: Introduce new macros for IInspectable WinRT implementation. include: Introduce new macros for COM IUnknown implementation. include: Introduce new macros for COM QueryInterface implementation. widl: Generate some QUERY_INTERFACE helper macros. include: Introduce new macros for COM AddRef/Release implementation. include: Introduce new helper macros for COM implementation. windows.gaming.input: Use the new INTERFACE_VTBL macros. widl: Generate some INTERFACE_VTBL helper macros. windows.gaming.input: Use a separate interface for IAgileObject. https://gitlab.winehq.org/wine/wine/-/merge_requests/6207

Show replies by date

Rémi Bernon

7 Aug 7 Aug

10:17 a.m.

New subject: [PATCH v3 13/13] include: Add a parameter to COM helper END to control QI traces.

3:42 p.m.

...

widl could generate 'implementation' source file based on some input IDL with something like coclasses desribing implemented objects. This source file could define all vtbls, forward shared functions, maybe even default `IUnknown`-alike implementation or class factories. (It would probably also need to generate a header file that the rest of module would use to access it, or the whole thing could just be a header). Anyway, I'm not sure, just throwing another idea for a consideration.

I thought about something like that also, although never really got to it, as I thought it would also be interesting to make mock class creation easier for testing. However I ended up with the idea that it wouldn't be very flexible to have it generate everything, and that it would add annoying indirection or hide some details. For instance with this approach here, I find it convenient that it allows you to selectively replace functions with generated ones through fine-grained macros. It can be useful for some exotic or specific use cases, COM aggregation for instance (although that could also be supported). It also only requires a couple of naming conventions in the implementation structs, but doesn't add any kind of indirection there, you still see all the details of the implementation, only the actual boiler place functions are hidden within macros. What I don't like much is the use of macros, which doesn't make it very easy to have a look at how the generated code actually looks like (well unless you have editor plugins that expand them for you), but I'm not sure it's a big issue. Alternatively we could perhaps use things like #pragma makedep instead, but although it could generate things in clear text in an annex file, it would be more misleading to the uninformed reader. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6207#note_80882

Elizabeth Figura (＠zfigura)

14 Nov 14 Nov

6:28 p.m.

New subject: [PATCH v3 0/13] MR6207: include: Introduce new COM implementation helper macros. - approved

This merge request was approved by Elizabeth Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6207

Vibhav Pant (＠vibhavp)

7:13 p.m.

Just out of curiosity, what do you think about an approach that instead creates a`QITAB` with an entry for every inherited interface in `write_widl_impl_macros_methods`, so that the final \`QueryInterface\` can be done with a `QISearch`? Would it reduce macro usage? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6207#note_87514

Rémi Bernon

18 Nov 18 Nov

11:02 a.m.

On Mon Nov 18 11:02:49 2024 +0000, Vibhav Pant wrote:

...

Just out of curiosity, what do you think about an approach that instead creates a`QITAB` with an entry for every inherited interface in `write_widl_impl_macros_methods`, so that the final \`QueryInterface\` can be done with a `QISearch`? Would it reduce macro usage? I'm not sure what you mean by `QITAB`, but implementing QueryInterface is just one small piece of macro usage. The main source is probably the higher level macro that generates all the boilerplates (ie: vtables and forwards for every implemented interfaces of a given class).

-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6207#note_87834

Jacek Caban (＠jacek)

12:46 p.m.

On Mon Nov 18 12:46:38 2024 +0000, Rémi Bernon wrote:

...

...
widl could generate 'implementation' source file based on some input IDL with something like coclasses desribing implemented objects. This source file could define all vtbls, forward shared functions, maybe even default `IUnknown`-alike implementation or class factories. (It would probably also need to generate a header file that the rest of module would use to access it, or the whole thing could just be a header). Anyway, I'm not sure, just throwing another idea for a consideration. I thought about something like that also, although never really got to it, as I thought it would also be interesting to make mock class creation easier for testing. However I ended up with the idea that it wouldn't be very flexible to have it generate everything, and that it would add annoying indirection or hide some details. For instance with this approach here, I find it convenient that it allows you to selectively replace functions with generated ones through fine-grained macros. It can be useful for some exotic or specific use cases, COM aggregation for instance (although that could also be supported). It also only requires a couple of naming conventions in the implementation structs, but doesn't add any kind of indirection there, you still see all the details of the implementation, only the actual boiler place functions are hidden within macros. What I don't like much is the use of macros, which doesn't make it very easy to have a look at how the generated code actually looks like (well unless you have editor plugins that expand them for you), but I'm not sure it's a big issue. Alternatively we could perhaps use things like #pragma makedep instead, but although it could generate things in clear text in an annex file, it would be more misleading to the uninformed reader. We could at least move those macros to private generated headers instead of extending public ones.

-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6207#note_87836

Vibhav Pant (＠vibhavp)

1:24 p.m.

On Mon Nov 18 11:02:49 2024 +0000, Rémi Bernon wrote:

...

I'm not sure what you mean by `QITAB`, but implementing QueryInterface is just one small piece of macro usage. The main source is probably the higher level macro that generates all the boilerplates (ie: vtables and forwards for every implemented interfaces of a given class). Here's a rough example of what I meant. Take `IWineForceFeedbackEffectImpl` for instance:

``` interface IWineForceFeedbackEffectImpl : IInspectable requires Windows.Gaming.Input.ForceFeedback.IForceFeedbackEffect ```

...

From this, widl would create something like (using unmangled interface names for clarity):

``` #define INTERFACES_IWineForceFeedbackEffectImpl(type, ...) \ const static QITAB QITAB_IWineForceFeedbackEffectImpl[] = \ { \ {&IID_IInspectable, offsetof( type, IWineForceFeedbackEffectImpl_iface )}, \ {&IID_IForceFeedbackEffect, offsetof( type, IForceFeedbackEffect_iface )}, \ __VA_ARGS__ \ }; ``` With the following definitions in `wine/comimpl.h`: ``` #define QITAB_ENTRY(type, X) {&IID##_x, offsetof(type, X##_face)}, #define QITAB_END {NULL, 0} ... #define IUNKNOWN_IMPL_QUERY_INTERFACE_( type, name, impl_from, log ) \ static HRESULT WINAPI type ## _QueryInterface( name *iface, REFIID iid, void **out ) \ { \ struct type *object = impl_from( iface ); \ HRESULT hres; TRACE( "object %p, iid %s, out %p.\n", object, debugstr_guid(iid), out ); \ hres = QISearch(object, QITAB##_name, iid, out); if (hres == E_NOINTERFACE) (log)( "%s not implemented, returning E_NOINTERFACE.", debugstr_guid(iid)); return hres; } ``` [This line](https://gitlab.winehq.org/wine/wine/-/merge_requests/6207/diffs#1645e0df6b8e...) in `force_feedback.c` could be written as ``` INTERFACE_IMPL_OUTER_IWineForceFeedbackEffectImpl( effect, QITAB_ENTRY(IAgileObject), QITAB_END, FIXME ); ``` Which would then expand to the `QITAB` array above + remaining macros. The idea is that this would replace all `QUERY_INTERFACE{S}{_OPT}` macros with one `QITAB` array, simplify `QueryInterface`'s code, and also provide a convenient way to implement `GetIids`, if we ever need to in the future (as we now have an array of all known IIDs for the object). `QISearch` is already in kernelbase, so this shouldn't add any new dependencies if I understand correctly. It also makes it mandatory for the implementation to implement the interfaces annotated with `requires`. (I do realize now that it doesn't really reduce macro usage all that much, but I'd still be interested in knowing what you think about this :P) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6207#note_87839

388

Age (days ago)

491

Last active (days ago)

List overview

Download

22 comments

4 participants

participants (4)

Elizabeth Figura (＠zfigura)
Jacek Caban (＠jacek)
Rémi Bernon
Vibhav Pant (＠vibhavp)

[PATCH v3 0/13] MR6207: include: Introduce new COM implementation helper macros.

tags

participants (4)