David Greene via llvm-dev
2019-Feb-12 21:47 UTC
[llvm-dev] changing variable naming rules in LLVM codebase
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
Zachary Turner via llvm-dev
2019-Feb-13 00:47 UTC
[llvm-dev] changing variable naming rules in LLVM codebase
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 >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190212/67ba83e8/attachment-0001.html>
Chandler Carruth via llvm-dev
2019-Feb-13 00:56 UTC
[llvm-dev] 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>