[llvm-dev] amount of camelCase refactoring causing some downstream overhead (original) (raw)

Robinson, Paul via llvm-dev llvm-dev at lists.llvm.org
Wed Feb 19 07:53:01 PST 2020

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

More information about the llvm-dev mailing list