Skip to content

Add ImplParam GAT: use primitive types directly in _Impl trait methods#4155

Draft
Copilot wants to merge 8 commits intomasterfrom
copilot/consider-removing-ref-in-map-test
Draft

Add ImplParam GAT: use primitive types directly in _Impl trait methods#4155
Copilot wants to merge 8 commits intomasterfrom
copilot/consider-removing-ref-in-map-test

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 7, 2026

Resolves the TODO in crates/tests/libs/implement/tests/map.rs that said // TODO: shouldn't require Ref for primitive.

Problem

When implementing IMap_Impl<i32, f32>, users had to write:

fn HasKey(&self, key: Ref<i32>) -> Result<bool> { ... }

even though i32 is a primitive copy type where Ref<T> adds no semantic value. The same issue affected event handler closures — EventHandler::<i32>::new(move |_, args: Ref<i32>| { assert_eq!(*args, 2); ... }) required unnecessary wrapping and dereferencing.

Solution

Added a Param<'a> generic associated type (GAT) to the Type<T, C> trait in windows-core:

  • For CopyType (primitives, GUID, enums, etc.): Param<'a> = T — the type itself, no Ref wrapper
  • For CloneType (HSTRING, etc.): Param<'a> = Ref<'a, T> — same as before
  • For InterfaceType (COM interfaces): Param<'a> = Ref<'a, T> — same as before

Two helpers are added to Type<T, C>:

  • abi_to_param(abi: &Self::Abi) -> Self::Param<'_> — used in generated vtable code to convert from raw ABI parameter to impl param type
  • param_as_default(param: &Self::Param<'_>) -> &Self::Default — used in generic impl method bodies to get &K::Default from either K (copy) or Ref<K> (others)

A public type alias is exported:

pub type ImplParam<'a, T> = <T as Type<T>>::Param<'a>;

The bindgen code generator is updated to:

  • Emit windows_core::ImplParam<'_, K> instead of windows_core::Ref<K> for generic type parameters in _Impl trait method signatures
  • Use <K as windows_core::Type<K>>::abi_to_param(&key) instead of transmute_copy(&key) in vtable upcall code for generic parameters

All affected bindings are regenerated (windows-collections, windows-future, Windows/Foundation, test bindgen files, reference_custom/src/bindings.rs, reference_no_deps/src/bindings.rs). Hand-written impls (map_view.rs, vector_view.rs, test files) are updated to use the new API. Test files using event handler closures (events_client/src/lib.rs, event_core/tests/tests.rs) are updated to use primitive values directly instead of Ref<primitive> / *args. Two clippy::explicit_auto_deref lints in the param_as_default implementations are resolved by returning param directly instead of &**param. The delegates.rs test is updated to remove the now-resolved *port dereference of a primitive i32.

Result

Users can now write:

impl IMap_Impl<i32, f32> for Map_Impl {
    fn HasKey(&self, key: i32) -> Result<bool> { ... }    // i32 directly, no Ref!
    fn Insert(&self, key: i32, value: f32) -> Result<bool> { ... }
    fn Lookup(&self, key: i32) -> Result<f32> { ... }
    fn Remove(&self, key: i32) -> Result<()> { ... }
}

Event handler closures over primitive types are also simplified:

// Before
EventHandler::<i32>::new(move |_, args: Ref<i32>| { assert_eq!(*args, 2); Ok(()) });

// After
EventHandler::<i32>::new(move |_, args| { assert_eq!(args, 2); Ok(()) });

For non-primitive types like HSTRING and COM interfaces, Ref<T> is still used as before (which remains the correct approach for borrowed, non-owned types).

…l trait methods

Agent-Logs-Url: https://github.com/microsoft/windows-rs/sessions/205ca06b-cbb1-41ee-8d46-dd1379661f73

Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
@kennykerr
Copy link
Copy Markdown
Collaborator

@copilot check the build failures

… directly (no Ref)

Agent-Logs-Url: https://github.com/microsoft/windows-rs/sessions/b7ddf051-5172-4d8e-ac1c-6bec9857a693

Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 7, 2026

@copilot check the build failures

Fixed in 70debcf. The CI failures were in events_client/src/lib.rs and event_core/tests/tests.rs — these test files had explicit args: Ref<i32> type annotations and *args dereferences in event handler closures. After the change, ImplParam<'_, i32> resolves to i32 directly (not Ref<i32>), so those were updated to use args: i32 and args directly. Also fixed generic_primitive.rs which had the same pattern.

@kennykerr
Copy link
Copy Markdown
Collaborator

@copilot fmt failed

@kennykerr
Copy link
Copy Markdown
Collaborator

@copilot more test failures

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 7, 2026

@copilot more test failures

Fixed in 0d8c552. Two clippy::explicit_auto_deref errors in crates/libs/core/src/type.rs — the param_as_default implementations for InterfaceType and CloneType were returning &**param instead of param (auto-deref handles the coercion).

@kennykerr
Copy link
Copy Markdown
Collaborator

@copilot clippy failure

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 7, 2026

@copilot clippy failure

Fixed in eb7a009. crates/tests/winrt/old/tests/delegates.rs line 50 had assert!(*port == 80)port is now i32 directly (not Ref<i32>), so dereferencing it caused E0614. Changed to assert!(port == 80) and removed the stale TODO comment.

@kennykerr
Copy link
Copy Markdown
Collaborator

@copilot build failures

… with ImplParam

Agent-Logs-Url: https://github.com/microsoft/windows-rs/sessions/347294e7-f5c5-4215-b0ce-f6f3bf474d8f

Co-authored-by: kennykerr <9845234+kennykerr@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 7, 2026

@copilot build failures

Fixed in 7e3ed76. The "Tests changed code in the repo" check was failing because crates/tests/winrt/reference_custom/src/bindings.rs and crates/tests/winrt/reference_no_deps/src/bindings.rs were not regenerated — they still had windows_core::Ref<T> and transmute_copy(&value) patterns. Updated both files to use ImplParam<'_, T> in trait signatures and <T as windows_core::Type<T>>::abi_to_param(&value) in vtable code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants