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

Legitimate uses for UniquePtrs even if not currently expanded #236

Closed
adetaylor opened this issue Jul 27, 2020 · 2 comments · Fixed by #336
Closed

Legitimate uses for UniquePtrs even if not currently expanded #236

adetaylor opened this issue Jul 27, 2020 · 2 comments · Fixed by #336

Comments

@adetaylor
Copy link
Collaborator

#228 (comment) gives code like this (I've changed the type names but nothing else):

use cxx::UniquePtr;

#[cxx::bridge(namespace = base::test)]
pub mod ffi {
    extern "C" {
        include!("base/test/scoped_feature_list.h");
        
        type ScopedFeatureList;
        
        fn Init(self: &mut ScopedFeatureList);
    }
}

pub struct ScopedFeatureList {
    instance: UniquePtr<ffi::ScopedFeatureList>,
}

impl ScopedFeatureList {
    pub fn init(&mut self) {
        self.instance.Init();
    }
}

This doesn't compile because:

error[E0277]: the trait bound `features::ffi::ScopedFeatureList: cxx::private::UniquePtrTarget` is not satisfied
  --> ../../base/rs/src/features.rs:15:5
   |
15 |     instance: UniquePtr<ffi::ScopedFeatureList>,
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `cxx::private::UniquePtrTarget` is not implemented for `features::ffi::ScopedFeatureList`
   | 
  ::: /Users/adetaylor/dev/chromium/src/out/Debug/../../third_party/rust/crates/cxx/src/unique_ptr.rs:14:8
   |
14 |     T: UniquePtrTarget,
   |        --------------- required by this bound in `cxx::UniquePtr`

This is because expand_unique_ptr never gets called, because no methods in the ffi block take a UniquePtr<ScopedFeatureList> as a parameter.

@adetaylor
Copy link
Collaborator Author

(that last sentence may be wrong, but that's what I reckon is happening, based on hunting for a workaround!)

Maybe the fix is to assume any type can be a UniquePtrTarget. If that's your preferred fix, let me know and I can work on a PR to that effect. Then again, the same class of problem might occur for CxxVector and maybe other things.

@dtolnay
Copy link
Owner

dtolnay commented Jul 29, 2020

This is because expand_unique_ptr never gets called, because no methods in the ffi block take a UniquePtr<ScopedFeatureList> as a parameter.

You guessed right -- this is exactly why.

The easiest workaround currently would be add an unused struct inside of the mod ffi which has any extra generic types you require as fields:

    struct ExtraInstantiations {
        _0: UniquePtr<ScopedFeatureList>,
    }

I had been hoping not to generate impls for every possible use of every type. Maybe doing it for just UniquePtr wouldn't be so bad. But once we start having maps, we'll need some better solution anyway: it wouldn't be good to instantiate std::map<K, V> for every possible combination of K, V.

As far as hacks go, an unused struct like that isn't the worst thing, but I am definitely open to supporting this better (with an attribute, or something else that makes the intent clearer; I really don't know).

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