llvm dev - Sep 2019 - [RFC] changing variable naming rules

Hi all,

To get wider visibility, build a broader consensus and address concerns on
this topic, I'm again raising this as an RFC. This is a proposal to change
the rule for variable names from CamelCase to camelBack _really this time_.

Background:

This has been proposed several times on this mailing list in the past. Most
recent one was by Michael Platings in February this year [1], and there
seems to be a general consensus that the status quo is not ideal.

In the previous RFC thread, I nominated lld [2] as a starter project for
renaming and made a sweeping change to rename variables in a few commits.
This renaming went well -- even though it broke buildbots, I managed to
unbreak them in a timely manner, and more importantly, it has been reported
that several downstream repos have successfully migrated to the new naming
scheme using a tool that I wrote to create sweeping changes. That being
said, some claimed that the renaming attempt didn't get enough attention,
despite being discussed in a thread that has 100+ emails. So I'm raising
this topic as a new thread.

I propose we do the same thing to another relatively small subproject,
clang-tools-extras, to gain more experience, and then migrate the entire
LLVM codebase to the new style. It seems technically doable, and even
though it would cause a short-term pain, people seem to feel more
comfortable with the new naming scheme than the current one. I also believe
that the migration won't be that painful.

Objectives:

 - Migrating the entire LLVM repo including subprojects to the new naming
scheme without breaking them.
 - Many projects, especially LLVM and Clang have lots of out-of-tree
downstream repos. We need to provide a tool to rebase such repos to a
commit after a renaming.
 - The sweeping change shouldn't break `git blame`.

What I learned from the lld's naming scheme change:

 - There are many member variables in the LLVM/lld codebase that have the
same name as accessors ignoring case (i.e. many classes define foo() as an
accessor to a member variable Foo). Such variables would conflict with
functions after renaming, so we had to rename accessors by prepending `get`.

 - A single large sweeping change seemed to work better than small
incremental changes for downstream repos. Downstream repo maintainers
rebased their trees to a commit just prior to the sweeping change, apply my
tool to rename all variables in their trees, and then rebase the trees onto
the sweeping change. Because the tool creates the same diffs for existing
code, downstream maintainers basically only had to merge their diffs at the
last step.

 - Even though my tool worked satisfactory, it couldn't rewrite code that
are excluded by #if, #ifdef and the like, because the clang-based tool
doesn't really see the code excluded by the preprocessor. That caused
several buildbot breakages.

 - git 2.23 (released in August) added a new option `--ignore-revs` to `git
blame` so that the command can take a list of commits that need to be
ignored by blame. Developers can set a default ignore file (typically named
`.git-blame-ignore-revs`) using `git config` so that blame automatically
ignores commits listed in the file. As far as I tried, that command worked
pretty well to ignore the sweeping change I made to lld, so the `git blame`
issue seems a solved problem now.

Migration plan:

Given the above findings, I propose we migrate to the new coding style in
the following steps.

 1. Change the codebase to eliminate name duplication between accessors and
members. This can be done incrementally with as many commits as we want.
 2. Complete the tool and apply it to the entire LLVM tree. I'll publish it
at GitHub so that people can take a look and try it out.
 3. Setup buildbots so that they checkout my GitHub tree, build it and run
its tests, to make sure that a sweeping change won't break them. (I
don't
know how to configure buildbots, but I presume this step is doable.)
 4. Give a heads-up and submit a sweeping change to clang-tools-extras, and
make sure that that won't break anything.
 5. Give a heads-up and submit a sweeping change to the entire LLVM.

I'd like to submit a sweeping change after LLVM migrates to GitHub to
minimize confusion.

[1] http://lists.llvm.org/pipermail/llvm-dev/2019-February/130083.html
[2] https://github.com/llvm/llvm-project/tree/master/lld
[3]
https://github.com/llvm/llvm-project/commit/3837f4273fcc40cc519035479aefe78e5cbd3055
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190904/7c9b5ef5/attachment.html>

Developers can set a default ignore file (typically named
`.git-blame-ignore-revs`) using `git config` so that blame automatically ignores
commits listed in the file.

To make this as painless as possible for all future generations of contributors,
we should provide an in-tree script that will DTRT to set this up, and document
it in the Getting Started page.  My understanding is that cloning can't set
any config options like this automatically, so an easy one-time script is the
next best thing.
--paulr

From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of Rui
Ueyama via llvm-dev
Sent: Tuesday, September 03, 2019 11:15 PM
To: llvm-dev
Subject: [llvm-dev] [RFC] changing variable naming rules

Hi all,

To get wider visibility, build a broader consensus and address concerns on this
topic, I'm again raising this as an RFC. This is a proposal to change the
rule for variable names from CamelCase to camelBack _really this time_.

Background:

This has been proposed several times on this mailing list in the past. Most
recent one was by Michael Platings in February this year [1], and there seems to
be a general consensus that the status quo is not ideal.

In the previous RFC thread, I nominated lld [2] as a starter project for
renaming and made a sweeping change to rename variables in a few commits. This
renaming went well -- even though it broke buildbots, I managed to unbreak them
in a timely manner, and more importantly, it has been reported that several
downstream repos have successfully migrated to the new naming scheme using a
tool that I wrote to create sweeping changes. That being said, some claimed that
the renaming attempt didn't get enough attention, despite being discussed in
a thread that has 100+ emails. So I'm raising this topic as a new thread.

I propose we do the same thing to another relatively small subproject,
clang-tools-extras, to gain more experience, and then migrate the entire LLVM
codebase to the new style. It seems technically doable, and even though it would
cause a short-term pain, people seem to feel more comfortable with the new
naming scheme than the current one. I also believe that the migration won't
be that painful.

Objectives:

 - Migrating the entire LLVM repo including subprojects to the new naming scheme
without breaking them.
 - Many projects, especially LLVM and Clang have lots of out-of-tree downstream
repos. We need to provide a tool to rebase such repos to a commit after a
renaming.
 - The sweeping change shouldn't break `git blame`.

What I learned from the lld's naming scheme change:

 - There are many member variables in the LLVM/lld codebase that have the same
name as accessors ignoring case (i.e. many classes define foo() as an accessor
to a member variable Foo). Such variables would conflict with functions after
renaming, so we had to rename accessors by prepending `get`.

 - A single large sweeping change seemed to work better than small incremental
changes for downstream repos. Downstream repo maintainers rebased their trees to
a commit just prior to the sweeping change, apply my tool to rename all
variables in their trees, and then rebase the trees onto the sweeping change.
Because the tool creates the same diffs for existing code, downstream
maintainers basically only had to merge their diffs at the last step.
 - Even though my tool worked satisfactory, it couldn't rewrite code that
are excluded by #if, #ifdef and the like, because the clang-based tool
doesn't really see the code excluded by the preprocessor. That caused
several buildbot breakages.
 - git 2.23 (released in August) added a new option `--ignore-revs` to `git
blame` so that the command can take a list of commits that need to be ignored by
blame. Developers can set a default ignore file (typically named
`.git-blame-ignore-revs`) using `git config` so that blame automatically ignores
commits listed in the file. As far as I tried, that command worked pretty well
to ignore the sweeping change I made to lld, so the `git blame` issue seems a
solved problem now.

Migration plan:

Given the above findings, I propose we migrate to the new coding style in the
following steps.

 1. Change the codebase to eliminate name duplication between accessors and
members. This can be done incrementally with as many commits as we want.
 2. Complete the tool and apply it to the entire LLVM tree. I'll publish it
at GitHub so that people can take a look and try it out.
 3. Setup buildbots so that they checkout my GitHub tree, build it and run its
tests, to make sure that a sweeping change won't break them. (I don't
know how to configure buildbots, but I presume this step is doable.)
 4. Give a heads-up and submit a sweeping change to clang-tools-extras, and make
sure that that won't break anything.
 5. Give a heads-up and submit a sweeping change to the entire LLVM.

I'd like to submit a sweeping change after LLVM migrates to GitHub to
minimize confusion.

[1] http://lists.llvm.org/pipermail/llvm-dev/2019-February/130083.html
[2] https://github.com/llvm/llvm-project/tree/master/lld
[3]
https://github.com/llvm/llvm-project/commit/3837f4273fcc40cc519035479aefe78e5cbd3055
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190904/47812ff6/attachment.html>

Good point. As far as I understand, after git migration we will make merge
commits illegal, and we'll have to have each developer run a few `git
config` commands after checking out the LLVM repository. The `git config`
for blame should be described that instruction. (Or it can be a script
containing just a few lines of `git config`, I don't have a strong
preference.)

On Wed, Sep 4, 2019 at 11:08 PM <paul.robinson at sony.com> wrote:
> Developers can set a default ignore file (typically named
> `.git-blame-ignore-revs`) using `git config` so that blame automatically
> ignores commits listed in the file.
>
>
>
> To make this as painless as possible for all future generations of
> contributors, we should provide an in-tree script that will DTRT to set
> this up, and document it in the Getting Started page.  My understanding is
> that cloning can't set any config options like this automatically, so
an
> easy one-time script is the next best thing.
>
> --paulr
>
>
>
> *From:* llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] *On Behalf Of
*Rui
> Ueyama via llvm-dev
> *Sent:* Tuesday, September 03, 2019 11:15 PM
> *To:* llvm-dev
> *Subject:* [llvm-dev] [RFC] changing variable naming rules
>
>
>
> Hi all,
>
> To get wider visibility, build a broader consensus and address concerns on
> this topic, I'm again raising this as an RFC. This is a proposal to
change
> the rule for variable names from CamelCase to camelBack _really this time_.
>
> Background:
>
> This has been proposed several times on this mailing list in the past.
> Most recent one was by Michael Platings in February this year [1], and
> there seems to be a general consensus that the status quo is not ideal.
>
> In the previous RFC thread, I nominated lld [2] as a starter project for
> renaming and made a sweeping change to rename variables in a few commits.
> This renaming went well -- even though it broke buildbots, I managed to
> unbreak them in a timely manner, and more importantly, it has been reported
> that several downstream repos have successfully migrated to the new naming
> scheme using a tool that I wrote to create sweeping changes. That being
> said, some claimed that the renaming attempt didn't get enough
attention,
> despite being discussed in a thread that has 100+ emails. So I'm
raising
> this topic as a new thread.
>
> I propose we do the same thing to another relatively small subproject,
> clang-tools-extras, to gain more experience, and then migrate the entire
> LLVM codebase to the new style. It seems technically doable, and even
> though it would cause a short-term pain, people seem to feel more
> comfortable with the new naming scheme than the current one. I also believe
> that the migration won't be that painful.
>
> Objectives:
>
>  - Migrating the entire LLVM repo including subprojects to the new naming
> scheme without breaking them.
>  - Many projects, especially LLVM and Clang have lots of out-of-tree
> downstream repos. We need to provide a tool to rebase such repos to a
> commit after a renaming.
>  - The sweeping change shouldn't break `git blame`.
>
> What I learned from the lld's naming scheme change:
>
>  - There are many member variables in the LLVM/lld codebase that have the
> same name as accessors ignoring case (i.e. many classes define foo() as an
> accessor to a member variable Foo). Such variables would conflict with
> functions after renaming, so we had to rename accessors by prepending
`get`.
>
>
>  - A single large sweeping change seemed to work better than small
> incremental changes for downstream repos. Downstream repo maintainers
> rebased their trees to a commit just prior to the sweeping change, apply my
> tool to rename all variables in their trees, and then rebase the trees onto
> the sweeping change. Because the tool creates the same diffs for existing
> code, downstream maintainers basically only had to merge their diffs at the
> last step.
>
>  - Even though my tool worked satisfactory, it couldn't rewrite code
that
> are excluded by #if, #ifdef and the like, because the clang-based tool
> doesn't really see the code excluded by the preprocessor. That caused
> several buildbot breakages.
>
>  - git 2.23 (released in August) added a new option `--ignore-revs` to
> `git blame` so that the command can take a list of commits that need to be
> ignored by blame. Developers can set a default ignore file (typically named
> `.git-blame-ignore-revs`) using `git config` so that blame automatically
> ignores commits listed in the file. As far as I tried, that command worked
> pretty well to ignore the sweeping change I made to lld, so the `git blame`
> issue seems a solved problem now.
>
> Migration plan:
>
> Given the above findings, I propose we migrate to the new coding style in
> the following steps.
>
>  1. Change the codebase to eliminate name duplication between accessors
> and members. This can be done incrementally with as many commits as we
want.
>  2. Complete the tool and apply it to the entire LLVM tree. I'll
publish
> it at GitHub so that people can take a look and try it out.
>  3. Setup buildbots so that they checkout my GitHub tree, build it and run
> its tests, to make sure that a sweeping change won't break them. (I
don't
> know how to configure buildbots, but I presume this step is doable.)
>  4. Give a heads-up and submit a sweeping change to clang-tools-extras,
> and make sure that that won't break anything.
>  5. Give a heads-up and submit a sweeping change to the entire LLVM.
>
> I'd like to submit a sweeping change after LLVM migrates to GitHub to
> minimize confusion.
>
> [1] http://lists.llvm.org/pipermail/llvm-dev/2019-February/130083.html
> [2] https://github.com/llvm/llvm-project/tree/master/lld
> [3]
>
https://github.com/llvm/llvm-project/commit/3837f4273fcc40cc519035479aefe78e5cbd3055
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190905/3c3eda71/attachment.html>

Hi Rui,

FWIW, I’m very +1 on this.  I think you did a great job with the LLD migration,
and I’m really happy you used that as a relatively small scale proof point to
develop the methodology and see what issues shake out.

Thank you for driving this!

-Chris

> On Sep 3, 2019, at 8:14 PM, Rui Ueyama via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Hi all,
> 
> To get wider visibility, build a broader consensus and address concerns on
this topic, I'm again raising this as an RFC. This is a proposal to change
the rule for variable names from CamelCase to camelBack _really this time_.
> 
> Background:
> 
> This has been proposed several times on this mailing list in the past. Most
recent one was by Michael Platings in February this year [1], and there seems to
be a general consensus that the status quo is not ideal.
> 
> In the previous RFC thread, I nominated lld [2] as a starter project for
renaming and made a sweeping change to rename variables in a few commits. This
renaming went well -- even though it broke buildbots, I managed to unbreak them
in a timely manner, and more importantly, it has been reported that several
downstream repos have successfully migrated to the new naming scheme using a
tool that I wrote to create sweeping changes. That being said, some claimed that
the renaming attempt didn't get enough attention, despite being discussed in
a thread that has 100+ emails. So I'm raising this topic as a new thread.
> 
> I propose we do the same thing to another relatively small subproject,
clang-tools-extras, to gain more experience, and then migrate the entire LLVM
codebase to the new style. It seems technically doable, and even though it would
cause a short-term pain, people seem to feel more comfortable with the new
naming scheme than the current one. I also believe that the migration won't
be that painful.
> 
> Objectives:
> 
>  - Migrating the entire LLVM repo including subprojects to the new naming
scheme without breaking them.
>  - Many projects, especially LLVM and Clang have lots of out-of-tree
downstream repos. We need to provide a tool to rebase such repos to a commit
after a renaming.
>  - The sweeping change shouldn't break `git blame`.
> 
> What I learned from the lld's naming scheme change:
> 
>  - There are many member variables in the LLVM/lld codebase that have the
same name as accessors ignoring case (i.e. many classes define foo() as an
accessor to a member variable Foo). Such variables would conflict with functions
after renaming, so we had to rename accessors by prepending `get`.
> 
>  - A single large sweeping change seemed to work better than small
incremental changes for downstream repos. Downstream repo maintainers rebased
their trees to a commit just prior to the sweeping change, apply my tool to
rename all variables in their trees, and then rebase the trees onto the sweeping
change. Because the tool creates the same diffs for existing code, downstream
maintainers basically only had to merge their diffs at the last step.
> 
>  - Even though my tool worked satisfactory, it couldn't rewrite code
that are excluded by #if, #ifdef and the like, because the clang-based tool
doesn't really see the code excluded by the preprocessor. That caused
several buildbot breakages.
> 
>  - git 2.23 (released in August) added a new option `--ignore-revs` to `git
blame` so that the command can take a list of commits that need to be ignored by
blame. Developers can set a default ignore file (typically named
`.git-blame-ignore-revs`) using `git config` so that blame automatically ignores
commits listed in the file. As far as I tried, that command worked pretty well
to ignore the sweeping change I made to lld, so the `git blame` issue seems a
solved problem now.
> 
> Migration plan:
> 
> Given the above findings, I propose we migrate to the new coding style in
the following steps.
> 
>  1. Change the codebase to eliminate name duplication between accessors and
members. This can be done incrementally with as many commits as we want.
>  2. Complete the tool and apply it to the entire LLVM tree. I'll
publish it at GitHub so that people can take a look and try it out.
>  3. Setup buildbots so that they checkout my GitHub tree, build it and run
its tests, to make sure that a sweeping change won't break them. (I
don't know how to configure buildbots, but I presume this step is doable.)
>  4. Give a heads-up and submit a sweeping change to clang-tools-extras, and
make sure that that won't break anything.
>  5. Give a heads-up and submit a sweeping change to the entire LLVM.
> 
> I'd like to submit a sweeping change after LLVM migrates to GitHub to
minimize confusion.
> 
> [1] http://lists.llvm.org/pipermail/llvm-dev/2019-February/130083.html
<http://lists.llvm.org/pipermail/llvm-dev/2019-February/130083.html>
> [2] https://github.com/llvm/llvm-project/tree/master/lld
<https://github.com/llvm/llvm-project/tree/master/lld>
> [3]
https://github.com/llvm/llvm-project/commit/3837f4273fcc40cc519035479aefe78e5cbd3055
<https://github.com/llvm/llvm-project/commit/3837f4273fcc40cc519035479aefe78e5cbd3055>
> 
> _______________________________________________
> 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/20190905/f4b7da89/attachment.html>

Personally I think the benefit-cost ratio is quite low here, and changing the
rules would bring little value to me.
Maybe I just don’t see that much benefits of changing the naming rules, having
got used to the current naming scheme after a few years with LLVM.


I'm a little bit curious to hear more about the experiences from the changes
in LLD.
I’m thinking about things like:

- ongoing reviews in Phabricator that suddenly needs major rebase

- merging of bugfixes to older release branches (I doubt that the script
supports doing the reverse rewrite as well)


When it comes to git blame and the ignore sweeping change file:

- Will that work when I do blame inside github (I assume it would need to be
configured to find the ignore file in some way)?
  Anyway, ignoring sweeping commits should probably not be the default in
github, since sometimes you want to find the sweeping commit.

- There are several more tools that also has blame functionality (e.g. Gerrit
Code Review that could be used by downstream forks).
  Just because there is a --ignore-revs option in newer git versions, it
wouldn't be used by all my tools. So I'm not sure it can be considered
as a solution.
 

Blame is also just one potential problem. I'd still end up with lots of
(IMHO) "irrelevant" changes when doing "git diff". For
example when trying to figure out what happened between LLVM 8.0.0 and LLVM
9.0.0 in some part of the code, by actually looking at code diffs and not just
the commit log.


Kind Regards,
Björn


From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of Chris
Lattner via llvm-dev
Sent: den 5 september 2019 18:09
To: Rui Ueyama <ruiu at google.com>
Cc: llvm-dev <llvm-dev at lists.llvm.org>
Subject: Re: [llvm-dev] [RFC] changing variable naming rules

Hi Rui,

FWIW, I’m very +1 on this.  I think you did a great job with the LLD migration,
and I’m really happy you used that as a relatively small scale proof point to
develop the methodology and see what issues shake out.

Thank you for driving this!

-Chris



On Sep 3, 2019, at 8:14 PM, Rui Ueyama via llvm-dev <mailto:llvm-dev at
lists.llvm.org> wrote:

Hi all,

To get wider visibility, build a broader consensus and address concerns on this
topic, I'm again raising this as an RFC. This is a proposal to change the
rule for variable names from CamelCase to camelBack _really this time_.

Background:

This has been proposed several times on this mailing list in the past. Most
recent one was by Michael Platings in February this year [1], and there seems to
be a general consensus that the status quo is not ideal.

In the previous RFC thread, I nominated lld [2] as a starter project for
renaming and made a sweeping change to rename variables in a few commits. This
renaming went well -- even though it broke buildbots, I managed to unbreak them
in a timely manner, and more importantly, it has been reported that several
downstream repos have successfully migrated to the new naming scheme using a
tool that I wrote to create sweeping changes. That being said, some claimed that
the renaming attempt didn't get enough attention, despite being discussed in
a thread that has 100+ emails. So I'm raising this topic as a new thread.

I propose we do the same thing to another relatively small subproject,
clang-tools-extras, to gain more experience, and then migrate the entire LLVM
codebase to the new style. It seems technically doable, and even though it would
cause a short-term pain, people seem to feel more comfortable with the new
naming scheme than the current one. I also believe that the migration won't
be that painful.

Objectives:

 - Migrating the entire LLVM repo including subprojects to the new naming scheme
without breaking them.
 - Many projects, especially LLVM and Clang have lots of out-of-tree downstream
repos. We need to provide a tool to rebase such repos to a commit after a
renaming.
 - The sweeping change shouldn't break `git blame`.

What I learned from the lld's naming scheme change:

 - There are many member variables in the LLVM/lld codebase that have the same
name as accessors ignoring case (i.e. many classes define foo() as an accessor
to a member variable Foo). Such variables would conflict with functions after
renaming, so we had to rename accessors by prepending `get`.

 - A single large sweeping change seemed to work better than small incremental
changes for downstream repos. Downstream repo maintainers rebased their trees to
a commit just prior to the sweeping change, apply my tool to rename all
variables in their trees, and then rebase the trees onto the sweeping change.
Because the tool creates the same diffs for existing code, downstream
maintainers basically only had to merge their diffs at the last step.
 - Even though my tool worked satisfactory, it couldn't rewrite code that
are excluded by #if, #ifdef and the like, because the clang-based tool
doesn't really see the code excluded by the preprocessor. That caused
several buildbot breakages.
 - git 2.23 (released in August) added a new option `--ignore-revs` to `git
blame` so that the command can take a list of commits that need to be ignored by
blame. Developers can set a default ignore file (typically named
`.git-blame-ignore-revs`) using `git config` so that blame automatically ignores
commits listed in the file. As far as I tried, that command worked pretty well
to ignore the sweeping change I made to lld, so the `git blame` issue seems a
solved problem now.

Migration plan:

Given the above findings, I propose we migrate to the new coding style in the
following steps.

 1. Change the codebase to eliminate name duplication between accessors and
members. This can be done incrementally with as many commits as we want.
 2. Complete the tool and apply it to the entire LLVM tree. I'll publish it
at GitHub so that people can take a look and try it out.
 3. Setup buildbots so that they checkout my GitHub tree, build it and run its
tests, to make sure that a sweeping change won't break them. (I don't
know how to configure buildbots, but I presume this step is doable.)
 4. Give a heads-up and submit a sweeping change to clang-tools-extras, and make
sure that that won't break anything.
 5. Give a heads-up and submit a sweeping change to the entire LLVM.

I'd like to submit a sweeping change after LLVM migrates to GitHub to
minimize confusion.

[1] http://lists.llvm.org/pipermail/llvm-dev/2019-February/130083.html
[2]
https://protect2.fireeye.com/url?k=ea62499f-b6f04480-ea620904-0cc47ad93db4-ecd933b4f617d714&q=1&u=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project%2Ftree%2Fmaster%2Flld
[3]
https://protect2.fireeye.com/url?k=3251f903-6ec3f41c-3251b998-0cc47ad93db4-9f9d4ccc78551a27&q=1&u=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project%2Fcommit%2F3837f4273fcc40cc519035479aefe78e5cbd3055
_______________________________________________
LLVM Developers mailing list
mailto:llvm-dev at lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

I do not support this.  I feel the benefit is low, and the churn cost is 
high.

I'm not strongly opposed or anything, I just don't believe this is 
worthwhile.

Philip

On 9/3/2019 8:14 PM, Rui Ueyama via llvm-dev wrote:
> Hi all,
>
> To get wider visibility, build a broader consensus and address 
> concerns on this topic, I'm again raising this as an RFC. This is a 
> proposal to change the rule for variable names from CamelCase to 
> camelBack _really this time_.
>
> Background:
>
> This has been proposed several times on this mailing list in the past. 
> Most recent one was by Michael Platings in February this year [1], and 
> there seems to be a general consensus that the status quo is not ideal.
>
> In the previous RFC thread, I nominated lld [2] as a starter project 
> for renaming and made a sweeping change to rename variables in a few 
> commits. This renaming went well -- even though it broke buildbots, I 
> managed to unbreak them in a timely manner, and more importantly, it 
> has been reported that several downstream repos have successfully 
> migrated to the new naming scheme using a tool that I wrote to create 
> sweeping changes. That being said, some claimed that the renaming 
> attempt didn't get enough attention, despite being discussed in a 
> thread that has 100+ emails. So I'm raising this topic as a new thread.
>
> I propose we do the same thing to another relatively small subproject, 
> clang-tools-extras, to gain more experience, and then migrate the 
> entire LLVM codebase to the new style. It seems technically doable, 
> and even though it would cause a short-term pain, people seem to feel 
> more comfortable with the new naming scheme than the current one. I 
> also believe that the migration won't be that painful.
>
> Objectives:
>
>  - Migrating the entire LLVM repo including subprojects to the new 
> naming scheme without breaking them.
>  - Many projects, especially LLVM and Clang have lots of out-of-tree 
> downstream repos. We need to provide a tool to rebase such repos to a 
> commit after a renaming.
>  - The sweeping change shouldn't break `git blame`.
>
> What I learned from the lld's naming scheme change:
>
>  - There are many member variables in the LLVM/lld codebase that have 
> the same name as accessors ignoring case (i.e. many classes define 
> foo() as an accessor to a member variable Foo). Such variables would 
> conflict with functions after renaming, so we had to rename accessors 
> by prepending `get`.
>
>  - A single large sweeping change seemed to work better than small 
> incremental changes for downstream repos. Downstream repo maintainers 
> rebased their trees to a commit just prior to the sweeping change, 
> apply my tool to rename all variables in their trees, and then rebase 
> the trees onto the sweeping change. Because the tool creates the same 
> diffs for existing code, downstream maintainers basically only had to 
> merge their diffs at the last step.
>
>  - Even though my tool worked satisfactory, it couldn't rewrite code 
> that are excluded by #if, #ifdef and the like, because the clang-based 
> tool doesn't really see the code excluded by the preprocessor. That 
> caused several buildbot breakages.
>
>  - git 2.23 (released in August) added a new option `--ignore-revs` to 
> `git blame` so that the command can take a list of commits that need 
> to be ignored by blame. Developers can set a default ignore file 
> (typically named `.git-blame-ignore-revs`) using `git config` so that 
> blame automatically ignores commits listed in the file. As far as I 
> tried, that command worked pretty well to ignore the sweeping change I 
> made to lld, so the `git blame` issue seems a solved problem now.
>
> Migration plan:
>
> Given the above findings, I propose we migrate to the new coding style 
> in the following steps.
>
>  1. Change the codebase to eliminate name duplication between 
> accessors and members. This can be done incrementally with as many 
> commits as we want.
>  2. Complete the tool and apply it to the entire LLVM tree. I'll 
> publish it at GitHub so that people can take a look and try it out.
>  3. Setup buildbots so that they checkout my GitHub tree, build it and 
> run its tests, to make sure that a sweeping change won't break 
> them. (I don't know how to configure buildbots, but I presume this 
> step is doable.)
>  4. Give a heads-up and submit a sweeping change to 
> clang-tools-extras, and make sure that that won't break anything.
>  5. Give a heads-up and submit a sweeping change to the entire LLVM.
>
> I'd like to submit a sweeping change after LLVM migrates to GitHub to 
> minimize confusion.
>
> [1] http://lists.llvm.org/pipermail/llvm-dev/2019-February/130083.html
> [2] https://github.com/llvm/llvm-project/tree/master/lld
> [3] 
>
https://github.com/llvm/llvm-project/commit/3837f4273fcc40cc519035479aefe78e5cbd3055
>
>
> _______________________________________________
> 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/20190907/af01f96c/attachment.html>

[just so i don't end up with "why haven't you spoken up"]

On Sun, Sep 8, 2019 at 1:32 AM Philip Reames via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>
> I do not support this.  I feel the benefit is low, and the churn cost is
high.
>
> I'm not strongly opposed or anything, I just don't believe this is
worthwhile.
Same thoughts.
> Philip
Roman
> On 9/3/2019 8:14 PM, Rui Ueyama via llvm-dev wrote:
>
> Hi all,
>
> To get wider visibility, build a broader consensus and address concerns on
this topic, I'm again raising this as an RFC. This is a proposal to change
the rule for variable names from CamelCase to camelBack _really this time_.
>
> Background:
>
> This has been proposed several times on this mailing list in the past. Most
recent one was by Michael Platings in February this year [1], and there seems to
be a general consensus that the status quo is not ideal.
>
> In the previous RFC thread, I nominated lld [2] as a starter project for
renaming and made a sweeping change to rename variables in a few commits. This
renaming went well -- even though it broke buildbots, I managed to unbreak them
in a timely manner, and more importantly, it has been reported that several
downstream repos have successfully migrated to the new naming scheme using a
tool that I wrote to create sweeping changes. That being said, some claimed that
the renaming attempt didn't get enough attention, despite being discussed in
a thread that has 100+ emails. So I'm raising this topic as a new thread.
>
> I propose we do the same thing to another relatively small subproject,
clang-tools-extras, to gain more experience, and then migrate the entire LLVM
codebase to the new style. It seems technically doable, and even though it would
cause a short-term pain, people seem to feel more comfortable with the new
naming scheme than the current one. I also believe that the migration won't
be that painful.
>
> Objectives:
>
>  - Migrating the entire LLVM repo including subprojects to the new naming
scheme without breaking them.
>  - Many projects, especially LLVM and Clang have lots of out-of-tree
downstream repos. We need to provide a tool to rebase such repos to a commit
after a renaming.
>  - The sweeping change shouldn't break `git blame`.
>
> What I learned from the lld's naming scheme change:
>
>  - There are many member variables in the LLVM/lld codebase that have the
same name as accessors ignoring case (i.e. many classes define foo() as an
accessor to a member variable Foo). Such variables would conflict with functions
after renaming, so we had to rename accessors by prepending `get`.
>
>  - A single large sweeping change seemed to work better than small
incremental changes for downstream repos. Downstream repo maintainers rebased
their trees to a commit just prior to the sweeping change, apply my tool to
rename all variables in their trees, and then rebase the trees onto the sweeping
change. Because the tool creates the same diffs for existing code, downstream
maintainers basically only had to merge their diffs at the last step.
>
>  - Even though my tool worked satisfactory, it couldn't rewrite code
that are excluded by #if, #ifdef and the like, because the clang-based tool
doesn't really see the code excluded by the preprocessor. That caused
several buildbot breakages.
>
>  - git 2.23 (released in August) added a new option `--ignore-revs` to `git
blame` so that the command can take a list of commits that need to be ignored by
blame. Developers can set a default ignore file (typically named
`.git-blame-ignore-revs`) using `git config` so that blame automatically ignores
commits listed in the file. As far as I tried, that command worked pretty well
to ignore the sweeping change I made to lld, so the `git blame` issue seems a
solved problem now.
>
> Migration plan:
>
> Given the above findings, I propose we migrate to the new coding style in
the following steps.
>
>  1. Change the codebase to eliminate name duplication between accessors and
members. This can be done incrementally with as many commits as we want.
>  2. Complete the tool and apply it to the entire LLVM tree. I'll
publish it at GitHub so that people can take a look and try it out.
>  3. Setup buildbots so that they checkout my GitHub tree, build it and run
its tests, to make sure that a sweeping change won't break them. (I
don't know how to configure buildbots, but I presume this step is doable.)
>  4. Give a heads-up and submit a sweeping change to clang-tools-extras, and
make sure that that won't break anything.
>  5. Give a heads-up and submit a sweeping change to the entire LLVM.
>
> I'd like to submit a sweeping change after LLVM migrates to GitHub to
minimize confusion.
>
> [1] http://lists.llvm.org/pipermail/llvm-dev/2019-February/130083.html
> [2] https://github.com/llvm/llvm-project/tree/master/lld
> [3]
https://github.com/llvm/llvm-project/commit/3837f4273fcc40cc519035479aefe78e5cbd3055
>
>
> _______________________________________________
> 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

What cost do you see here?  Rui has done a significant amount of work to make
this effectively zero cost.

The improvements are meaningful, and (as was discussed on the other threads)
pretty much every large scale change in the LLVM world has been shot down with
objections like “it is too much churn”.

This is a huge problem, because it leads to stagnation in the codebase and does
not allow modernization.  LLVM has always had the development philosophy of
"trying to be the best”, even if it comes at a cost.  The unwillingness to
maintain a stable C++ API is one very significant aspect of this.

I don’t see how this case is any different.

-Chris

> On Sep 7, 2019, at 3:32 PM, Philip Reames via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> I do not support this.  I feel the benefit is low, and the churn cost is
high.
> 
> I'm not strongly opposed or anything, I just don't believe this is
worthwhile.
> 
> Philip
> 
> On 9/3/2019 8:14 PM, Rui Ueyama via llvm-dev wrote:
>> Hi all,
>> 
>> To get wider visibility, build a broader consensus and address concerns
on this topic, I'm again raising this as an RFC. This is a proposal to
change the rule for variable names from CamelCase to camelBack _really this
time_.
>> 
>> Background:
>> 
>> This has been proposed several times on this mailing list in the past.
Most recent one was by Michael Platings in February this year [1], and there
seems to be a general consensus that the status quo is not ideal.
>> 
>> In the previous RFC thread, I nominated lld [2] as a starter project
for renaming and made a sweeping change to rename variables in a few commits.
This renaming went well -- even though it broke buildbots, I managed to unbreak
them in a timely manner, and more importantly, it has been reported that several
downstream repos have successfully migrated to the new naming scheme using a
tool that I wrote to create sweeping changes. That being said, some claimed that
the renaming attempt didn't get enough attention, despite being discussed in
a thread that has 100+ emails. So I'm raising this topic as a new thread.
>> 
>> I propose we do the same thing to another relatively small subproject,
clang-tools-extras, to gain more experience, and then migrate the entire LLVM
codebase to the new style. It seems technically doable, and even though it would
cause a short-term pain, people seem to feel more comfortable with the new
naming scheme than the current one. I also believe that the migration won't
be that painful.
>> 
>> Objectives:
>> 
>>  - Migrating the entire LLVM repo including subprojects to the new
naming scheme without breaking them.
>>  - Many projects, especially LLVM and Clang have lots of out-of-tree
downstream repos. We need to provide a tool to rebase such repos to a commit
after a renaming.
>>  - The sweeping change shouldn't break `git blame`.
>> 
>> What I learned from the lld's naming scheme change:
>> 
>>  - There are many member variables in the LLVM/lld codebase that have
the same name as accessors ignoring case (i.e. many classes define foo() as an
accessor to a member variable Foo). Such variables would conflict with functions
after renaming, so we had to rename accessors by prepending `get`.
>> 
>>  - A single large sweeping change seemed to work better than small
incremental changes for downstream repos. Downstream repo maintainers rebased
their trees to a commit just prior to the sweeping change, apply my tool to
rename all variables in their trees, and then rebase the trees onto the sweeping
change. Because the tool creates the same diffs for existing code, downstream
maintainers basically only had to merge their diffs at the last step.
>> 
>>  - Even though my tool worked satisfactory, it couldn't rewrite
code that are excluded by #if, #ifdef and the like, because the clang-based tool
doesn't really see the code excluded by the preprocessor. That caused
several buildbot breakages.
>> 
>>  - git 2.23 (released in August) added a new option `--ignore-revs` to
`git blame` so that the command can take a list of commits that need to be
ignored by blame. Developers can set a default ignore file (typically named
`.git-blame-ignore-revs`) using `git config` so that blame automatically ignores
commits listed in the file. As far as I tried, that command worked pretty well
to ignore the sweeping change I made to lld, so the `git blame` issue seems a
solved problem now.
>> 
>> Migration plan:
>> 
>> Given the above findings, I propose we migrate to the new coding style
in the following steps.
>> 
>>  1. Change the codebase to eliminate name duplication between accessors
and members. This can be done incrementally with as many commits as we want.
>>  2. Complete the tool and apply it to the entire LLVM tree. I'll
publish it at GitHub so that people can take a look and try it out.
>>  3. Setup buildbots so that they checkout my GitHub tree, build it and
run its tests, to make sure that a sweeping change won't break them. (I
don't know how to configure buildbots, but I presume this step is doable.)
>>  4. Give a heads-up and submit a sweeping change to clang-tools-extras,
and make sure that that won't break anything.
>>  5. Give a heads-up and submit a sweeping change to the entire LLVM.
>> 
>> I'd like to submit a sweeping change after LLVM migrates to GitHub
to minimize confusion.
>> 
>> [1] http://lists.llvm.org/pipermail/llvm-dev/2019-February/130083.html
<http://lists.llvm.org/pipermail/llvm-dev/2019-February/130083.html>
>> [2] https://github.com/llvm/llvm-project/tree/master/lld
<https://github.com/llvm/llvm-project/tree/master/lld>
>> [3]
https://github.com/llvm/llvm-project/commit/3837f4273fcc40cc519035479aefe78e5cbd3055
<https://github.com/llvm/llvm-project/commit/3837f4273fcc40cc519035479aefe78e5cbd3055>
>> 
>> 
>> 
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<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/20190907/1acc5d44/attachment.html>