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

Aaron Ballman via llvm-dev llvm-dev at lists.llvm.org
Sat Sep 7 16:46:23 PDT 2019


On Sat, Sep 7, 2019, 7:13 PM Chandler Carruth via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

>
>
> On Sat, Sep 7, 2019, 15:39 Roman Lebedev via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> [just so i don't end up with "why haven't you spoken up"]
>>
> Same.
>
>
>> 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.
>>
> Same.
>

Same.


>
>> > 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
>> _______________________________________________
>> 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/20190907/738157a8/attachment.html>


More information about the llvm-dev mailing list