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

Wed Sep 4 19:28:52 PDT 2019

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>