Support type aliases across bridges using different C++ namespaces

[sbrocket]

Related to #297

[sbrocket]

Needs documentation but please let me know what you think first.

[dtolnay]

Thanks for the PR! I have not gotten a chance to read the implementation yet but I am skimming the tests (tests/ffi/alias*.rs) -- my sense is that the usage is going to feel pretty clumsy as a partial solution for "how do we allow one bridge to involve items from more than one namespace", which is the actual thing to solve, I think. If a C++ function in one namespace has an argument type in a different namespace (like alias::ffi::c_take_unique_ptr in the test suite), requiring to express that as two separate cxx::bridge modules with an extern type alias from one to the other and an alias_namespace attribute seems too convoluted.

This isn't fully designed but I wonder if allowing namespace controlled per extern block (inheriting from the namespace of the cxx::bridge if not specified on the extern block) would solve the same thing better.

#[cxx::bridge]
mod ffi {
    #[namespace = types]
    extern "C" {
        type SameC;
    }
    #[namespace = alias::tests]
    extern "C" {
        fn take_unique_ptr(c: UniquePtr<SameC>);
    }
}

Then, for the case where you do actually want items to be split across different cxx::bridge modules for Rust code organization reasons, the above just composes with the existing support for sharing a single Rust type across all different mentions of the opaque C++ type, by inserting = path::to::SameC.

Any thoughts on this in comparison to the approach in the PR?

---

Regarding your earlier comment about "unexpected token" error on an unquoted namespace: that isn't a proc macro or syn thing, just a silly rustc parsing limitation. See this playground for a repro without any macro involved. I opened rust-lang/rust#76734 with a fix.

[sbrocket]

Thanks for the PR! I have not gotten a chance to read the implementation yet but I am skimming the tests (tests/ffi/alias*.rs) -- my sense is that the usage is going to feel pretty clumsy as a partial solution for "how do we allow one bridge to involve items from more than one namespace", which is the actual thing to solve, I think. If a C++ function in one namespace has an argument type in a different namespace (like alias::ffi::c_take_unique_ptr in the test suite), requiring to express that as two separate cxx::bridge modules with an extern type alias from one to the other and an alias_namespace attribute seems too convoluted.

Great point. There's a couple different use cases being solved for here. I've been approaching this mostly with my use case in mind - where I'm going to use this for CXX shared types (once I add aliasing support for that), not C++ opaque types - but the use case you bring up is also totally valid.

I think cxx::alias_namespace is fine for the case where you have a bridge module for the other namespace anyway, as is the case with the current tests and alias::ffi::c_take_unique_ptr. I definitely would not want to force users to create an extra bridge just to use this attribute, however.

This isn't fully designed but I wonder if allowing namespace controlled per extern block (inheriting from the namespace of the cxx::bridge if not specified on the extern block) would solve the same thing better.

#[cxx::bridge]
mod ffi {
    #[namespace = types]
    extern "C" {
        type SameC;
    }
    #[namespace = alias::tests]
    extern "C" {
        fn take_unique_ptr(c: UniquePtr<SameC>);
    }
}

Then, for the case where you do actually want items to be split across different cxx::bridge modules for Rust code organization reasons, the above just composes with the existing support for sharing a single Rust type across all different mentions of the opaque C++ type, by inserting = path::to::SameC.

Any thoughts on this in comparison to the approach in the PR?

We need to keep the module-level attribute that I currently have to support shared type aliases (once I add those, right after or in parallel with this PR), but supporting an attribute at the extern block level as well makes a lot of sense to me. I had forgotten that you can have multiple extern blocks. What do you think of the example below?

#[cxx::bridge(namespace = alias::tests)]
#[cxx::alias_namespace(other::ffi = foobar)
mod ffi {
    type Shared = other::ffi::Shared;

    #[namespace = types]
    extern "C" {
        type SameC;
    }

    extern "C" {
        fn convert(c: UniquePtr<SameC>) -> Shared;
    }
}

Also, potential alternative names for cxx::alias_namespace: cxx::module_namespace, cxx::bridge_namespace,cxx::remote_namespace (meh), or simply cxx::namespace (meh, I prefer a clearer distinction between the module-level and extern-level attributes).

FYI: You may not have noticed if you didn't dig into the individual commits, but I had originally added exactly that #[namespace = foo] attribute but on individual alias items, before refactoring it out to the current cxx::alias_namespace module-level attribute. I can just reintroduce that and shift it to the extern block level.

Regarding your earlier comment about "unexpected token" error on an unquoted namespace: that isn't a proc macro or syn thing, just a silly rustc parsing limitation. See this playground for a repro without any macro involved. I opened rust-lang/rust#76734 with a fix.

Ah great, thanks!

[sbrocket]

Ping, looking for a response to proposal above.

[dtolnay]

Sure thing -- I apologize about the holdup. The timing was unfortunate here since I was on vacation from the day before this PR was submitted until Tuesday of this week, and now I am working through an enormous review backlog in both https://github.com/rust-lang/rust and my own projects in addition to work.

[sbrocket]

No problem, just wanted to get it back on your radar and agree on a design so I don't go through multiple implementation iterations unnecessarily.

[dtolnay]

We need to keep the module-level attribute that I currently have to support shared type aliases (once I add those, right after or in parallel with this PR)

I would still very much prefer no module-level alias_namespace attribute. I think the mental model for knowing when to reach for it is too complicated. Is there any other way to express what you need?

From the code snippet in #298 (comment) you could do:

#[cxx::bridge]
mod ffi {
    #[namespace = foobar]
    use other::ffi::Shared;

    #[namespace = types]
    extern "C" {
        type SameC;
    }

    #[namespace = alias::tests]
    extern "C" {
        fn convert(c: UniquePtr<SameC>) -> Shared;
    }
}

so basically:

- #[alias_namespace(other::ffi = foobar)]
...
-     type Shared = other::ffi::Shared;
+     #[namespace = foobar]
+     use other::ffi::Shared;
...
-     type Shared1 = other::ffi::Shared1;
-     type Shared2 = other::ffi::Shared2;
+     #[namespace = foobar]
+     use other::ffi::{Shared1, Shared2};
...
-     type MyShared = other::ffi::Shared;
+     #[namespace = foobar]
+     use other::ffi::Shared as MyShared;

[sbrocket]

I’d say it depends on whether you’re still thinking of namespace as a module level setting, as it is today and what I had in mind initially.

I notice above you shifted the bridge’s alias::tests namespace out of the cxx::bridge attribute and down to the extern C block, even though it does nothing there. Are you thinking of removing it from that attribute? If namespace remains a module level setting, then alias_namespace makes sense to me as a way to specify “this other Rust bridge module uses namespace X” to match the namespace in cxx::bridge. It would feel weird if we kept the namespace argument in cxx::bridge and then didn’t have a similar module-level attribute when composing bridges. I was imagining the other namespace attribute would be rarely used honestly, but I may be underestimating how often folks will use opaque C++ types from multiple namespaces in a single bridge?

Putting the namespace attribute at only the individual alias and extern block level is certainly the most flexible option if you have to pick one, but conceptually something seems off to me about it that I can’t put my finger on. It doesn’t seem like how namespaces are naturally used in C++, if that makes sense, but I could be wrong. Maybe it’s because it feels odd to have to repeat the attribute if I have a module with both some shared types and an extern C block that should use the same namespace? It could also just feel weird because this doesn’t have a clear C++ analogue; normally you’d never need to specify the namespace at both the type declaration and everywhere it’s included.

I also see from your example above that you seem to be favoring use decls rather than type aliases for pulling in shared types from other modules. Are you thinking of switching the current type alias support to that? I guess we’d have to without the module level attribute, to avoid needing lots of namespace attributes to pull in multiple shared types.

Should I drop these PRs and just let you implement, or are these useful to you?

[dtolnay]

I notice above you shifted the bridge’s alias::tests namespace out of the cxx::bridge attribute and down to the extern C block, even though it does nothing there. Are you thinking of removing it from that attribute?

#[namespace = ...] should be a hierarchical thing. If an item (fn, struct, enum, type, use) directly has a namespace attribute then that's the namespace for that item. Otherwise if the surrounding extern block (if any) has a namespace attribute then that's the namespace. Otherwise if the surrounding module has a namespace then we fall back to that. Otherwise assume ::.

I may be underestimating how often folks will use opaque C++ types from multiple namespaces in a single bridge?

I think this is the case. And it's not just about opaque C++ types, but all FFI items. It is not unusual for a function's argument types or return type to be from a different namespace as the function. I indicated in #298 (review) that I think the actual problem to solve is how to allow one bridge to involve items from more than one namespace, after which a resolution for type aliases across bridges would fall out naturally.

It doesn’t seem like how namespaces are naturally used in C++, if that makes sense, but I could be wrong.

I would say I don't see this as a design goal. The way to view a cxx::bridge module is just as describing what exists in Rust and C++ that each side needs to know about the other. What matters to me is that the description is straightforward such that the individual pieces of functionality implemented by cxx::bridge are easily comprehensible in isolation and compose intuitively, not that it necessarily resembles semantics of C++ as closely as possible.

I also see from your example above that you seem to be favoring use decls rather than type aliases for pulling in shared types from other modules. Are you thinking of switching the current type alias support to that?

Yes; the ExternType::Kind associate type/marker types from #325 make this possible because we now have a way for a downstream module to reflect on whether a particular symbol from outside the module is an opaque (extern) type or one which both Rust and C++ can hold by value.

Should I drop these PRs and just let you implement, or are these useful to you?

The advantage you have is potentially more time able to dedicate to cxx, since I am also on the hook for rust-lang/rust and 100 other projects maintenance at the same time.

One thing that worked well in #325 was a substantial amount of back-and-forth in #313 to iterate and exactly pin down the design beforehand, such that the PR was implementing a design that we both already agreed was good.

For #298 (review) there is a chain of reasonable implementation steps which I'd be working through at whatever rate I'm able, but if a PR exists, I would prioritize accepting that.

• Emit all mentions of a type in generated C++ code (in function arguments and so forth) as a fully qualified path ::name::space::of::Type, even if it currently always happens to be within the same module.

• Store a distinct Namespace on each FFI item rather than having a single global namespace for the whole code generation.

• Implement #[namespace = ...] attribute to determine namespace of individual items.

• Implement hierarchical handling of namespace which prioritizes item-level attribute > extern block > module.

• Emit ExternType impls for shared structs.

• Safe ExternType derive for extern Rust items. extern "Rust" { #[derive(ExternType)] type TheType; }

• Permit use inside of cxx::bridge.

[dtolnay]

I've copied the implementation plan from the previous comment to #353, so I'll close the PR in favor of tracking it there.

Thanks anyway!

[sbrocket]

I’ve been on vacation this week. Will read above in more detail and see if there’s ways I can contribute next week when I’m back.