Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Design C++ namespace scheme #46

Closed
dtolnay opened this issue Feb 26, 2020 · 3 comments · Fixed by #48
Closed

Design C++ namespace scheme #46

dtolnay opened this issue Feb 26, 2020 · 3 comments · Fixed by #48

Comments

@dtolnay
Copy link
Owner

dtolnay commented Feb 26, 2020

#5 and #8 call out that prefixed names like RustString and RustBox are not idiomatic and we should be placing these types in a namespace instead.

Thinking only of the code that I would want downstream to be writing, the style I find most readable is something like:

void f(rust::Str s, rust::Box<Thing> b);

Some questions for @pravic and @slurps-mad-rips or anyone else:

  • I have the feeling it isn't appropriate for me to declare a top-level rust namespace with familiar type names like Box in it. That namespace "belongs" to the official Rust project. Is that feeling correct or is it not so bad?

  • I can define our types in cxxbridge::rust and recommend an import downstream to bring rust in scope when there isn't a collision with something else. Are C++ style guides generally okay with that or would people end up forced to write out cxxbridge::rust::... all over the place?

  • For the import, as far as I can tell it would be spelled using rust = cxxbridge::rust;. Is that right? There doesn't seem to be a way to avoid repeating "rust" such as using cxxbridge::rust;.

    main.cc:11:18: error: using declaration cannot refer to a namespace
    using cxxbridge::rust;
          ~~~~~~~~~~~^
    main.cc:11:18: error: namespace ‘cxxbridge01::rust’ not allowed in using-declaration
       11 | using cxxbridge::rust;
          |                  ^~~~
  • Any other thoughts on the namespace scheme? Thanks!

@bruxisma
Copy link

Responses in order of questions:

  • The only top level namespace owned by anyone is the posix namespace (reserved for the Austin Group, if they ever decide to create a C++ interface) and std. If you're really concerned, what you can do is namespace rust { inline namespace cxxbridge { /* declarations */ }} (in C++20, you can do namespace rust::inline cxxbridge {), This will mangle the cxxbridge symbols, and if anyone ever has an issue or a Rust toplevel namespace is ever used, you can make the inline namespace uninlined and already compiled code won't break. That said, I don't think ABI stability really matters for this project :)

  • People would most likely not be down for this. While some folks are fine with it, not everyone is. It's a personal preference, but at that point, why have the ::rust inner namespace?

  • Namespaces aren't treated like other declarations/using expressions in C++. You can do namespace rust = cxxbridge::rust or, using namespace cxxbridge. Both of these bring the rust namespace into scope, however the second form is looked down on, and the former approach is usually not liked because "why didn't you just make that the top level anyhow?". Most people end up doing namespace fs = std::filesystem in C++17 and it was enough of an issue that in C++23 (because we ran out of time for papers), we'll most likely have an alias namespace of std::fs 😅

  • Nothing else really, but keep in mind some people may want idiomatic C++ naming for the types as well. It wouldn't hurt to create type aliases that are all snake case. (i.e., template <class T> using box = Box<T>;)

@dtolnay
Copy link
Owner Author

dtolnay commented Feb 26, 2020

Thank you! That answer solves everything I wanted.

I'll go with namespace rust::inline cxxbridge01. You are right that ABI stability doesn't matter here, but rather ABI instability is a requirement. Since Rust allows multiple versions of a library to be linked together, and since this library claims to be safe, depending on multiple versions of it from one binary needs to behave safely.

And I would be happy to expose snake case type aliases. I am sticking with the current case as the default featured one in documentation for my-employer-is-always-right reasons and it matches https://google.github.io/styleguide/cppguide.html#Type_Names, but I sympathize with wanting lowercase names where the surrounding code uses them.

@bruxisma
Copy link

@dtolnay fair enough! I will say, keep in mind the ::inline syntax is C++20 and isn't available everywhere at this time. That said if you're trying to enforce ABI instability, I would recommend doing
namespace rust { inline namespace cxxbridge { inline namespace v<generate a hash/use a date/version here> {. This way hard linker errors will fire if the most inner inline namespace can't be found. Not the most empathetic approach to signaling what went wrong exactly to users, but it's better than bizarre runtime issues 🙂

That said, since you're at Facebook, you might want to reach out to Michael Park and Eric Niebler who are on the C++ committee. They might be able to provide additional guidance regarding C++ API design :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants