[llvm-dev] RFC: changing variable naming rules in LLVM codebase

[David Greene via llvm-dev]

via llvm-dev <llvm-dev at lists.llvm.org> writes:

> Looking at names of "variables" there's reasonable support for making 
> them visually more distinct from other kinds of names.  Regarding 
> making data-member names distinct from local variables, some people 
> don't see why it should matter, others find it helpful; given this 
> neutral-to-helpful spectrum, going with the kind-of helpful convention 
> seems preferable.

There are drawbacks to the "m_" prefix notation, though.  It makes it
more work to move entities between class and local scope.  It's extra
typing, it's hard to pronounce, etc.

Now, these may be considered trivial complaints, but given that some of
them have already been raised, I think we need some discussion about it.
If there's overwhelming support for "m_" (and/or "g_" etc.) then fine,
but if there's about even opinions either way, we ought to go with the
status quo, at least for now.

> So:
>
> - Local variables and formal parameters should be lower_case, with
>   one exception: Variables/parameters that have lambda/function
>   type should follow the function-name spelling rules.

"lower_case" is a pretty drastic change from CamelCase and camelCase.
So far the only argument for it I've seen is, "lldb uses it all over the
place."  Having one subproject drive the standard for everything else
seems backward.  It's certainly possible I missed other opinions on this
though.

> - Initialisms and other abbreviations would be considered words for 
>   this purpose, so we have names such as:
>     tli   // Local variable for TargetLoweringInfo
>     m_cgm // Data member for CodeGenModule

I actually think we need something stronger.  Acronyms should be
discouraged unless it's absolutely clear what it is.  As has been
pointed out, "tli" means at least two common and very different things:
"TargetLoweringInfo" and "TargetLibraryInfo."

> - I don't have a good suggestion for file-static/global variables.
>   Some people have suggested a "g_" prefix for globals, or possibly
>   an "s_" prefix for class-static data.

Is this really helpful or does it just make the code more difficult to
work with?  I don't know since I've never used such conventions before.
But I'm hesitant to introduce really strong binding of scopes to names
because it make refactoring more tedious/difficult.

> Some people have worried that the churn will cause blame issues.
> I respectfully point out that in my own archaeology I have to deal
> with lots of clang-format/indentation/other random semantically
> meaningless refactoring, this is just one more.  Also the point is
> not to optimize for git-blame but to optimize for reading what is
> there at the moment.

I want to call out that we already have a policy on this in the current
coding standards:

  Our long term goal is for the entire codebase to follow the
  convention, but we explicitly do not want patches that do large-scale
  reformatting of existing code.

I could argue things either way, but we should realize that a mechanical
reformatting goes explicitly against current stated (and presumably
previously-agreed-upon) policy.

                               -David