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

Wed Jul 10 04:12:39 PDT 2019

Rui,

I have created D64474 to change comments explicitly stating the parameter
names for constants ( /*parameterName=*/<constant> ). I did this by hand to
match the new variable names. Do you know if there would be a way to update
these comments with a tool similar to what you used to change these names?
Perhaps it would be much more difficult, I'm guessing clang's AST doesn't
have a way to describe comments? It's obviously not a huge deal to have
these changed it could be done over time.

Best,
Alex

On Wed, Jul 10, 2019 at 12:18 AM Rui Ueyama via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> Hi Joan,
>
> On Tue, Jul 9, 2019 at 9:46 PM Joan Lluch <joan.lluch at icloud.com> wrote:
>
>> Hi Rui,
>>
>> I’m totally positive on switching to camelCase convention. In fact I have
>> been always uncomfortable with the current naming approach.
>>
>> My only suggestion/concern is that we should provide a transition path
>> not only for the trunk code in the repository, but for eventual
>> out-of-trunk code with implementations of custom architectures. I have
>> currently a custom backend implemented on top of LLVM 9 and therefore this
>> change will surely break my code. I assume that developers affected by this
>> will be able to run the converting tools to fix their own code.
>>
>
> The tool that I wrote for lld's style conversion should work for
> out-of-trunk code, so as I described in the previous email, third-party
> code maintainer should be able to use the tool to convert the style first
> in their repositories and then rebase in order to avoid large merge
> conflicts.
>
> The tool needs to be polished to convert other subprojects such as clang,
> but I'll keep your request in mind. I'll try my best to provide a smooth
> transition path for out-of-trunk code for any change that I'll submit for
> the style change.
>
> John
>>
>> On 9 Jul 2019, at 09:23, Rui Ueyama via llvm-dev <llvm-dev at lists.llvm.org>
>> wrote:
>>
>> Thanks, Chris.
>>
>> Looks like everybody is at least mildly comfortable with my variable name
>> renaming change, so I'll to submit that change to lld subdirectory soon. If
>> you have any objections, please let me know. Note that this is not the end
>> of my effort but actually the beginning of it. As Chris said, I believe we
>> should do this to the entire LLVM codebase. I'm planning to do that
>> directory-by-directory basis.
>>
>> On Tue, Jul 9, 2019 at 2:03 PM Chris Lattner <clattner at nondot.org> wrote:
>>
>>> This looks really great to me Rui, and I’m also pleased to see the
>>> positive comments on the review thread.  Thank you for driving this forward!
>>>
>>> -Chris
>>>
>>>
>>> On Jul 4, 2019, at 9:50 PM, Rui Ueyama <ruiu at google.com> wrote:
>>>
>>> Hi all,
>>>
>>> I wrote a tool <https://reviews.llvm.org/D64123> to batch-rename
>>> variable names so that they are in camelCase, and I applied the tool to lld
>>> subdirectory. You can see my change at https://reviews.llvm.org/D64121.
>>> If you have any comments, please reply.
>>>
>>> If people are happy about this change, I can do the same thing for other
>>> directories including LLVM itself and Clang.
>>>
>>> On Mon, Jun 10, 2019 at 6:34 PM Rui Ueyama <ruiu at google.com> wrote:
>>>
>>>> On Mon, Jun 10, 2019 at 6:27 PM Michael Platings <
>>>> Michael.Platings at arm.com> wrote:
>>>>
>>>>> Hi Rui,
>>>>>
>>>>>
>>>>>
>>>>> As per the provisional plan [1] we’re still at step 1: improving git
>>>>> blame. The status of this is that there are some fairly mature patches in
>>>>> the Git project’s queue [2], and I’m hopeful that it will be accepted in
>>>>> something close to its current form.
>>>>>
>>>>>
>>>>>
>>>>> But if you can get started on steps 2 & 3 i.e. making forks available
>>>>> with the possible changes applied then that would be great. My hope is that
>>>>> once everyone can see what the options really look like then it will be
>>>>> easier to reach consensus.
>>>>>
>>>>
>>>> Sure, I'll try to do that. I'll probably start with finding identifiers
>>>> and typenames that will conflict after the naming scheme change and rename
>>>> them so that they won't conflict. The number of such symbols would
>>>> hopefully be small, and submitting such renaming changes wouldn't be
>>>> distracting. After that, I think I can create a mechanical change to rename
>>>> variables to see how it will look like.
>>>>
>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> -Michael
>>>>>
>>>>>
>>>>>
>>>>> [1]
>>>>> https://llvm.org/docs/Proposals/VariableNames.html#provisional-plan
>>>>>
>>>>> [2]
>>>>> https://public-inbox.org/git/20190515214503.77162-8-brho@google.com/T/
>>>>>
>>>>>
>>>>>
>>>>> *From:* Rui Ueyama <ruiu at google.com>
>>>>> *Sent:* 07 June 2019 08:42
>>>>> *To:* Chris Lattner <clattner at nondot.org>
>>>>> *Cc:* Michael Platings <Michael.Platings at arm.com>;
>>>>> llvm-dev at lists.llvm.org; nd <nd at arm.com>
>>>>> *Subject:* Re: [llvm-dev] RFC: changing variable naming rules in LLVM
>>>>> codebase
>>>>>
>>>>>
>>>>>
>>>>> This thread is not active for a while, but I'm still interested in
>>>>> seeing a change.
>>>>>
>>>>>
>>>>>
>>>>> Reading through this thread, looks like we can agree that we want to
>>>>> change the local variable naming scheme so that they start with a lowercase
>>>>> letter. Besides that, there were discussions about lower_case, camelCase,
>>>>> m_ prefix, and each argument seems as convincing as others. I'm not sure
>>>>> what is the decision making process in a situation like this.
>>>>>
>>>>>
>>>>>
>>>>> I'd personally vote for changing local variables to start with a
>>>>> lowercase letter and keep other naming conventions as-is to make it a
>>>>> minimum change.
>>>>>
>>>>>
>>>>>
>>>>> As I stated before, I'm happy to make a change to lld to see how a
>>>>> naming convention change will look like, but in order to do that I need to
>>>>> get at least a rough consensus to do that. What is a way to proceed?
>>>>>
>>>>>
>>>>>
>>>>> On Sat, May 25, 2019 at 3:00 PM Chris Lattner via llvm-dev <
>>>>> llvm-dev at lists.llvm.org> wrote:
>>>>>
>>>>> Hi Michael,
>>>>>
>>>>>
>>>>>
>>>>> I’m still very interested in seeing a change here.  If someone is
>>>>> interested in seeing a codebase using the proposed camelBack convention for
>>>>> variables names, the MLIR codebase is public
>>>>> <https://github.com/tensorflow/mlir> and uses it.
>>>>>
>>>>>
>>>>>
>>>>> If you’re curious to see what this looks like in practice, there are
>>>>> lots of examples in the codebase, here is an example .cpp file
>>>>> <https://github.com/tensorflow/mlir/blob/master/lib/Transforms/LoopUnrollAndJam.cpp>
>>>>> , here is another
>>>>> <https://github.com/tensorflow/mlir/blob/master/lib/Parser/Parser.cpp>,
>>>>> here is an example header
>>>>> <https://github.com/tensorflow/mlir/blob/master/include/mlir/IR/Location.h>
>>>>> .
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> We are still working our way through open sourcing logistics (not all
>>>>> the code is out yet), but we would still like to contribute at least parts
>>>>> of this to LLVM if the project is interested.  [[This is just an FYI, not
>>>>> itself a proposal yet - one will be coming when we’re ready.]]
>>>>>
>>>>>
>>>>>
>>>>> -Chris
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On May 21, 2019, at 3:01 AM, Michael Platings via llvm-dev <
>>>>> llvm-dev at lists.llvm.org> wrote:
>>>>>
>>>>>
>>>>>
>>>>> Hi folks,
>>>>>
>>>>> Git is on its way to learning how to ignore commits, allowing us to do
>>>>> variable renaming and other small refactorings without breaking git blame.
>>>>>
>>>>> It's like git-hyper-blame [1] but significantly more powerful as it
>>>>> uses fuzzy matching to match lines, including lines that may have been
>>>>> split or joined.
>>>>>
>>>>> A preview release of Git with this new feature is at:
>>>>> https://github.com/mplatings/git/releases/tag/ignore-rev
>>>>>
>>>>> Some of you have told me that you already have to spend time running
>>>>> git blame multiple times to look past uninteresting commits so I'd love for
>>>>> you to give this feature a try and see if it helps you. Your feedback will
>>>>> be very valuable.
>>>>>
>>>>> Thanks,
>>>>> -Michael
>>>>>
>>>>> [1]
>>>>> https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/git-hyper-blame.html
>>>>> _______________________________________________
>>>>> 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/20190710/45068960/attachment-0001.html>