Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better generic support in implement #2986

Closed
wants to merge 1 commit into from

Conversation

roblabla
Copy link
Contributor

Fixes: #2984

It turns out windows-rs already supports generics in the implement macro, but would handle generic bounds of the form <T: Bound> a bit wrong. It would generate code like this:

impl<T: AmsiProvider> ::core::convert::From<AmsiClassFactory<T: AmsiProvider>>
for ::windows::core::IUnknown {
    fn from(this: AmsiClassFactory<T: AmsiProvider>) -> Self {
        let this = AmsiClassFactory_Impl::<T: AmsiProvider>::new(this);
        let boxed = ::core::mem::ManuallyDrop::new(::std::boxed::Box::new(this));
        unsafe { ::core::mem::transmute(&boxed.identity) }
    }
}

This is wrong, first line should read impl<T: AmsiProvider> ::core::convert::From<AmsiClassFactory<T>>, dropping the bound when applying the generic on the right hand side of the for.

Syn has a function for exactly this use-case: Generics::split_for_impl, which returns three values: An ImplGeneric to apply to the impl and contains the bounds, a TypeGenerics to apply on a type and does not contain the bounds,and the WhereClause with the additional bounds.

This MR reworks the implement macro to use this function, thus fixing this corner case.

@kennykerr
Copy link
Collaborator

Please ensure that you've run all of cargo test, clippy, and fmt on the entire workspace before pushing.

@kennykerr
Copy link
Collaborator

Closing for now. Feel free to reopen if you'd like to revisit in future.

@kennykerr kennykerr closed this Apr 16, 2024
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.

Support generic bounds on implement macro
2 participants