autocxx icon indicating copy to clipboard operation
autocxx copied to clipboard

We do not uniqueify cxx::bridge names except for functions

Open adetaylor opened this issue 4 years ago • 6 comments

This error encountered on an internal codebase:

"thread '<unnamed>' panicked at 'Unable to generate header and C++ code: Syn(Error("the name Key is defined multiple times"))', /src/main.rs:189:18"

Working on reducing now.

adetaylor avatar May 14 '21 19:05 adetaylor

Minimized test case:

namespace a {
namespace spanner {
class Key;
}
} // namespace a
namespace spanner {
class Key {
public:
  bool b(a::spanner::Key &);
};
} // namespace spanner

adetaylor avatar May 16 '21 02:05 adetaylor

This problem is understood. It's because, for functions, we're using a bridge_name_tracker to ensure that entries in the (flat) cxx::bridge namespace are unique. We're not doing that for typedefs (which is what goes wrong here) not for structs/enums.

adetaylor avatar May 16 '21 21:05 adetaylor

I'm now thinking that we should "simply" mangle all names in the cxx::bridge mod to include namespaces. To avoid conflicts in the cxx::bridge output Rust code, we'd need to avoid using #[rust_name]. We can use RustRenameStrategy::RenameInOutputMod to get them back to the desired name.

It would be ideal if we could come up with a name mangling scheme which, for simple names, happens to match user expectations. This would significantly help with debugging when people were looking through complex call stacks across languages.

e.g.

  • A (in root namespace) => A
  • foo in bar namespace => bar_foo

but then we would need something different for a type literally called bar_foo or a struct foo nested inside a struct bar.

So the alternative is indeed to have a bridge_name_tracker as originally planned, which allocates a new name for each thing, and that new name is stored with the thing in the API list. This remains a bit fiddly, so if we can make a deterministic mangling scheme good enough, maybe we should.

adetaylor avatar Feb 08 '22 06:02 adetaylor

Just wanted to say that I'm encountering this issue with multiple definitions of cxxbridge1$new_autocxx_autocxx_wrapper in gen3.cxx and gen1.cxx as well as cxxbridge1$GetState_autocxx_wrapper in gen10.cxx and gen7.cxx.

Is it possible for the names to be mangled based on the gen # in the file name? When I cargo clean and rebuild the names of the gen files are the same, so it seems like it would be a predictable way to mangle the names. Could just append _gen3 and _gen1 to the end maybe?

VirxEC avatar Feb 01 '23 18:02 VirxEC

That feels to me like a slightly different problem than the one here - the current issue is about multiple conflicting names within a single include_cpp! whereas it sounds like you have a conflict between multiple include_cpp! macro invocations. Please could you raise a new issue to avoid muddling this one?

adetaylor avatar Feb 01 '23 18:02 adetaylor

Sure haha

VirxEC avatar Feb 01 '23 18:02 VirxEC