[llvm-dev] RFC: changing variable naming rules in LLVM codebase (original) (raw)

David Greene via llvm-dev llvm-dev at lists.llvm.org
Fri Feb 22 11:40:07 PST 2019


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 lowercase, 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 mcgm // 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


More information about the llvm-dev mailing list