autocxx

autocxx copied to clipboard

The naming convention that just used the C++ function name directly doesn't work with multiple include_cpps in the same binary. The test case has them in the same rust file, which could be solved other ways, but ultimately we may link the generated C++ code from multiple Rust files which means the names need to be globally unique.

Thanks for this!

I'm aware that this is a problem with multiple mods in the same linked binary. I'm not yet sure that this is how I want to solve it.

Thoughts:

1. Your proposed change isn't sufficient if there are several types called A in different mods.
2. For make_string, we therefore make the name unique per mod by using a hash of the configuration on the symbol name. That would arguably be the "correct" thing to do for every symbol...
3. ... although even that doesn't work if there are multiple identical include_cpp! macros within the same crate (though if there is, I think it's reasonable to declare that such cases aren't supported).
4. However I'm not thrilled about mangling the symbol names for all cxx::bridge entries. That seems to add complexity for the 99% case where there's only one such mod. ("Complexity" is mostly but not entirely invisible to the user. For example added layers of function shims, more complex symbol names in stack traces.) I haven't figured out how to resolve this yet.
5. If we really do want to mangle all the cxx::bridge names like this, then we can probably lose some other complexity. Perhaps we always assume there's both a Rust-side and C++-side wrapper function.

Yea, managing the names in all these different namespaces is tricky...

1. Your proposed change isn't sufficient if there are several types called A in different mods.

Good point, I had it in my head that the namespace was included, but looking again it isn't.

2. For make_string, we therefore make the name unique per mod by using a hash of the configuration on the symbol name. That would arguably be the "correct" thing to do for every symbol...
3. ... although even that doesn't work if there are multiple identical include_cpp! macros within the same crate (though if there is, I think it's reasonable to declare that such cases aren't supported).

Could you include the filename and line number of the include_cpp! in the hash to fix that?

4. However I'm not thrilled about mangling the symbol names for all cxx::bridge entries. That seems to add complexity for the 99% case where there's only one such mod. ("Complexity" is mostly but not entirely invisible to the user. For example added layers of function shims, more complex symbol names in stack traces.) I haven't figured out how to resolve this yet.

Are there any un-mangled names right now? I might be not using some features, but I've got some fairly complex classes being wrapped and the only functions using this codepath that I see are special member functions which need wrappers anyways.

Could you include the filename and line number of the include_cpp! in the hash to fix that?

Unfortunately, getting such information out of a proc_macro::Span requires nightly, as far as I understand it. In any case, I think this is an extremely niche case.

Are there any un-mangled names right now? I might be not using some features, but I've got some fairly complex classes being wrapped and the only functions using this codepath that I see are special member functions which need wrappers anyways.

Hmm, there should be. For simple functions. But, fair comment, maybe we've added so many cases requiring wrappers that nearly all names get mangled one way or another already. I will do some experiments and report back!

Are there any un-mangled names right now? I might be not using some features, but I've got some fairly complex classes being wrapped and the only functions using this codepath that I see are special member functions which need wrappers anyways.

I gathered some figures

|| Example || Total names || Of which mangled with autocxx_wrapper || | chromium-fake-render-frame-host | 44 | 41 | | s2 | 68 | 44 | | steam-mini | 11 | 10 |

No conclusions yet! I think my next step might be to attempt to put together a PR which does indeed mangle all names, and see how much code we can discard. I did already try this in #785; see also https://github.com/google/autocxx/issues/486#issuecomment-1032263796, but I think I should revisit it.

It's possible that this specific test case is fixed by #1235 , but there are remaining problems - #1239.

Looks like my current use cases don't need this any more, so I'll close this. I'm not sure what combination of my code changing vs fixes in autocxx fixed it, but current autocxx builds my code without needing this.