[llvm-dev] RFC: changing variable naming rules in LLVM codebase & git-blame

Chris Lattner via llvm-dev llvm-dev at lists.llvm.org
Sun Jul 28 16:08:38 PDT 2019


On Jul 23, 2019, at 9:17 AM, JF Bastien via llvm-dev <llvm-dev at lists.llvm.org> wrote:
>> On Jul 23, 2019, at 8:30 AM, James Y Knight via llvm-dev <llvm-dev at lists.llvm.org> wrote:
>> 
>> As a very frequent explorer of history, I really don't think this is
>> as big an issue as it may seem. Even absent refactorings, you often
>> run into the "wrong" commit when looking at blame (e.g., someone just
>> added a comma rather than actually changing the code you care about),
>> and have to look past that, to another previous commit.
>> 
>> Any interactive blame tool ought to have an easy way to do this. For
>> example, in emacs's annotation mode (which is what I use), you just
>> press 'a' with the cursor on the line in question to re-annotate at
>> the commit previous to that.
> 
> You might very well be right, but your answer sounds like you *think* it’ll work out. For a huge change like this I’d like to *know* for a fact that you’re right. Changing variable naming will touch every single line except comments, so it’s quite different from what we usually see today.
> 
> Artem mentions a new git feature that’ll do this automatically. Again: sounds great, do we *know* that it’ll work?

Hi JF,

As discussed on other posts, there is good reason to believe that the forthcoming git feature will cover this case.  We aren’t the only project that has been held up by similar concerns about sweeping changes adversely affecting history spelunking.  Similarly, as James Knight points out, this isn’t really that big of a problem in practice even without the feature.

Sub projects in LLVM have gone through similar phases, including LLDB which switched from 4 space indentation to 2 (as well as 80 columns iirc), which affected literally everything.  This was unpopular, but was a good thing to do, because commonality and consistency across the project is important.  I’m not aware of a show-stopper problem even this massive of a change caused.

I think that Rui rolled this out in an incredibly great way with LLD, incorporating a lot of community feedback and discussion, and (as you say) this thread has accumulated many posts and a lot of discussion, so I don’t see the concern about lack of communication.

The reason that I personally pivoted to believe that a “do everything all at once in one commit generated by Rui’s script” approach was the right thing was seeing how lld contributors were able to cleanly move forward even when they have significant out of tree changes - they merged to the N-1 commit, then ran Rui's script on *their* trees, and were caught up with commit N.  This seems to have provided a very smooth transition path, where the conversion was merely a bump in the road, not a major mountain to climb over.

In any case, I hope that we as a community don’t end up in a place where we are afraid of doing sweeping changes that make the codebase better.  It is important to continually reinvest in the health of the codebase, and it isn’t bad to incentivize people to merge their changes upstream.  Being overwhelmingly afraid of ‘breaking history’ is a path that leads stagnation and ultimate replacement by newer technologies.  There are many other projects that have suffered this fate, usually for a combination of reasons including things like this.

-Chris



More information about the llvm-dev mailing list