llvm dev - Jul 2019 - RFC: changing variable naming rules in LLVM codebase

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 at
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>

Alex,

Thank you for pointing that out. I haven't thought that before. Maybe it is
not very easy to update a comment like that using Clang AST, but I guess
that a simple Perl-style regex (such as `/*[A-Z])(.*)=*/.\1`) would work
fine as the pattern is unique, and I don't think that there are too many
false positives. Let me try.

On Wed, Jul 10, 2019 at 8:12 PM Alex Brachet-Mialot <
alexbrachetmialot at gmail.com> wrote:
> 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 at 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/4a414047/attachment.html>

Also, now that I think about it, I believe doxygen will fail to build if
the @parameter comments aren't changed to match the new names, my guess is
it case sensitive. So perhaps we will need to find a way to manually change
these names for doxygen comments?

On Wed, Jul 10, 2019 at 7:12 AM Alex Brachet-Mialot <
alexbrachetmialot at gmail.com> wrote:
> 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 at 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/a4ad85f7/attachment.html>

Good point, too. I believe I can find lines starting with `@parameter` and
apply the same name conversion rules to identifiers after `@parameter`.
Since lld doesn't use doxygen comments, it is fine for now, but before
moving forward, I'll address that. Thank you for pointing that out.

On Wed, Jul 10, 2019 at 8:20 PM Alex Brachet-Mialot <
alexbrachetmialot at gmail.com> wrote:
> Also, now that I think about it, I believe doxygen will fail to build if
> the @parameter comments aren't changed to match the new names, my guess
is
> it case sensitive. So perhaps we will need to find a way to manually change
> these names for doxygen comments?
>
> On Wed, Jul 10, 2019 at 7:12 AM Alex Brachet-Mialot <
> alexbrachetmialot at gmail.com> wrote:
>
>> 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 at 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/81fb4ab6/attachment.html>