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

[Rui Ueyama via llvm-dev]

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>