[llvm-dev] RFC: changing variable naming rules in LLVM codebase
James Henderson via llvm-dev
llvm-dev at lists.llvm.org
Tue Jul 9 03:16:40 PDT 2019
Hi Rui,
Is your tool in a good state to be run on people's downstream repositories?
It would be nice to be able to update variable names there too at the same
time.
James
On Tue, 9 Jul 2019 at 08:24, 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190709/74febf3d/attachment.html>
More information about the llvm-dev
mailing list