During a recent admin meeting we discussed problems with our existing namespace names. This document proposes a change to how we name namespaces in the blenkernel and editors modules.
Problem
The topic came up because of name-collisions with the blender::seq, blender::ed::seq and potentially blender::bke::seq namespace.
However, these kinds of name-clashes are not new. We have the same with e.g. bke::curves and ed::curves. Often the name curves also collides with variable names resulting in yet more ambiguous local code. For example, when reading a line like the following, it’s not really clear what namespace curves:: refers to:
The blenkernel and editors modules are somewhat special compared to all other modules for two reasons:
They contain code for many distinct parts of Blender and thus have many sub-namespaces and will likely get more as more code becomes “proper” C++.
Code in these modules can always be moved to a sub-namespace. While we have some code in the bke:: namespace directly currently, this code typically makes more sense in e.g.bke::curves or bke::mesh. Afaik, there is no code directly in the ed namespace.
Proposal
The ambiguity of the namespace names together with the mentioned properties of blenkernel and editors leads to the following proposal to improve the situation:
Remove the bke and ed namespace.
Instead use namespace names of the form bke_curves, bke_mesh, ed_curves, etc.
Further nested sub-namespaces are not flattened. So there can be e.g. blender::bke_mesh::normals.
Outcome
This approach does have a couple of advantages:
Significant reduction in namespace name collisions resulting in less ambiguous code.
Less deeply nested namespaces.
Forces the use of sub-namespaces in bke. Currently, it’s a bit of a mix where some code is in sub-namespaces and some parts are not, without any obvious rule.
Potentially improved API discoverability with auto-completion, because starting with ed_ or bke_ should result in a list of available sub-modules, whereas currently bke:: would result in many different unrelated things. Searching for ed:: does not have this problem currently, because it already contains only sub-namespaces.
The main disadvantage is that it is a somewhat unconventional pattern for namespace names and it’s not a clean solution from a “C++ purist’s” point of view.
Feedback is welcome. This has not been agreed upon in the admin meeting yet, but the sentiment was overall positive.
@brecht@sergey@HooglyBoogly@julianeisel (and others): Would be useful to hear from you. During the Admin meeting we agreed again that moving forwards with the proposal would be good.
If there is no feedback in next 2 weeks, I’ll start implementing the proposal.
Personally I would suggest to drop bke:: or bke_ entirely though.
That would overall be fine with me too. It does imply that there is no difference between e.g. the node related stuff in blenkernel and that in the nodes module. Both would be in blender::nodes. Is that what you mean?
I’m personally really not a fan of the bke_ or ed_ prefixes for namespaces. It’s just using the semantics of namespaces all wrong. Maybe others are more comfortable ignoring the semantics for practical purposes but I would personally rather not. I find Brecht’s suggestion a much better compromise in this regard.
I also see value in the base blender::bke namespace. I find it helpful to distinguish “core” types that are at a different level than types defined in higher level modules like editors. So I think it would be good if this proposal was more specific about what we would do with many of our current namespace usages.
Honestly not entirely sure what we’re fixing here? collisions between namespaces? that should only happen if you use overly broad and gratuitous using’s
If we’re confused enough to not know if this is bke::curves or ed::curves just use the fully qualified name? Any future dev reading the code will thank you for typing those extra 4-5 chars.
The google C++ style guide forbids using-directives, didn’t see anything in our style guide, but i have to admit, i didn’t look that hard may have missed it.
That would be my choice too. I don’t see it as a big problem to have a few more characters when it’s necessary. But I also recognize that some developers find these collisions annoying. Though reading the text above I do also miss the reasoning for why it’s annoying.
I think we should look a bit outside of the problem of BKE and editors, and ensure we have consistent naming convention that works everywhere.
The current presentation is quite confusing, and does not feel related to the seq example. If we limit the discussion to bke and ed just stick to bke::foo and ed::bar.
Originally is started with the namespaces blender::seq (which is the “core” of sequencer) and blender::ed::seq. In the editor code to access the sequencer “core” you’d end up using blender::seq::foo everywhere, which is quite verbose.
To me it seemed like the use and nesting of namespaces started to be something we are fighting with, and not something that comes naturally.
And another thing which I was initially raising is the depth of namespaces. I am not sure it actually helps.
Indeed, but that is the simple case, which does not answer how to solve the problem that originally kick-started the discussion.
To me if we’d need to be using blender:: it will indicate we’re doing something wrong.
I’d be quite careful with this, and I’d really like to see a more complete proposal of the namespaces. I don’t want to kick-start process of removing the module prefix as it will make it harder to see bad level calls at a glance.
Indeed!
Currently to me the proposal is a big vague, and there are some good points raised here, and some points that I can not say I agree with (but also maybe I misinterpreting them).
It shouldn’t take much time to map the current modules and directories and visualize the proposed namespace changes.
Coming from Rust land, deeply nested namespaces seem fine to me. However, I think that’s partly because Rust’s equivalent of using isn’t a glob import like it is in C++.
For example, if you have the namespace blender::bke::anim::fcurve, you would likely do this if you planned to use things from that namespace a lot:
use blender::bke::anim::fcurve;
// ...
fcurve::some_function(a, b, c);
fcurve::some_other_function(a, b, c);
It’s clear where all the identifiers in the file come from (modulo the incredible lack of clarity due to #include, of course).
You preserve just the part of the namespace path needed to distinguish things appropriately in the context of the given source file. This keeps things unambiguous without being too verbose/long.
So I would advocate for keeping nested namespaces, and ditching (the IMO poorly designed) using in favor of namespace aliases to trim namespace paths as appropriate.
@Cessen made my point in a much nicer way, but i’d likely go a bit further when headers are involved
in .cc files, yeah using in whatever form it may take, has advantages as it turns down verbosity a tiny bit there’s really no denying that, so while i’d like to advocate for eradicating using completely, not a battle i feel i can win.
Now public headers on the other hand, now that’s a different story, as they are essentially contracts with the outside world, i welcome the verbosity there, i’d demand it if i could! I don’t want to scroll up to the top of the file to see if there’s any usings when looking at a function signature, the fewer usings the better!