From fdb5c6bfe6ea75d31ae36c12e7abb26e2e78d5d7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 4 May 2026 18:00:00 +0000 Subject: [PATCH 1/4] Initial plan From 332a9a6d35a8f36f6cab46235fadca02714bdcf6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 4 May 2026 18:13:25 +0000 Subject: [PATCH 2/4] Add `--implements` option to scope `_Impl` scaffold emission Agent-Logs-Url: https://github.com/microsoft/windows-rs/sessions/bbdff62b-17d1-4f21-89d9-39619bdc6d80 Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com> --- crates/libs/bindgen/src/config/mod.rs | 18 ++++++ crates/libs/bindgen/src/implements.rs | 41 ++++++++++++ crates/libs/bindgen/src/lib.rs | 41 ++++++++++++ .../libs/bindgen/src/types/cpp_interface.rs | 2 + crates/libs/bindgen/src/types/interface.rs | 2 +- .../data/bindgen/implements/expected.rs | 62 +++++++++++++++++++ .../data/bindgen/implements/fixture.toml | 5 ++ .../harness/data/bindgen/implements/input.rdl | 9 +++ .../tests/fixtures/harness/tests/fixtures.rs | 10 +++ 9 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 crates/libs/bindgen/src/implements.rs create mode 100644 crates/tests/fixtures/harness/data/bindgen/implements/expected.rs create mode 100644 crates/tests/fixtures/harness/data/bindgen/implements/fixture.toml create mode 100644 crates/tests/fixtures/harness/data/bindgen/implements/input.rdl diff --git a/crates/libs/bindgen/src/config/mod.rs b/crates/libs/bindgen/src/config/mod.rs index 8d94a97e0c..41d9766a61 100644 --- a/crates/libs/bindgen/src/config/mod.rs +++ b/crates/libs/bindgen/src/config/mod.rs @@ -25,6 +25,7 @@ pub struct Config<'a> { pub sys_fn_ptrs: bool, pub sys_fn_extern: bool, pub implement: bool, + pub implements: &'a Implements, pub noexcept: bool, pub specific_deps: bool, pub derive: &'a Derive, @@ -39,6 +40,23 @@ impl Config<'_> { clone.namespace = namespace; clone } + + /// Returns `true` if the `_Impl` scaffolding for the given type should be + /// emitted, based on the `--implement` and `--implements` options. + /// + /// `default` is the behavior to fall back on when neither option restricts + /// emission: `true` for types that are emitted unconditionally today (such + /// as COM/Win32 interfaces) and `false` (or some other condition such as + /// `!is_exclusive`) for WinRT interfaces. + pub fn should_implement(&self, name: TypeName, default: bool) -> bool { + if self.implement { + true + } else if !self.implements.is_empty() { + self.implements.matches(name) + } else { + default + } + } } impl<'a> Config<'a> { diff --git a/crates/libs/bindgen/src/implements.rs b/crates/libs/bindgen/src/implements.rs new file mode 100644 index 0000000000..021db45289 --- /dev/null +++ b/crates/libs/bindgen/src/implements.rs @@ -0,0 +1,41 @@ +use super::*; + +/// Matcher used by the `--implements` option to determine whether the `_Impl` +/// scaffolding for a given type should be emitted. +/// +/// Patterns may be a fully-qualified type name (`Namespace.Name`) or a +/// namespace prefix (which matches every type under that namespace). +#[derive(Debug, Default)] +pub struct Implements(Vec); + +impl Implements { + pub fn new(patterns: &[&str]) -> Self { + Self(patterns.iter().map(|s| s.to_string()).collect()) + } + + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + pub fn matches(&self, name: TypeName) -> bool { + self.0 + .iter() + .any(|rule| match_type_name(rule, name.namespace(), name.name())) + } +} + +fn match_type_name(rule: &str, namespace: &str, name: &str) -> bool { + if rule.len() <= namespace.len() { + return namespace_starts_with(namespace, rule); + } + + if !rule.starts_with(namespace) { + return false; + } + + if rule.as_bytes()[namespace.len()] != b'.' { + return false; + } + + name == &rule[namespace.len() + 1..] +} diff --git a/crates/libs/bindgen/src/lib.rs b/crates/libs/bindgen/src/lib.rs index b0e324ac4a..37d59de678 100644 --- a/crates/libs/bindgen/src/lib.rs +++ b/crates/libs/bindgen/src/lib.rs @@ -10,6 +10,7 @@ mod derive; mod derive_writer; mod filter; mod guid; +mod implements; mod index; mod io; mod libraries; @@ -31,6 +32,7 @@ use derive::*; use derive_writer::*; use filter::*; use guid::*; +use implements::*; use io::*; pub use libraries::*; use param::*; @@ -86,6 +88,7 @@ pub fn builder() -> Bindgen { /// | `--sys-fn-ptrs` | Additionally generates function pointers for sys-style Rust bindings. | /// | `--sys-fn-extern` | Generates extern declarations rather than link macros for sys-style Rust bindings. | /// | `--implement` | Includes implementation traits for WinRT interfaces. | +/// | `--implements` | Includes implementation traits for the listed types only. | /// | `--noexcept` | Assumes all WinRT methods do not raise exceptions. | /// | `--link` | Overrides the default `windows-link` implementation for system calls. | /// @@ -363,6 +366,7 @@ where "--implement" => { builder.implement(); } + "--implements" => kind = ArgKind::Implements, "--noexcept" => { builder.noexcept(); } @@ -394,6 +398,9 @@ where ArgKind::Derive => { builder.derive(arg); } + ArgKind::Implements => { + builder.implements(arg); + } ArgKind::Rustfmt => { builder.rustfmt(arg); } @@ -434,6 +441,7 @@ pub struct Bindgen { output: String, references: Vec, derive: Vec, + implements: Vec, rustfmt: String, link: String, flat: bool, @@ -597,6 +605,35 @@ impl Bindgen { self } + /// Add a type to the list of types for which implementation scaffolding + /// should be emitted. + /// + /// Each entry may be a fully-qualified type name (`Namespace.Name`) or a + /// namespace prefix that matches every type defined under it. When at + /// least one entry is provided and `--implement` is **not** set, the + /// `_Impl` scaffolding is generated only for types matching the list, + /// rather than for every interface/class in the filter set. This is a + /// finer-grained alternative to the broad `--implement` switch and can + /// significantly reduce build time when only a handful of interfaces + /// need to be implemented. + pub fn implements(&mut self, name: &str) -> &mut Self { + self.implements.push(name.to_string()); + self + } + + /// Add multiple types to the list of types for which implementation + /// scaffolding should be emitted. See [`Bindgen::implements`]. + pub fn implements_iter(&mut self, names: I) -> &mut Self + where + I: IntoIterator, + S: AsRef, + { + for name in names { + self.implements.push(name.as_ref().to_string()); + } + self + } + /// Assume that all WinRT methods do not raise exceptions, regardless of whether /// they have the `NoExceptionAttribute`. This causes bindgen to emit infallible /// signatures for HRESULT-returning WinRT methods, skipping `Result` propagation. @@ -765,11 +802,13 @@ impl Bindgen { } let derive_str: Vec<&str> = self.derive.iter().map(|s| s.as_str()).collect(); + let implements_str: Vec<&str> = self.implements.iter().map(|s| s.as_str()).collect(); let filter = Filter::new(&reader, &include, &exclude); let references = References::new(&reader, references); let types = TypeMap::filter(&reader, &filter, &references); let derive = Derive::new(&reader, &types, &derive_str); + let implements = Implements::new(&implements_str); let warnings = WarningBuilder::default(); let config = Config { @@ -790,6 +829,7 @@ impl Bindgen { sys_fn_ptrs: self.sys_fn_ptrs, sys_fn_extern: self.sys_fn_extern, implement: self.implement, + implements: &implements, noexcept: self.noexcept, specific_deps: self.specific_deps, link, @@ -816,6 +856,7 @@ enum ArgKind { Rustfmt, Reference, Derive, + Implements, Link, } diff --git a/crates/libs/bindgen/src/types/cpp_interface.rs b/crates/libs/bindgen/src/types/cpp_interface.rs index a4ddf8f21d..1247975981 100644 --- a/crates/libs/bindgen/src/types/cpp_interface.rs +++ b/crates/libs/bindgen/src/types/cpp_interface.rs @@ -234,6 +234,7 @@ impl CppInterface { }); } + if config.should_implement(self.def.type_name(), true) { let impl_name: TokenStream = format!("{}_Impl", self.def.name()).into(); let cfg = if config.package { @@ -433,6 +434,7 @@ impl CppInterface { } }); } + } result } diff --git a/crates/libs/bindgen/src/types/interface.rs b/crates/libs/bindgen/src/types/interface.rs index 326cb1366d..374a76f8d9 100644 --- a/crates/libs/bindgen/src/types/interface.rs +++ b/crates/libs/bindgen/src/types/interface.rs @@ -340,7 +340,7 @@ impl Interface { } } - if config.implement || !is_exclusive { + if config.should_implement(type_name, !is_exclusive) { let impl_name: TokenStream = format!("{}_Impl", trim_tick(self.def.name())).into(); let generics: Vec<_> = self diff --git a/crates/tests/fixtures/harness/data/bindgen/implements/expected.rs b/crates/tests/fixtures/harness/data/bindgen/implements/expected.rs new file mode 100644 index 0000000000..5f87e257fd --- /dev/null +++ b/crates/tests/fixtures/harness/data/bindgen/implements/expected.rs @@ -0,0 +1,62 @@ +pub mod Test { + windows_core::imp::define_interface!(IBar, IBar_Vtbl); + impl IBar { + pub unsafe fn Bar(&self) -> i32 { + unsafe { + (windows_core::Interface::vtable(self).Bar)(windows_core::Interface::as_raw(self)) + } + } + } + #[repr(C)] + #[doc(hidden)] + pub struct IBar_Vtbl { + pub Bar: unsafe extern "system" fn(*mut core::ffi::c_void) -> i32, + } + windows_core::imp::define_interface!(IFoo, IFoo_Vtbl); + impl IFoo { + pub unsafe fn Foo(&self) -> i32 { + unsafe { + (windows_core::Interface::vtable(self).Foo)(windows_core::Interface::as_raw(self)) + } + } + } + #[repr(C)] + #[doc(hidden)] + pub struct IFoo_Vtbl { + pub Foo: unsafe extern "system" fn(*mut core::ffi::c_void) -> i32, + } + pub trait IFoo_Impl { + fn Foo(&self) -> i32; + } + impl IFoo_Vtbl { + pub const fn new() -> Self { + unsafe extern "system" fn Foo( + this: *mut core::ffi::c_void, + ) -> i32 { + unsafe { + let this = + (this as *mut *mut core::ffi::c_void) as *const windows_core::ScopedHeap; + let this = &*((*this).this as *const Identity); + IFoo_Impl::Foo(this) + } + } + Self { + Foo: Foo::, + } + } + } + struct IFoo_ImplVtbl(core::marker::PhantomData); + impl IFoo_ImplVtbl { + const VTABLE: IFoo_Vtbl = IFoo_Vtbl::new::(); + } + impl IFoo { + pub fn new<'a, T: IFoo_Impl>(this: &'a T) -> windows_core::ScopedInterface<'a, Self> { + let this = windows_core::ScopedHeap { + vtable: &IFoo_ImplVtbl::::VTABLE as *const _ as *const _, + this: this as *const _ as *const _, + }; + let this = core::mem::ManuallyDrop::new(windows_core::imp::Box::new(this)); + unsafe { windows_core::ScopedInterface::new(core::mem::transmute(&this.vtable)) } + } + } +} diff --git a/crates/tests/fixtures/harness/data/bindgen/implements/fixture.toml b/crates/tests/fixtures/harness/data/bindgen/implements/fixture.toml new file mode 100644 index 0000000000..8080881848 --- /dev/null +++ b/crates/tests/fixtures/harness/data/bindgen/implements/fixture.toml @@ -0,0 +1,5 @@ +filter = "Test" +no_allow = true +no_comment = true +specific_deps = true +implements = ["Test.IFoo"] diff --git a/crates/tests/fixtures/harness/data/bindgen/implements/input.rdl b/crates/tests/fixtures/harness/data/bindgen/implements/input.rdl new file mode 100644 index 0000000000..32599622d6 --- /dev/null +++ b/crates/tests/fixtures/harness/data/bindgen/implements/input.rdl @@ -0,0 +1,9 @@ +#[win32] +mod Test { + interface IFoo { + fn Foo(&self) -> i32; + } + interface IBar { + fn Bar(&self) -> i32; + } +} diff --git a/crates/tests/fixtures/harness/tests/fixtures.rs b/crates/tests/fixtures/harness/tests/fixtures.rs index e6c20160a0..3c55adff28 100644 --- a/crates/tests/fixtures/harness/tests/fixtures.rs +++ b/crates/tests/fixtures/harness/tests/fixtures.rs @@ -103,6 +103,8 @@ struct FixtureConfig { no_comment: bool, noexcept: bool, specific_deps: bool, + implement: bool, + implements: Vec, references: Vec, /// `winmd_to_rdl` only: prebuilt winmd (or directory) to consume. winmd_input: Option, @@ -138,6 +140,8 @@ impl FixtureConfig { "no_comment" => cfg.no_comment = parse_bool(value), "noexcept" => cfg.noexcept = parse_bool(value), "specific_deps" => cfg.specific_deps = parse_bool(value), + "implement" => cfg.implement = parse_bool(value), + "implements" => cfg.implements = parse_string_list(value), "references" => cfg.references = parse_string_list(value), "winmd_input" => cfg.winmd_input = Some(parse_string(value)), "kind" => cfg.kind = Some(parse_string(value)), @@ -333,6 +337,12 @@ fn run_bindgen(f: &Fixture) { if cfg.noexcept { bindgen.noexcept(); } + if cfg.implement { + bindgen.implement(); + } + for implements in &cfg.implements { + bindgen.implements(implements); + } bindgen.write().unwrap(); diff_or_update(&actual_rs, &f.input("expected.rs")); From 96077799588a3216bf748720ea0622ffa890bfcf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 4 May 2026 18:14:21 +0000 Subject: [PATCH 3/4] Document match_type_name helper in implements.rs Agent-Logs-Url: https://github.com/microsoft/windows-rs/sessions/bbdff62b-17d1-4f21-89d9-39619bdc6d80 Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com> --- crates/libs/bindgen/src/implements.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/libs/bindgen/src/implements.rs b/crates/libs/bindgen/src/implements.rs index 021db45289..9b0c90eb7b 100644 --- a/crates/libs/bindgen/src/implements.rs +++ b/crates/libs/bindgen/src/implements.rs @@ -24,6 +24,11 @@ impl Implements { } } +/// Returns `true` if `rule` selects the type `namespace.name`. A `rule` whose +/// length is less than or equal to `namespace.len()` is treated as a namespace +/// prefix and matches every type whose namespace starts with `rule`. Otherwise +/// `rule` must be a fully-qualified `Namespace.Name` whose namespace component +/// equals `namespace` exactly and whose name component equals `name` exactly. fn match_type_name(rule: &str, namespace: &str, name: &str) -> bool { if rule.len() <= namespace.len() { return namespace_starts_with(namespace, rule); From 408bcbab5d95efb02861596c57f23845bf61a973 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 4 May 2026 18:36:38 +0000 Subject: [PATCH 4/4] Make implements take an iterator; fix cpp_interface fmt Agent-Logs-Url: https://github.com/microsoft/windows-rs/sessions/1d8f2160-2c59-4bbc-ae67-4edba01af9d8 Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com> --- crates/libs/bindgen/src/lib.rs | 13 +- .../libs/bindgen/src/types/cpp_interface.rs | 158 +++++++++--------- .../tests/fixtures/harness/tests/fixtures.rs | 4 +- 3 files changed, 85 insertions(+), 90 deletions(-) diff --git a/crates/libs/bindgen/src/lib.rs b/crates/libs/bindgen/src/lib.rs index 37d59de678..e63adf8184 100644 --- a/crates/libs/bindgen/src/lib.rs +++ b/crates/libs/bindgen/src/lib.rs @@ -399,7 +399,7 @@ where builder.derive(arg); } ArgKind::Implements => { - builder.implements(arg); + builder.implements([arg]); } ArgKind::Rustfmt => { builder.rustfmt(arg); @@ -605,7 +605,7 @@ impl Bindgen { self } - /// Add a type to the list of types for which implementation scaffolding + /// Add types to the list of types for which implementation scaffolding /// should be emitted. /// /// Each entry may be a fully-qualified type name (`Namespace.Name`) or a @@ -616,14 +616,7 @@ impl Bindgen { /// finer-grained alternative to the broad `--implement` switch and can /// significantly reduce build time when only a handful of interfaces /// need to be implemented. - pub fn implements(&mut self, name: &str) -> &mut Self { - self.implements.push(name.to_string()); - self - } - - /// Add multiple types to the list of types for which implementation - /// scaffolding should be emitted. See [`Bindgen::implements`]. - pub fn implements_iter(&mut self, names: I) -> &mut Self + pub fn implements(&mut self, names: I) -> &mut Self where I: IntoIterator, S: AsRef, diff --git a/crates/libs/bindgen/src/types/cpp_interface.rs b/crates/libs/bindgen/src/types/cpp_interface.rs index 1247975981..63bfb92ebc 100644 --- a/crates/libs/bindgen/src/types/cpp_interface.rs +++ b/crates/libs/bindgen/src/types/cpp_interface.rs @@ -235,54 +235,58 @@ impl CppInterface { } if config.should_implement(self.def.type_name(), true) { - let impl_name: TokenStream = format!("{}_Impl", self.def.name()).into(); - - let cfg = if config.package { - fn combine(interface: &CppInterface, dependencies: &mut TypeMap, config: &Config) { - for method in interface.get_methods(config).iter() { - if let CppMethodOrName::Method(method) = method { - dependencies.combine(&method.dependencies); + let impl_name: TokenStream = format!("{}_Impl", self.def.name()).into(); + + let cfg = if config.package { + fn combine( + interface: &CppInterface, + dependencies: &mut TypeMap, + config: &Config, + ) { + for method in interface.get_methods(config).iter() { + if let CppMethodOrName::Method(method) = method { + dependencies.combine(&method.dependencies); + } } } - } - - let mut dependencies = self.dependencies(config.reader); - combine(self, &mut dependencies, config); - - base_interfaces.iter().for_each(|interface| { - if let Type::CppInterface(ty) = interface { - combine(ty, &mut dependencies, config); - } - }); - Cfg::new(&dependencies, config).write(config, false) - } else { - quote! {} - }; + let mut dependencies = self.dependencies(config.reader); + combine(self, &mut dependencies, config); - let mut names = MethodNames::new(); + base_interfaces.iter().for_each(|interface| { + if let Type::CppInterface(ty) = interface { + combine(ty, &mut dependencies, config); + } + }); - let field_methods: Vec<_> = methods - .iter() - .map(|method| match method { - CppMethodOrName::Method(method) => { - let name = names.add(method.def); - if has_unknown_base { - quote! { #name: #name::, } - } else { - quote! { #name: #name::, } + Cfg::new(&dependencies, config).write(config, false) + } else { + quote! {} + }; + + let mut names = MethodNames::new(); + + let field_methods: Vec<_> = methods + .iter() + .map(|method| match method { + CppMethodOrName::Method(method) => { + let name = names.add(method.def); + if has_unknown_base { + quote! { #name: #name::, } + } else { + quote! { #name: #name::, } + } } - } - CppMethodOrName::Name(method) => { - let name = names.add(*method); - quote! { #name: 0, } - } - }) - .collect(); + CppMethodOrName::Name(method) => { + let name = names.add(*method); + quote! { #name: 0, } + } + }) + .collect(); - let mut names = MethodNames::new(); + let mut names = MethodNames::new(); - let impl_methods: Vec<_> = methods.iter().map(|method| match method { + let impl_methods: Vec<_> = methods.iter().map(|method| match method { CppMethodOrName::Method(method) => { let name = names.add(method.def); let signature = method.write_abi(config, true); @@ -312,23 +316,23 @@ impl CppInterface { _ => quote! {}, }).collect(); - let mut names = MethodNames::new(); + let mut names = MethodNames::new(); - let trait_methods: Vec<_> = methods - .iter() - .map(|method| match method { - CppMethodOrName::Method(method) => { - let name = names.add(method.def); - let signature = method.write_impl_signature(config, true); - quote! { fn #name #signature; } - } - _ => quote! {}, - }) - .collect(); + let trait_methods: Vec<_> = methods + .iter() + .map(|method| match method { + CppMethodOrName::Method(method) => { + let name = names.add(method.def); + let signature = method.write_impl_signature(config, true); + quote! { fn #name #signature; } + } + _ => quote! {}, + }) + .collect(); - let impl_base = base_interfaces.last().map(|ty| ty.write_impl_name(config)); + let impl_base = base_interfaces.last().map(|ty| ty.write_impl_name(config)); - let field_base = base_interfaces.last().map(|ty|{ + let field_base = base_interfaces.last().map(|ty|{ match ty { Type::IUnknown => quote! { base__: windows_core::IUnknown_Vtbl::new::(), }, Type::Object => quote! { base__: windows_core::IInspectable_Vtbl::new::(), }, @@ -344,30 +348,30 @@ impl CppInterface { } }); - // If any methods were skipped due to missing dependencies, the interface cannot be - // fully described, so omit the ability to implement it rather than emitting a - // partial vtable with null function pointer slots. Also propagate the omission - // when any base interface had its `_Impl` trait omitted, since a derived `_Impl` - // cannot reference a base `_Impl` that wasn't emitted. - let has_skipped_methods = methods - .iter() - .any(|method| matches!(method, CppMethodOrName::Name(_))) - || base_interfaces.iter().any(|ty| match ty { - Type::CppInterface(ty) => ty.has_skipped_methods(config), - _ => false, - }); + // If any methods were skipped due to missing dependencies, the interface cannot be + // fully described, so omit the ability to implement it rather than emitting a + // partial vtable with null function pointer slots. Also propagate the omission + // when any base interface had its `_Impl` trait omitted, since a derived `_Impl` + // cannot reference a base `_Impl` that wasn't emitted. + let has_skipped_methods = methods + .iter() + .any(|method| matches!(method, CppMethodOrName::Name(_))) + || base_interfaces.iter().any(|ty| match ty { + Type::CppInterface(ty) => ty.has_skipped_methods(config), + _ => false, + }); - if has_skipped_methods { - config.warnings.skip_implement(self.def); + if has_skipped_methods { + config.warnings.skip_implement(self.def); - if has_unknown_base { - result.combine(quote! { - #cfg - impl windows_core::RuntimeName for #name {} - }); - } - } else { - result.combine( if has_unknown_base { + if has_unknown_base { + result.combine(quote! { + #cfg + impl windows_core::RuntimeName for #name {} + }); + } + } else { + result.combine( if has_unknown_base { let matches = base_interfaces.iter().filter_map(|ty|{ match ty { Type::CppInterface(ty) => { @@ -433,7 +437,7 @@ impl CppInterface { } } }); - } + } } result diff --git a/crates/tests/fixtures/harness/tests/fixtures.rs b/crates/tests/fixtures/harness/tests/fixtures.rs index 3c55adff28..0b56ef0307 100644 --- a/crates/tests/fixtures/harness/tests/fixtures.rs +++ b/crates/tests/fixtures/harness/tests/fixtures.rs @@ -340,9 +340,7 @@ fn run_bindgen(f: &Fixture) { if cfg.implement { bindgen.implement(); } - for implements in &cfg.implements { - bindgen.implements(implements); - } + bindgen.implements(&cfg.implements); bindgen.write().unwrap(); diff_or_update(&actual_rs, &f.input("expected.rs"));