Alex Denisov via llvm-dev
2019-Feb-12 21:17 UTC
[llvm-dev] changing variable naming rules in LLVM codebase
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
Krzysztof Parzyszek via llvm-dev
2019-Feb-12 21:28 UTC
[llvm-dev] changing variable naming rules in LLVM codebase
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 >
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