Implement the Google variable naming style more faithfully.

[fruffy]

Fixes #4404.

The closest approximation of the Google style I could find is defined here: https://github.com/googleapis/google-cloud-cpp/blob/main/.clang-tidy#L104

We deviate quite a bit. For example, we use camelCase in most places instead of CamelCase and snake_case for variables. I do not have a strong opinion on the style except that we should decide for one and stick to it. These days clang-tidy is advanced enough to actually enforce it, which is nice.

Pinging some people on this PR to get agreement. This will be helpful in reviews going forward.

[ChrisDodd]

We tend to use snake_case for private/internal members (including for members that are "logically" private but exposed publicly for logging/debugging). Not sure if clang-tidy provides a way to note that.

[fruffy]

Yes, specific case for private members is possible with https://clang.llvm.org/extra/clang-tidy/checks/readability/identifier-naming.html#cmdoption-arg-PrivateMemberCase

But marking a public member logically private is not.

[fruffy]

At least in the compiler front end and libraries, the common style seems to be camelCase for everything. We can easily change that with an automated clang-tidy run. But that will assuredly break downstream back ends and repositories.

[asl]

Do we want to add abseil-* checks here?

[fruffy]

Do we want to add abseil-* checks here?

Sure.

[fruffy]

@ChrisDodd

With type you mean a typedef?

typedef or nested struct

Unfortunately, there is no way to distinguish between a private and public typedef for now. But we could add the _t suffix.

However, the current configuration here uses the prefix style. So _camelCase for private member variables instead of lower_case. This might be more consistent than snake case for only private members?

This is what it would look like:

Classes/Structs/NameSpaces/TypeDefs: PascalCase
Functions/Methods: camelCase
Public/local variables: camelCase
Private variables: _camelCase

For reference, other styles:
Google C++ style is:

Classes/Structs/NameSpaces/TypeDefs: PascalCase
Functions/Methods: PascalCase
Public/local variables: snake_case
Private variables: _snake_case?

C# style is:

Classes/Structs/NameSpaces/TypeDefs: PascalCase
Functions/Methods: PascalCase
Public/local variables: PascalCase
Private variables: _camelCase?

Java Style

Classes/Structs/NameSpaces/TypeDefs: PascalCase
Functions/Methods: camelCase
Public/local variables: camelCase
Private variables: camelCase

[vlstill]

I don't have many strong opinions about naming style. Personally I find _t suffix annoying and prefer uppercase for types which matches with the proposed style. But mainly I am for naming consistency.

Do you prefer to enforce this (by a CI check)?

Did you try how large changes are needed to make the codebase conform?

[fruffy]

Do you prefer to enforce this (by a CI check)?

We are still missing Clang-tidy support for this (#4254).

Did you try how large changes are needed to make the codebase conform?

There are quite a few changes needed across the codebase, but I'd say we can just gradually fix them. Just having the clang-tidy standards will encourage developers to use the right naming scheme.

[vlstill]

Do you prefer to enforce this (by a CI check)?

We are still missing Clang-tidy support for this (#4254).

Would the clang-tidy still run very slowly if you disabled all the checks except for the formatting once? Maybe then it would be manageable.

Of course, if we wanted enforce it we would also have to re-format whole codebase at once.

[fruffy]

Would the clang-tidy still run very slowly if you disabled all the checks except for the formatting once?

Yes, I have not found a way yet. But even having it as part of CMakelists is already useful for cleanup, e.g., #4326

Of course, if we wanted enforce it we would also have to re-format whole codebase at once.

I do not think this is necessary, we can do this gradually. Unlike clang-format these changes are more sensitive.