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

Sat Sep 7 15:32:16 PDT 2019

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>