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

Chandler Carruth via llvm-dev llvm-dev at lists.llvm.org
Sat Sep 7 16:13:01 PDT 2019


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.


> > 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190907/8e624202/attachment.html>


More information about the llvm-dev mailing list