ExternTypes may be safely passed and returned by value

[adetaylor]

        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 give_bob() -> Bob;
                type Bob = super::bindgen::Bob;
            }
        }

doesn't work because we can't return ExternTypes by value.

Each extern type may be one of three fundamental categories of type:

• An alias to another cxx::bridge type (in which case it's a simple alias and we don't do anything in the current instantiation)
• A type which is_trivially_move_constructible && is_trivially_destructible. If so, we allow allow it to be passed by value into/our of cxx types, and we check this with static_asserts as discussed in Provide builtin handling of exposing a C++ constructor #280. ("trivial" types for short henceforth)
• A type which isn't trivially move constructable/destructable. In these cases we don't allow use by value but we do allow generation of UniquePtr traits etc. as per Trait impls needed for type aliases #312.

This distinction is implied by #280 but this could be done as a pre-requisite step first. I also feel it would be desirable to pin this down before finalizing design for #312.

The question is, for a given ExternType, how does cxx know which of these three categories of type it is? What's your plan there? Is it to require the developer to implement an extra trait for trivial types which is then checked for safety using static_assert? Or did you plan to do something funky to auto-detect trivial types at build time?

(NB for this last category of types, my higher-level code generator plans to create shims which allow us to call pre-existing C++ APIs that take a T by instead passing a UniquePtr<T>. I have yet to do this bit. It makes superficial sense that "complicated" types need to be entirely allocated, accessed, and managed within C++ but can still be owned by Rust. Figuring out whether this is ergonomic in practice is arguably the entire point of my https://github.com/google/autocxx experiments... if this doesn't work out it's back to the drawing board!)

[adetaylor]

Hypothetical design:

• ExternType gains a new method along these lines:

pub enum TypeCategory { // better name needed
   #[doc(hidden)]
   Alias,
   Trivial,
   NonTrivial
}

pub unsafe trait ExternType {
    //...
    type Id;

    fn get_category() -> TypeCategory {
       TypeCategory::NonTrivial // default implementation
    }
}

Where we expand ExternType ourselves, we return Alias. Users' pre-existing implementations of ExternType will (obviously) get NonTrivial and thus get current behavior.

But folk can override get_category if they need to state that their type is trivial.

Then:

• For Alias types, we do nothing.
• For Trivial or NonTrivial types, we may generate UniquePtrTarget and other traits depending on the decisions in Trait impls needed for type aliases #312.
• For Trivial types, we allow them to be passed by value into and out of functions just as if the type were declared in the cxx::bridge block. We also generate some static_asserts to verify that they really are trivial.

[adetaylor]

(though maybe if the ExternType is never actually instantiated, we may need to do this with an associated type like for Id. But something along these lines...)

[adetaylor]

I'm fiddling around here. I so far have concluded:

• I probably can't use the existing ExternType trait derivation, since it's probably not accessible at the point of macro expansion or C++ expansion. As I understand it, it's only used to generate something which can be used for a compile-time assertion check when building the resultant Rust code.
• Instead, it's probably necessary for this "trivial" annotation to be somewhere inside the #[cxx::bridge] mod. Right now I'm imagining that a type can impl a new trait here to indicate that it's trivial. Fiddling in that direction.

I probably won't have another chance to touch this till next week.

[dtolnay]

That approach looks promising to me.

As you found, the design of cxx is such that we can't know most things during macro expansion, but we can generate static assertions to verify things post macro expansion.

Looking at your categorization of Alias, Trivial, NonTrivial --- is there any need to distinguish Alias vs NonTrivial? I hope we can treat those exactly the same.

At that point the only relevant distinction would be Trivial (obtainable by value in Rust) vs NonTrivial (not obtainable by value in Rust). And the only verification we would need to emit is that every type alias which is exposed by value to Rust (whether in an extern function signature or as a field of a shared struct) has an ExternType impl which indicates it is Trivial.

Our current scheme for verifying (though this is certainly not the only possible one) is based on:

cxx/src/extern_type.rs

Lines 109 to 110 in acc7fb0

	#[doc(hidden)]
	pub fn verify_extern_type<T: ExternType<Id = Id>, Id>() {}

which we instantiate as follows to verify post macro expansion that a particular type has a specific expected Id in its ExternType impl.

const _: fn() = verify_extern_type::<YourType, TheExpectedId>;

I imagine a verify similar mechanism for verifying the Trivial vs NonTrivial distinction too.

// if your type alias was exposed by value to Rust, then we generate:
const _: fn() = verify_triviality::<YourType, Trivial>;

Beyond that, I believe the existing Rust orphan rules are good enough that we should be able to let you put #312-style "request for an impl" wherever the user wants. (But I need to revisit this more carefully to confirm that I am sure about this.)

[adetaylor]

Aha, I think you're saying that there should be both something in the ExternType impl (presumably outside the #[cxx::bridge] block) and some syntax inside the #[cxx::bridge] block. I was now thinking we only need the latter? Then there should be no need for the Rust-side verification that you outline, and just C++-side static_asserts?

Is there some need to do something in the ExternType impl as well as inside the #[cxx::bridge]?

As for your question:

Looking at your categorization of Alias, Trivial, NonTrivial --- is there any need to distinguish Alias vs NonTrivial?

First, an assumption: if there's an alias, the target cxx::bridge actually generates all the code for the type, e.g. UniquePtrTarget impls, plus all the Rust-side and C++-side wrappers, and the alias therefore means there is no codegen beyond just a simple Rust type.

I haven't yet checked that assumption, but that motivated the distinction I made.

I was therefore thinking:

• Alias: we need to look up the Trivial-ness by examining the target type over in its distant #[cxx::bridge] section. (Which we can't, of course, and I don't really have a plan there. As it affects how we do codegen based on our #[cxx::bridge] section, we really do need to know. But it would be a shame if we had to specify the trivial-ness in both #[cxx::bridge] blocks). Also, for an Alias, we will never generate UniquePtrTarget or other traits because that will be done by the target #[cxx::bridge].
• NonTrivial: may generate UniquePtrTarget etc. but can't pass by value.
• Trivial: may generate UniquePtrTarget etc. and can pass by value.

I'm also trying to plan ahead for all the different properties we might ever want to attach to types. If we're designing syntax for trivial/non-trivial maybe we should be thinking of a more general type traits arrangement.

Possible properties (and I know many of these will never happen, and may be more appropriate for a higher level code generator; my point is really just to indicate that we might want some general syntax for specifying what the user wants of a type):

Property	Description	Applies to extern types	Applies to opaque C++ types	C++ codegen or static assertions needed	Rust codegen needed
Trivial	Whether this can be passed by value in Rust, as discussed	Yes	No	Yes	No except in APIs
Unique pointer target	Whether we should allow this too be stored in UniquePtrs per #236	Yes	Yes	No	Yes
Vector target, etc.	Other traits as per the previous line	Yes	Yes	No	Yes
Generate field accessors	Generate accessor functions to get and set C++ fields	Yes (probably non-trivial only)	Yes	Yes	Yes
Allow pass UniquePtr by value	Allow this type to be passed into C++ functions "by value" when all we have is a UniquePtr, for compatibility with existing functions that take the type by value	Yes (non-trivial only)	Yes	Yes	Modifies existing codegen

Each of those could be its own impl block within the #[cxx::bridge] (as you propose in #236 and I'm starting to hack about with in 84744b7) but I do wonder if we need something nicer. e.g.

#[cxx::bridge]
mod ffi {
  // ...
  impl TypeTraits for MyRustType {
    const TRAITS = TypeTraits {
       trivial = true;
       generate_accessors = true;
    }
  }
}

but it's not very Rusty syntax.

[dtolnay]

Focusing just on Trivial/NonTrivial for now:

I think you're saying that there should be both something in the ExternType impl (presumably outside the #[cxx::bridge] block) and some syntax inside the #[cxx::bridge] block. I was now thinking we only need the latter? Then there should be no need for the Rust-side verification that you outline, and just C++-side static_asserts?

Is there some need to do something in the ExternType impl as well as inside the #[cxx::bridge]?

I think my plan is the opposite of your plan. You're thinking about what syntax goes in the cxx::bridge module. I am thinking no new syntax or annotation is needed in the cxx::bridge module at all to support this feature.

Here are the expansion rules:

extern "C++" {
    // If this type is an "original", not an alias:
    //   pub struct ZeusClient(::cxx::private::Opaque);  // same as today
    //   impl ExternType for ZeusClient { type Kind = cxx::kind::Opaque; }  // "Opaque" == NonTrivial
    type ZeusClient;

    // If an alias, only validate the type Id; no new impl:
    //   verify_id::<crate::bindgen::ZeusClient, cxx::type_id!("ZeusClient")>;
    type ZeusClient = crate::bindgen::ZeusClient;

    // If type is ever mentioned by value, verify trivial:
    //   verify_kind::<ZeusClient, cxx::kind::Trivial>;
    fn new(arg: &str) -> ZeusClient;

    // As per #312. Allowed wherever the user feels like, subject to orphan rules.
    // Optional if T is an "original" and has some signature involving UniquePtr<T> in the same cxx::bridge.
    impl UniquePtr<ZeusClient> {}
}

The user handwrites or uses a bindgen to bring Rust definitions of trivial extern types.

mod bindgen {
    #[repr(C)]
    pub struct ZeusClient {
        pub port: u16,
    }
}

unsafe impl cxx::ExternType for bindgen::ZeusClient {
    type Id = cxx::type_id!("ZeusClient");
    type Kind = cxx::kind::Trivial;
}

Next to verify_kind::<ZeusClient, cxx::kind::Trivial> we can also verify is_trivially_move_constructible && is_trivially_destructible on the C++ side, but actually this is just nice-to-have since the user went on the record with their unsafe impl to unsafely claim it is trivial, so it's on them if that were wrong.

I read your last two comments again and don't spot anything that would require more ceremony/new syntax in the cxx::bridge module, but I could just be missing it.

[adetaylor]

Ah! I hadn't thought of making this determination based on how the type is used. OK! I will think on that, thanks.

[adetaylor]

I'm well on the way here.

I have run across one question which I wanted to talk about.

When should we auto-generate UniquePtrTarget for an alias?

• If it's an alias to a type defined in another #[cxx::bridge], ideally we do not want to generate UniquePtrTarget, because this would presumably conflict with a UniquePtrTarget implemented in the destination #[cxx::bridge].
• If it's an alias to something outside of #[cxx::bridge], e.g. a bindgen-generated struct, we almost certainly do want to generate UniquePtrTarget.

I think your plan here is to retain the current behavior of never automatically generating a UniquePtrTarget for an alias, and instead requiring the user to do impl UniquePtrTarget themselves per #236. And the same for all the other traits. Right? If so I'll do the impl UniquePtrTarget bit as part of this PR as well.

[dtolnay]

I think your plan here is to retain the current behavior of never automatically generating a UniquePtrTarget for an alias, and instead requiring the user to do impl UniquePtrTarget themselves per #236. And the same for all the other traits. Right? If so I'll do the impl UniquePtrTarget bit as part of this PR as well.

That's right.

Separate PR would also be good if it separates cleanly from #325.