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

Trait impls needed for type aliases #312

Closed
adetaylor opened this issue Sep 22, 2020 · 1 comment · Fixed by #336
Closed

Trait impls needed for type aliases #312

adetaylor opened this issue Sep 22, 2020 · 1 comment · Fixed by #336

Comments

@adetaylor
Copy link
Collaborator

Commit 2b821e6 disabled type aliases for non-local types.

This causes problems with this sort of code:

mod ffi {
    unsafe impl cxx::ExternType for bindgen::Bob {
        type Id = cxx::type_id!("Bob");
    }
    mod bindgen {
        #[repr(C)]
        pub struct Bob {
            pub a: u32,
            pub b: u32,
        }
    }
    #[cxx::bridge]
    pub mod cxxbridge {
        extern "C" {
            include!("input.h");
            pub fn take_bob(a: UniquePtr<Bob>) -> u32;
            type Bob = super::bindgen::Bob;
        }
    }
}

because of course

error[E0277]: the trait bound `ffi::bindgen::Bob: cxx::private::UniquePtrTarget` is not satisfied
  --> demo/src/main.rs:32:32
   |
32 |             pub fn take_bob(a: UniquePtr<Bob>) -> u32;
   |                                ^^^^^^^^^^^^^^ the trait `cxx::private::UniquePtrTarget` is not implemented for `ffi::bindgen::Bob`
   | 
  ::: /Users/adetaylor/dev/cxx/src/unique_ptr.rs:14:8
   |
14 |     T: UniquePtrTarget,
   |        --------------- required by this bound in `cxx::UniquePtr`

(Context: previously my https://github.com/google/autocxx experiments rewrote the bindgen output to generate a self-contained cxx::bridge, but I believe the better thing is to have lots of ExternTypes pointing from the cxx::bridge to the bindgen output, so I'm trying to make that work. This may be the first of several issues as I dig into it.)

I'm eager to raise a PR to fix this but I can't see the easiest way. Given the context of 2b821e6, I assume some badness occurs if we generate trait impls for aliases to types in other cxx::bridge blocks. Is that the case? If so, I've a feeling we can't distinguish this case versus the type-defined-entirely-outside-cxxbridge case we've got here, and we might need some new syntax. Thoughts?

@dtolnay
Copy link
Owner

dtolnay commented Sep 22, 2020

I assume some badness occurs if we generate trait impls for aliases to types in other cxx::bridge blocks. Is that the case?

Yes this is the reason. You end up with multiple conflicting impls on the same type with no way to turn it off.

I think what I would prefer is for a module to be able to explicitly request certain impls to be instantiated. We would still keep our current implicit impls for convenience in all the cases where a particular impl is "obviously needed".

#[cxx::bridge]
mod ffi {
    extern "C++" {
        type Bob = super::bindgen::Bob;
    }

    impl UniquePtr<Bob> {}
}

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 a pull request may close this issue.

2 participants