[llvm-dev] amount of camelCase refactoring causing some downstream overhead

[Robinson, Paul via llvm-dev]

On behalf of my colleagues who do the actual merge work
(all of which I get to review, so I do see it all too),
let me say I feel the pain of the downstream projects.

I wouldn't change upstream policy, although in this particular
case (see details below) it certainly looked like the patches
were renaming subsets of APIs from *the same classes* in several
rounds, rather than all at once.  That feels a bit unfriendly.

More inline.

> -----Original Message-----
> From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of Nicolai
> Hähnle via llvm-dev
> Sent: Tuesday, February 18, 2020 10:08 PM
> To: David Blaikie <dblaikie at gmail.com>
> Cc: llvm-dev <llvm-dev at lists.llvm.org>; Valentin Churavy
> <v.churavy at gmail.com>
> Subject: Re: [llvm-dev] amount of camelCase refactoring causing some
> downstream overhead
> 
> On Wed, Feb 19, 2020 at 3:22 AM David Blaikie via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
> > I think the somewhat unspoken change in LLVM social conventions (&
> somewhat policy, I think it's written down in some places) is the "keep
> patches as small as practically possible" - grouping unrelated renamings
> would be something I'd usually (without concern for downstream consumers)
> push back against for all the usual reasons: easier to review, easier to
> revert strategically if something goes wrong, etc.

Right, except mechanical formatting/renaming patches fall outside that
convention.  You don't clang-format a file in 3 rounds, you do it all
at once and get it over with.  The set of renamings here were not 
"unrelated" to my eye, it was renaming a big pile of nonconforming APIs 
(all in MC, I think) in multiple rounds, that looked like they all affected
essentially the same set of files.
Doing those in multiple patches was about as bad as a commit-revert-commit
sequence, from a downstream merge POV.  The latter is unavoidable, but the
former IS avoidable.

> >
> > What I'm not clear on is why one big rename patch is easier for a
> downstream consumer than two smaller renames - I haven't fully understood
> the nature of this particular downstream consumer's approach makes this
> interesting.

If you have (as I do) nonzero amounts of downstream code calling those
APIs, then each change is at least a build fail if not an actual merge
conflict.  Fixing these in N rounds costs more than fixing all in one go;
you're doing 1/N as much editing each time but still doing N builds to 
verify before you can commit the fixup.
Each conflict/failure is an interruption to the auto-merge and increases
how far behind it gets, until we have a period of no-problem merges and
can catch up again.  We don't like falling behind; we'd rather keep up
so problems that need fixing upstream can be done in a timely way.

--paulr

> 
> The question that really matters is whether the rename is fully
> automatic or not. If the rename is done by an automatic script (e.g.
> clang-tidy based), then the same script can be run "downstream" ahead
> of merges to avoid virtually all of the conflicts. And if the rename
> is performed by an automatic script, it seems like it'd actually be
> simpler for everybody to do it in one big patch, or perhaps one C++
> namespace at a time.
> 
> The biggest remaining problem in this scenario is with *users* of LLVM
> that call into the C++ API and aim to maintain source-compatibility
> against multiple versions of LLVM. I currently cannot see how that
> would work.
> 
> Full disclaimer about where I'm coming from: We (AMD's graphics
> compiler) are affected by the rename on two counts:
> 
> 1) In llvm-project itself, we're upstream (AMDGPU backend), but we do
> have fairly long-lived internal branches for development against
> unreleased hardware that are still kept up-to-date with automatic
> merges multiple times per day.
> 
> 2) Outside of llvm-project, the compiler frontend (llpc, which is also
> open-source) also tracks llvm-project master closely and calls into
> the C++ interface, so would also be affected.
> 
> (We currently don't attempt to maintain compatibility with multiple
> versions of LLVM, though it's something that I'd like us to do at some
> point.)
> 
> Cheers,
> Nicolai
> 
> 
> 
> >
> > - Dave