doc: update enumerator naming in developer notes #21930

Per our current doc, the general rule is we follow the C++ Core Guidelines, which for enumerator naming stipulate:

https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Renum-caps

Don’t use ALL_CAPS for enumerators
Reason: Avoid clashes with macros.

but our examples (and often, codebase) are contradictory to it (and perhaps to common usage), which can be confusing when a contributor needs to choose a style to use. This patch:

• updates the enumerator examples to snake_case per CPP Core Guidelines
• clarifies for contributors that in this project enumerators may be snake_case, PascalCase or ALL_CAPS, and to use what seems appropriate.

Concept ACK. I guess it should be clear from the context that those are compile-time constants. Also, IIRC it is not possible to name a enum value ERROR, because this conflicts with some windows macro in our cross build. Error might work.

Concept ACK.

Mildly disagree with this change. I think it's nice to be able to declare variables that change at runtime as variable, and declare variables that don't change as CONSTANT, and just use the choice to try to make code more understandable. It's possible a constant could conflict with a preprocessor macro, though, and this could be bad. I don't check for preprocessor macros under my bed every night, and I'm not too scared of them, but maybe I should be! So ignore this feedback if this seems like the clear way to go.

Why not, considering we use enum class.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

No conflicts as of last run.

ACK if this better matches what is in actual use in the project.

"clashes with macros" wouldn't be so much of an issue of C++ namespaced the things, but it doesn't, and as @ryanofsky already jokes they can just pop up from the kernel headers or libc headers out of nothing.

Checked the books/literature (Stroustrup, Meyers, Lippman, Moo, etc.) and they mostly use lowercase_underscore (snake_case) with a couple of the C++98 era ones using PascalCase (UpperCamelCase).

The Google style guide (https://google.github.io/styleguide/cppguide.html#Enumerator_Names) is a bit different but "use kEnumName not ENUM_NAME" for new code since 2009 for the same reasons.

Currently in this project it seems to be mostly ALL_CAPS, or in newer code lowercase (in src/bench/nanobench.h) or PascalCase (git grep -A5 "enum class ConnectionDirection").

I don't have a strong opinion (on my own I'd probably use lowercase_underscore (snake_case) with scoped enums as AFAICT it's widely used nowadays) but I think the important thing is to say which naming to use and to clarify if our choice is different from the CPP Core Guidelines so people aren't confused on which style to use.

I don't have a strong opinion (on my own I'd probably use lowercase_underscore (snake_case) with scoped enums as AFAICT it's widely used nowadays) but I think the important thing is to say which naming to use and to clarify if our choice is different from the CPP Core Guidelines so people aren't confused on which style to use.

Again just an opinion to throw in the mix, but I like CONSTANT<<3/2 to emphasize unchangedness of expressions when enums are used as collections of constants in bitwise or arithmetic code, and I like Confused NotConfused NotConfusedAndHungry ConfusedButSleepy for readability of english words in logical / event-handling code that more closely resembles natural language. I would not want to codify this and think it's fine to have guidelines that say there is no rule, and please do what seems appropriate.

guidelines that say there is no rule, and please do what seems appropriate.

Yes, that could be a helpful clarification too.

If there is no rule, we shouldn't be mentioning it

Per the current doc, the general rule is we follow the CPP Core Guidelines, but our examples are contradictory to it (and perhaps to common usage). I found this confusing.

Re-pushed an update that combines @ryanofsky's suggestion, open to feedback.

ACK 2d813c42ed5573d3afd80d677e2ded41e8d5b7e8. This looks good to me and it seems nice to move closer to cpp core guidelines and link to them even if not adopting them wholly.

Feel free to ignore my review suggestions below. I think they help but they're just my opinions and none are important.

Updated per @ryanofsky feedback (thanks!) and rebased for #18568.

ACK 77f37f58ad2f349cecb2eda28b415267d3d7d76e. I think this is good because it modernizes the example and clarifies current conventions.

cr ACK 77f37f58ad2f349cecb2eda28b415267d3d7d76e

Agree with @ryanofsky regarding:

[…] it seems nice to move closer to cpp core guidelines and link to them even if not adopting them wholly.

ACK 77f37f58ad2f349cecb2eda28b415267d3d7d76e.