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

[Sanjoy Das via llvm-dev]

On Mon, Feb 18, 2019 at 2:16 AM Michael Platings via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> Regarding a plan for conversion, I'm keen to avoid perfect being the enemy of better.
>
> Privately, people I've spoken with have told me that they're opposed to a large scale conversion. Reasons given include breaking git blame, and creating needless merge conflicts. I might be wrong, but the evidence I've seen suggests that it's going to be very hard to get consensus on a conversion.
>
> So what's worse: inconsistent capitalization or keeping a convention that discourages good naming?
>
> Taking my previous example [1]:
>
> InnerLoopVectorizer LB(L, PSE, LI, DT, TLI, TTI, AC, ORE, VF.Width, IC,
>   &LVL, &CM);
>
> If we imagine that over time it evolves such that 50% of the variables have been renamed to camelBack versions of the type names, then it will look like this:
>
> InnerLoopVectorizer LB(loop, PSE, loopInfo, DT, targetLibraryInfo, TTI,
>   assumptionCache, ORE, vectorizationFactor.Width, IC,
>   &loopVectorizationLegality, &CM);

I find myself less productive in a codebase with inconsistent styling
like you show above because it is more difficult to "guess" the name
of a variable.  E.g. is the LoopInfo parameter named LI or loopInfo?
I'll have to double check to be sure, which adds an extra step.

So maybe a gradual transition plan could be to allow these upper case
acronyms for specific classes?  For instance we could start by
designating a set of "common" classes like Function, BasicBlock
DominatorTree, LoopInfo, ScalarEvolution whose instances would
instances still be called F, BB, DT, LI and SE, but mandate all other
classes should use the new camelCase convention to name their
instances?  I think this helps readability since the reader will know
that "BB" is always the basic block, "F" is always the function etc.
And I don't have the "guessing" problem I mentioned above since
VectorationFactor instances are always "vectorizationFactor" and
LoopInfo instances are always "LI". We could then try to shrink this
set down over time (or not).

-- Sanjoy

>
> Yes it's inconsistent, but IMHO it still conveys so much more information than the original that the benefit greatly outweighs the cost.
>
> So is the disruption implied by changing to the new convention justified? I think so.
>
> [1] https://github.com/llvm/llvm-project/blob/ec0263027811f48b907224ede0dd24c33c1c7507/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp#L7433
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev