Discussion: Style guide for constant naming in code

Historically, Blender codestyle was following the ‘macro’ naming convention (ALL_CAPS) for its constants (actual defines, and enum items).

Once C++ code started to be added to the project, part of the codebase has ‘silently’ switched to using regular variable names convention (snake_case) for some constants (in particular const expr ones), and/or the ‘class’ naming convention (PascalCase), especially for its enum class items.

There were already some discussion about this topic over one year ago, but no conclusion was reached.

We are getting more and more mixed style in the codebase, and need to solve this one way or the other at that point.

NOTE: This is somewhat a follow-up/rehashing/generalization of previous discussions like Style for enumerator values and its related PR.

Scope

This is a discussion to decide about a code style to enforce on compile-time constants (i.e. macro defines, constexpr and enum items). runtime constants (const keyword) are not considered here.

Note: each naming styles have various names, using wikipedia as reference here.

Potential Solutions

There are at least three ideas I am aware of currently:

  • Go back to using ALL_CAPS.
  • Move everything to snake_case (and potentially UpperCamelCase for enum items).
  • Use the ‘google’ style, i.e. using a prefix (k): kCamelCase.

ALL_CAPS

This is the “historic Blender” style. All constant should use this same style.

Here is a mockup PR enforcing this style in blenkernel module.

Pros:

  • Allows to immediately identify constant.
  • Still followed by the majority of Blender codebase currently.

Cons:

snake_case & UpperCamelCase

These have been used in more recent Blender C++ code quite extensively. While not documented anywhere (to my knowledge), it seems to follow the following rules:

  • macros and anonymous enums use ALL_CAPS.
  • constexpr use snake_case.
  • Scoped enum class items use UpperCamelCase.

Here is a mockup PR enforcing this style in blenkernel module.

Pros:

Cons:

  • Uses various styles for different types of constant.
  • Does not immediately convey the ‘is a constant’ meta-information when reading code.

Google Style

This style is not used in Blender code currently. In a nutshell, it reuses the ‘Hungarian notation’ idea to mark all non-macro constant names with the k prefix, and camel case: kCamelCase.

Pros:

Cons:

  • Would be the most disruptive change in our codebase.
  • Still two different notations for constant (ALL_CAPS macros and kCamelCase for enum and constexpr).

Tooling

We should try to have automation to enforce these rules, once we agree on them. clang-tidy seems like a good candidate (see also the two mockup PRs linked above, which where generated using it).

This implies that we may be limited in our final style choices, by what the chosen tool can do.

1 Like

Note: This initial post represent my current understanding of this topic. I most likely missed or forgot some aspects of it.

I’d suggest to approach this discussion in two steps:

  • First agree on current state and pros and cons of the proposed styles.
  • Only then discuss which one we should go for.

In my opinion scoped constants (enum class) should use UpperCamelCase. It’s much more readable, and the scoping and context makes it clear that the value is a constant.

Beyond that, I personally don’t care if we decide whether to use upper case or snake case for constants. I would really rather not have some prefix like “k” or “e” though. I don’t think scoped enums and global/local constants need to follow the same style.

For me the important thing is to see constants at a glance, without having to hover or search deeper.

snake_case for global constexpr seems quite bad to me, hard to distinguish from local variables. And I’m not sure Hungarian notation is popular among Blender developers at all?

I don’t care too much about the enum class case, as it’s easy to distinguish locally regardless if it’s ALL_CAPS or CamelCase.

I think template value arguments must be also mentioned, and i really hate template<int COUNT>`

The finalized guidelines (see also this Coordination module’s notes) has been published, thanks to everyone for the discussions.

New rules go against a style of all bli constexpr variables. At this point now is_same_any_v is wrong and should be `IS_SAME_ANY_V`… how did that thing was accepted at all

As was stated in the first post? I don’t really see the problem with this. Whichever way you choose things will have to get changed to make it consistent.

Visually the ALL_CAPS is a bit shouty maybe, but I really like that it’s immediately clear if something is constant. is_same_any_v doesn’t tell me anything. Looks like a local variable.

We are talking about constants here…
Though technically a templated constexpr is a constant at compile-time, from a coding perspective it’s not really, since its ‘result’ value will differ depending on its template ‘parameters’?

`.. whether they are macro `#define` or `constexpr``

So, do we apply `AAA_AAA` to all constexpr values or this was only about runtim constants and templates?

You didnt get my point: all why use them did not used that style at all.

This applies to constexpr that are global or class constants. constexpr is a language feature that can be used for other things than ‘plain’ constants (like in your templated example), just like macros can be used for many other things than #define-ing constants. Once again, I would not consider/call is_same_any_v a constant.

This is more worse now.. can define in function body also fallow variables name style? why layout affects name style?

Ah, sorry.I misunderstood.

1 Like