llvm dev - Feb 2019 - changing variable naming rules in LLVM codebase

FWIW, I'm pretty strongly opposed to humbleCamelCase. We already use that
style so something else. One of the biggest advantages of changing the
variable naming convention would be to differentiate variables from other
constructs IMO, and that's the nature of many examples here.

Using underscore_names for variables as Zach suggests would,, onhe other
hand, be a significant improvement IMO.

That said, all of these changes are pretty dramatic and expensive. I'm
personally in favor but you'd want a *lot* of buy in from the community as
well as some really good tooling I think to help people update code they're
about to make substantial changes to prior to those changes, much like we
often do with clamg-format.



On Tue, Feb 12, 2019, 16:47 Zachary Turner via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> One additional consideration is that LLDB currently uses underscore_names.
> It might be worth considering that style on these grounds alone, as that
> would bring a very large existing part of LLVM into conformance
> On Tue, Feb 12, 2019 at 1:47 PM David Greene via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> It very much depends on what is following the code snippit.  If the
>> second "if" is guarding a substantial block of code,
"constantExpr"
>> might very well be a good name.  Otherwise something like
"cExpr" or
>> "constExpr" might be fine.
>>
>> In the past when I have seen things like "CE" in the code,
it's not
>> always immediately clear to me what it is.  I have to go find the
>> declaration.
>>
>>                              -David
>>
>>
>> Krzysztof Parzyszek via llvm-dev <llvm-dev at lists.llvm.org>
writes:
>>
>> > The reason is clear: the variable name in such a context
doesn't add
>> > anything, since it's obvious what it is. Long names should be
used
>> > where meaning needs to be conveyed, otherwise they just obfuscate
the
>> > code needlessly.
>> >
>> > -Krzysztof
>> >
>> > On 2/12/2019 3:17 PM, Alex Denisov via llvm-dev wrote:
>> >> I would assume that the proper name in this case is
constantExpr, and
>> not CE.
>> >> This is not really an acronym, but rather a shortcut taken for
some
>> unclear reason.
>> >>
>> >>> On 12. Feb 2019, at 13:02, Björn Pettersson A via llvm-dev
<
>> llvm-dev at lists.llvm.org> wrote:
>> >>>
>> >>> (Sorry if this subject already has been discussed, but I
could not
>> find any clear rules/recommendations.)
>> >>>   What would the recommendation be for acronyms (I’ve seen
the rule
>> >>> about avoiding them unless they are “well known”,
>> >>> but sometimes an acronym is useful, and we at least need
to have some
>> recommendation for the “well known” ones).
>> >>>   Example:
>> >>>        if (ConstantExpr *CE =
dyn_cast<ConstantExpr>(V))
>> >>>        if (CE->getOpcode() ==
Instruction::GetElementPtr &&
>> >>>            CE->getOperand(0)->isNullValue()) {
>> >>>   In the above example, is the recommendation to use “ce”
instead
>> >>> of “CE” now? Or should it be “cE”?
>> >>> With lowerCamelCase one might think that “cE” is the
correct one
>> (although I personally think that one looks quite ugly).
>> >>>   Maybe there should be an exception that variable names
that start
>> >>> with an acronym still should start with an upper case
letter?
>> >>>   /Björn
>> >>>     From: llvm-dev <llvm-dev-bounces at
lists.llvm.org> On Behalf Of
>> >>> Michael Platings via llvm-dev
>> >>> Sent: den 7 februari 2019 23:11
>> >>> To: llvm-dev at lists.llvm.org
>> >>> Cc: nd <nd at arm.com>
>> >>> Subject: [llvm-dev] RFC: changing variable naming rules in
LLVM
>> codebase
>> >>>   TL;DR: change the rule for variable names from
UpperCamelCase to
>> >>> lowerCamelCase.
>> >>>
>> >>>
>> >>>
>> >>> Just to get wider visibility on this, I'm raising this
again as an
>> RFC, as suggested by Roman Lebedev.
>> >>>
>> >>>
>> >>>
>> >>> My original post from last week is here and gives a
rationale:
>> http://lists.llvm.org/pipermail/llvm-dev/2019-February/129854.html.
>> There seemed to be general agreement that the status quo is not ideal.
>> >>>
>> >>>
>> >>>
>> >>> Chris Lattner pointed out that this came up in 2014 as
well:
>>
http://lists.llvm.org/pipermail/llvm-dev/2014-October/077685.html>>
>> >>>
>> >>>
>> >>> I've created a patch to implement the change. Review
and comments
>> welcome: https://reviews.llvm.org/D57896>>
>> >>> _______________________________________________
>> >>> LLVM Developers mailing list
>> >>> llvm-dev at lists.llvm.org
>> >>>
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>> >> _______________________________________________
>> >> LLVM Developers mailing list
>> >> llvm-dev at lists.llvm.org
>> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>> >
>> > _______________________________________________
>> > LLVM Developers mailing list
>> > llvm-dev at lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190212/fd276401/attachment.html>

Chandler Carruth <chandlerc at gmail.com> writes:
> FWIW, I'm pretty strongly opposed to humbleCamelCase. We already use
> that style so something else. One of the biggest advantages of
> changing the variable naming convention would be to differentiate
> variables from other constructs IMO, and that's the nature of many
> examples here.
I guess I don't see a lot of point to being different for difference's
sake.  Functions use humbleCamelCase but it's fairly easy to
differentiate a function from a variable at the point of use, unless one
is referencing a function's address in which case it kinda is like a
variable in that context.

humbleCamelCase also has the advantage of removing the weird special
case for lambdas, where in one sense it's a function and in another
sense it's a variable.

What are the current uses of humbleCamelCase that concern you?

In the end I don't really care what the convention is.  I'm not sure a
mechanical updating of the source is worth it, as that can make using
git blame slightly inconvenient.

                               -David

Chandler wrote:
> FWIW, I'm pretty strongly opposed to humbleCamelCase. We already use
that
> style so something else. 
Presumably you are equally opposed to RegularCamelCase, because we already
use *that* style for something else.

But really, objecting on the grounds that a given style is already used for
function names is really a very weak argument.  IME function names are 
*incredibly* *hard* to confuse with anything else, because they *always* have 
surrounding syntactic context. Given `TheStuff->fooBar().getThingy()` is it
even conceivable that you might not instantly get that fooBar and getThingy
are methods?  Therefore, using the same convention for some other kind of
name is Not Confusing.

OTOH, `TheStuff` comes out of nowhere with no clues to its origin, and *that*
is a barrier to code-reading IME.  Even renaming it to `stuff` would help
approximately zero percent. Parameter? Local? Class member? Global?  LLVM has 
incredibly few globals for other reasons, but using the same convention for 
locals and class members is a real problem for code-reading, especially code 
operating in methods for classes you're not super familiar with.

I acknowledge that the current RFC doesn't propose a member naming
convention
different from other variables, but IMO it really ought to.  *That* is the
distinction that would really help in reading unfamiliar code.
--paulr

I want to reiterate the benefit that underscore_names would bring.  To be
clear it's not my favorite style, but it does have a very concrete
advantage which is that we have a very large subproject already using it.
it doesn't make sense to do a purely aesthetic move that not everyone is
going to agree on anyway, when we could do one with actual tangible value.

On Wed, Feb 13, 2019 at 8:52 AM <paul.robinson at sony.com> wrote:
> Chandler wrote:
>
> > FWIW, I'm pretty strongly opposed to humbleCamelCase. We already
use that
> > style so something else.
>
> Presumably you are equally opposed to RegularCamelCase, because we already
> use *that* style for something else.
>
> But really, objecting on the grounds that a given style is already used for
> function names is really a very weak argument.  IME function names are
> *incredibly* *hard* to confuse with anything else, because they *always*
> have
> surrounding syntactic context. Given `TheStuff->fooBar().getThingy()` is
it
> even conceivable that you might not instantly get that fooBar and getThingy
> are methods?  Therefore, using the same convention for some other kind of
> name is Not Confusing.
>
> OTOH, `TheStuff` comes out of nowhere with no clues to its origin, and
> *that*
> is a barrier to code-reading IME.  Even renaming it to `stuff` would help
> approximately zero percent. Parameter? Local? Class member? Global?  LLVM
> has
> incredibly few globals for other reasons, but using the same convention
> for
> locals and class members is a real problem for code-reading, especially
> code
> operating in methods for classes you're not super familiar with.
>
> I acknowledge that the current RFC doesn't propose a member naming
> convention
> different from other variables, but IMO it really ought to.  *That* is the
> distinction that would really help in reading unfamiliar code.
> --paulr
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190213/934259df/attachment.html>

On Wed, Feb 13, 2019 at 6:52 AM <paul.robinson at sony.com> wrote:
> Chandler wrote:
>
> > FWIW, I'm pretty strongly opposed to humbleCamelCase. We already
use that
> > style so something else.
>
> Presumably you are equally opposed to RegularCamelCase, because we already
> use *that* style for something else.
>
I would be opposed to going through the very significant cost of *changing*
the naming convention, merely to end up there.

The convention we already use has a huge advantage of already being
relatively consistent.

>
> But really, objecting on the grounds that a given style is already used for
> function names is really a very weak argument.  IME function names are
> *incredibly* *hard* to confuse with anything else, because they *always*
> have
> surrounding syntactic context. Given `TheStuff->fooBar().getThingy()` is
it
> even conceivable that you might not instantly get that fooBar and getThingy
> are methods?  Therefore, using the same convention for some other kind of
> name is Not Confusing.
>
I disagree FWIW... Lambdas (and callables generally) at the least make this
ambiguous.

I think the fact that named things are called does not fully disambiguate
what they are.

I'm not trying to say that the collision with functions is *as* confusing
as that of colliding with types. Merely that both seem confusing. And I
find `foo_bar_baz` and `fooBarBaz` basically equivalent[1]. So between
those equivalents, I would choose the one with fewer collisions.

[1]: Ok, not quite, but I find this to be a more personal preference and am
trying to weight it lower as a consequence. I find functions much more
similar to types -- they are manifest properties of the program. I find
`FooBarBaz` and `fooBarBaz` to be very similar looking. There is *a*
distinction, but it is a minor one. I would prefer a greater visual
difference for variables, which `foo_bar_baz` provides.

OTOH, `TheStuff` comes out of nowhere with no clues to its origin,
and
> *that*
> is a barrier to code-reading IME.  Even renaming it to `stuff` would help
> approximately zero percent. Parameter? Local? Class member? Global?  LLVM
> has
> incredibly few globals for other reasons, but using the same convention
> for
> locals and class members is a real problem for code-reading, especially
> code
> operating in methods for classes you're not super familiar with.
>
> I acknowledge that the current RFC doesn't propose a member naming
> convention
> different from other variables, but IMO it really ought to.  *That* is the
> distinction that would really help in reading unfamiliar code.
>
I could see a lot of utility of this, but I do find it to be orthogonal.

> --paulr
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190218/6024eef4/attachment.html>