windows-core: declarative interface_decl! macro_rules alternative to #[interface] - #4403
Conversation
Agent-Logs-Url: https://github.com/microsoft/windows-rs/sessions/8a414874-c962-4d7f-b56c-6ce85f57ae9c Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/windows-rs/sessions/f08f93eb-2759-4f4f-878b-75426596dab0 Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/windows-rs/sessions/e0730650-bb06-4596-8420-1f276817d6b6 Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/windows-rs/sessions/e0730650-bb06-4596-8420-1f276817d6b6 Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/windows-rs/sessions/219bbb2d-2854-4941-8cc8-91286a9caf38 Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/windows-rs/sessions/8a9ad3f6-dae1-44f1-806c-212b655ac246 Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
…ows-future Agent-Logs-Url: https://github.com/microsoft/windows-rs/sessions/8a9ad3f6-dae1-44f1-806c-212b655ac246 Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/windows-rs/sessions/7a871c10-9203-4136-b60d-c556715fefb5 Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds macro_rules!-based alternatives to #[interface] / #[implement] in windows-core, migrates remaining hand-written implementers to the declarative macros, and adjusts feature flags so consumers can opt out of proc-macro dependencies.
Changes:
- Introduce
interface_decl!,implement_decl!, andimplement_decl_generic!inwindows-core. - Port several
windows-collections/windows-futuretypes from#[implement]to the new declarative macros. - Update Cargo feature wiring to make proc-macros optional and fix test/CI configuration.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/tests/misc/no_std/Cargo.toml | Enables windows-core/proc-macros for the no-std test crate. |
| crates/tests/misc/linux/Cargo.toml | Enables windows-core/proc-macros for the linux misc test crate. |
| crates/tests/libs/interface_core/tests/macro_rules_decl.rs | New tests exercising interface_decl!/implement_decl! interoperability and ABI basics. |
| crates/tests/libs/interface_core/Cargo.toml | Enables windows-core/proc-macros feature for interface_core tests. |
| crates/tests/libs/implement_core/Cargo.toml | Enables windows-core/proc-macros feature for implement_core tests. |
| crates/tests/libs/collections/Cargo.toml | Enables windows-core/proc-macros feature for collections tests. |
| crates/libs/windows/Cargo.toml | Changes default features to include windows-core/proc-macros. |
| crates/libs/future/src/async_spawn.rs | Ports async implementers from #[implement] to implement_decl!/implement_decl_generic!. |
| crates/libs/future/src/async_ready.rs | Ports ready-state implementers from #[implement] to implement_decl!/implement_decl_generic!. |
| crates/libs/future/Cargo.toml | Disables windows-core default features for windows-future. |
| crates/libs/core/src/lib.rs | Adds new macro modules and gates proc-macro re-exports behind proc-macros feature. |
| crates/libs/core/src/interface_macro.rs | Adds interface_decl! + helpers to declare interfaces without proc macros. |
| crates/libs/core/src/implement_macro.rs | Adds implement_decl! + helpers to wire non-generic implementers without proc macros. |
| crates/libs/core/src/implement_generic_macro.rs | Adds implement_decl_generic! for generic implementers without proc macros. |
| crates/libs/core/Cargo.toml | Makes proc-macro deps optional and adds proc-macros feature (enabled by default). |
| crates/libs/collections/src/vector_view.rs | Ports #[implement] sites to implement_decl_generic!. |
| crates/libs/collections/src/vector.rs | Ports #[implement] sites to implement_decl_generic!. |
| crates/libs/collections/src/observable_vector.rs | Ports #[implement] sites to implement_decl_generic!/implement_decl!. |
| crates/libs/collections/src/observable_map.rs | Ports #[implement] sites to implement_decl_generic!. |
| crates/libs/collections/src/map_view.rs | Ports #[implement] sites to implement_decl_generic!. |
| crates/libs/collections/src/map.rs | Ports #[implement] sites to implement_decl_generic!. |
| crates/libs/collections/src/key_value_pair.rs | Ports #[implement] sites to implement_decl_generic!. |
| crates/libs/collections/src/iterable.rs | Ports #[implement] sites to implement_decl_generic!. |
| crates/libs/collections/Cargo.toml | Disables windows-core default features for windows-collections. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $vis unsafe fn $mname(&self $(, $aname: $aty)*) -> $crate::Result<$res> { | ||
| unsafe { | ||
| ($crate::Interface::vtable(self).$mname)($crate::Interface::as_raw(self) $(, $aname)*).ok() | ||
| } | ||
| } |
| let this_outer: &Identity = unsafe { | ||
| &*((this as *const *const ()).offset(OFFSET) as *const Identity) | ||
| }; | ||
| unsafe { <Identity as $impl_trait>::$mname(this_outer $(, $aname)*) }.into() |
|
|
||
| [dependencies.windows-core] | ||
| workspace = true | ||
| features = ["proc-macros"] |
|
|
||
| [features] | ||
| default = ["std"] | ||
| default = ["std", "windows-core/proc-macros"] |
| macro_rules! __implement_decl_offset_negate { | ||
| (()) => { | ||
| -1isize | ||
| }; |
| -16isize | ||
| }; // Hand-written implementers rarely declare more than a handful of interfaces; the | ||
| // hard cap is more than the practical maximum. If you hit this, split your | ||
| // implementation across multiple objects or use the proc-macro. | ||
| } |
| macro_rules! __implement_decl_index_plus_two { | ||
| () => { | ||
| 2usize | ||
| }; |
| }; | ||
| (() () () () () () () () () () () () () () ()) => { | ||
| 17usize | ||
| }; |
| // Each interface needs an internal field/const name on the generated wrapper. The | ||
| // names are never user-visible (no item generated by this macro lets external code | ||
| // refer to them), so we draw them from a fixed anonymous pool. The pool size | ||
| // matches the cap in `__implement_decl_offset_negate!`. |
| // this macro accumulates four token lists (vtbl fields, vtbl initializers, thunk fn defs, | ||
| // and matches-arms for parent chain traversal — currently unused but reserved for a | ||
| // future extension) and emits the whole `struct $vtbl { ... } impl $vtbl { ... }` block | ||
| // at the end. |
Agent-Logs-Url: https://github.com/microsoft/windows-rs/sessions/cdafe004-6e87-4446-81e8-0368e00bb143 Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/windows-rs/sessions/2d9defe5-6e06-496b-b49f-24d5f5501d11 Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
…o.rs Agent-Logs-Url: https://github.com/microsoft/windows-rs/sessions/c3ad3f31-7c1f-4573-85d2-2b16189466f1 Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
| macro_rules! interface_decl { | ||
| ( | ||
| $(#[doc = $doc:expr])* | ||
| $vis:vis unsafe trait $name:ident ( $vtbl:ident, $impl_trait:ident ) : $parent:ty = $iid:literal { | ||
| $($methods:tt)* | ||
| } | ||
| ) => { | ||
| // Struct + Interface + Debug. | ||
| $crate::imp::define_interface!($name, $vtbl, $iid); | ||
| // CanInto + From conversions to ancestors. | ||
| $crate::imp::interface_hierarchy!($name, $parent); |
There was a problem hiding this comment.
Don't bother capturing docs at all.
There was a problem hiding this comment.
Done in 2268a7a — the doc capture is gone from interface_decl! and both helper macros.
| ($vis:vis, | ||
| $(#[doc = $mdoc:expr])* | ||
| unsafe fn $mname:ident (&self $(, $aname:ident : $aty:ty)* $(,)? ) -> $ret:ty ; | ||
| $($rest:tt)* | ||
| ) => { | ||
| $(#[doc = $mdoc])* | ||
| #[inline] | ||
| $vis unsafe fn $mname(&self $(, $aname: $aty)*) -> $ret { | ||
| unsafe { |
There was a problem hiding this comment.
Just remove these unused and untested arms.
There was a problem hiding this comment.
Removed in 2268a7a — only Result<()> and void return-type arms remain in the wrapper, trait-method, and vtable helpers. Updated the test to drop the value-returning method.
| #[repr(C)] | ||
| #[allow(non_camel_case_types, non_snake_case)] | ||
| $impl_vis struct $impl_name < $($gp),+ > | ||
| where $($wc)* | ||
| { |
There was a problem hiding this comment.
Let's just require the where clause and pass it through as is.
There was a problem hiding this comment.
Done in 2268a7a — the where clause is now mandatory and forwarded verbatim. Every existing call site already supplies one.
| //! The leading ident of every interface entry must be unique within a single invocation — | ||
| //! it doubles as the per-chain struct field name and as the name of the per-chain | ||
| //! associated constant on `Foo_Impl`. |
| $vis:vis unsafe trait $name:ident ( $vtbl:ident, $impl_trait:ident ) : $parent:ty = $iid:literal { | ||
| $($methods:tt)* | ||
| } | ||
| ) => { | ||
| // Struct + Interface + Debug. | ||
| $crate::imp::define_interface!($name, $vtbl, $iid); | ||
| // CanInto + From conversions to ancestors. | ||
| $crate::imp::interface_hierarchy!($name, $parent); |
There was a problem hiding this comment.
Just assuming pub as define_interface does is sufficient.
There was a problem hiding this comment.
Done in 2268a7a — $vis dropped from interface_decl!; the trait, struct, vtable, and wrapper fns are all pub unconditionally.
|
Copilot address new review comments. |
Agent-Logs-Url: https://github.com/microsoft/windows-rs/sessions/94b1f1da-fcc5-46a1-aca4-313bc3786f2c Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
Adds
macro_rules!-based alternatives to#[interface]/#[implement]inwindows-core, migrates the remaining hand-written generic implementers inwindows-collectionsandwindows-futureover to them, and makes the proc-macros optional so consumers can drop thesyn/quote/proc-macro2build dependencies.Changes Made
interface_decl!— declarative alternative to#[interface]for COM interfaces inheriting directly fromIUnknown. The generated interface struct, vtable,_Impltrait, and safe-caller wrappers are alwayspub(matchingdefine_interface!). Supported method shapes are-> Result<()>(vtable returnsHRESULT) and void; value-returning andResult<T>shapes are intentionally not accepted.implement_decl!— declarative alternative to#[implement]. Single macro with two top-level arms:impl Name as Vis Name_Impl: [Iface, …]impl<G,…> Name as Vis Name_Impl: [Iface<…>, …] where …(thewhereclause is mandatory and forwarded verbatim).All generic-form helpers live alongside the non-generic helpers in
implement_macro.rs(no separateimplement_generic_macro.rs). Vtable type for each interface entry is reached via<Iface as Interface>::Vtable, so callers don't spell out_Vtblidents.windows-collections(iterable,map,vector,observable_map,observable_vector,vector_view,map_view,key_value_pair) andwindows-future(async_ready,async_spawn) now usesimplement_decl!instead of bespoke unsafe wiring.windows-coregains a default-onproc-macrosfeature that gates the re-exports of#[implement]/#[interface].windows-collectionsandwindows-futuredisable default features so they no longer pull in the proc-macro toolchain. The top-levelwindowscrate keepswindows-core/proc-macrosenabled by default for backwards compatibility.crates/tests/libs/interface_core/tests/macro_rules_decl.rsexercises bothinterface_decl!andimplement_decl!end-to-end (multi-interface implementer, QueryInterface across the chain, refcount drop, IID matching).Testing
cargo check -p windows-core -p windows-collections -p windows-futurepassescargo test -p test_interface_core -p test_collectionspasses (4/4 inmacro_rules_decl)parallel_validation(Code Review + CodeQL) clean