cxx
cxx copied to clipboard
Implement glob import semantics for builtin type names
Currently we disallow any user-defined type name that collides with one of the builtin type names.
error[cxxbridge]: reserved name
┌─ src/main.rs:3:12
│
3 │ struct UniquePtr { broken: bool }
│ ^^^^^^^^^ reserved name
error[cxxbridge]: reserved name
┌─ src/main.rs:5:12
│
5 │ struct c_char { broken: bool }
│ ^^^^^^ reserved name
However, this is bad because it makes it a breaking change for us to add new builtin bindings, such as #681. If the user already had their own type with the same name as the new binding, that code needs to continue to work.
We need to implement basically the same semantics that Rust has for prelude types and glob imports — a user-defined type replaces any builtin binding of the same name.
Design sketch:
Constraints:
- We will need to behave differently in both C++ and Rust code generation. If
c_charisstd::os::raw::c_char, it must be so prefixed inimpl ToTokens, whilst if it's a type declared in thecxx::bridgemodthen it shouldn't be. Similar changes are needed in the generated C++. - We could potentially pass more global information into the C++ codegen, but not (readily) into the Rust codegen because we're implementing the existing signatures within the
ToTokenstrait. - We need to support referring to types which are declared later:
#[cxx::bridge] mod ffi { extern "C++" { fn needs_a_type_declared_later(a: c_char); } struct c_char { // ... }; }
Design proposal:
- Split
Type::Identinto two different enum variants,Type::BuiltInversusType::Ident. parse.rscontinues always to generateType::Ident.- Within
generate, we continue to doTypes::collectas now, unchanged. - But then we call a new function immediately after
Types::collect, calledTypes::spot_builtins(apis: &mut [Api], types: &Types). - That function visits all
Types anywhere in the APIs, and replacesType::IdentwithType::BuiltInin all cases where the type is not known totypes. - The checking phase, and Rust and C++ code generation, are all modified to assume that
Type::Identis never anAtomand is always a type defined in thecxx::bridgemod; whilstType::Builtinis always anAtom.
Does that sound about right @dtolnay?
If you aren't going to get to it soon @adetaylor, I could potentially take a good stab at implementing the above design this coming week.
That would be great, yes. I don't think I'm realistically going to get to it in the next few days. If I start, I'll post here - but it's extremely unlikely :) (Either way we should wait for @dtolnay to tell me that my design sketch is nonsense)
(Had some free time, decided to take a look at this again.)
@adetaylor Design proposal:
- [...]
- But then we call a new function immediately after
Types::collect, calledTypes::spot_builtins(apis: &mut [Api], types: &Types).- That function visits all
Types anywhere in the APIs, and replacesType::IdentwithType::BuiltInin all cases where the type is not known totypes.- [...]
This can't work as-written, as Types borrows from apis. If we want to fix-up the builtin Types in-place, there are three simple-ish options that I see:
- restructure
Types::collectto not borrow fromapis(do the fixup, then actually borrow everything to continue); - do
Types::collect, record whichTypes to fixup, do the fixups, re-Types::collect; or - use internal mutability (e.g.
Types::Ident(NamedType, Cell<bool>)) to do the fixup.
The other approach I can think of is to not do an inplace fixup of Types, but to instead have an extra list of Types in Types of "hey this is a Type::Ident of a builtin, not a user-defined type." I don't know how much that would impact downstream.
#908 attempts to implement this via keeping a new mapping of types to atoms, which is used by downstream passes instead of Atom::from. It needs significant testing to ensure that it actually works, but the approach seems sound. However, it only handles atoms (e.g. c_char) at the moment, and not builtins (e.g. UniquePtr). Since builtins are unique kinds of Types, this approach of mapping Type::Ident to Atom won't work there.