llvm dev - Feb 2020 - amount of camelCase refactoring causing some downstream overhead

Hi there,

At the end of last week we saw a number of commits go in that were camelCasing
batches of MCStreamer::Emit* and AsmPrinter::Emit* functions.

For example:
- https://reviews.llvm.org/rG549b436beb4129854e729a3e1398f03429149691
- https://reviews.llvm.org/rGa55daa146166353236aa528546397226bee9363b
- https://reviews.llvm.org/rG0bc77a0f0d1606520c7ad0ea72c434661786a956

Unfortunately all these individual commits trigger the same merge conflicts over
and over again with our downstream repo, which takes us some manual intervention
every time.

I understand uniformity is a nice to have, but:
1 - is it worth it to do this work right now? I can remember the casing debate a
few months back, which seems unrelated to this work which seems manual, but
I'm unsure of the outcome.
2 - If this work should be done, it would be nice if all of the work is done in
one batch, to save us some of the downstream overhead.

Thanks
/Ties

My usual take on this would be that it's within the LLVM project norms to
fix up naming on a case by case basis (independent of the recent discussion
you mentioned) - especially if different subsets of a single
interface/group of related interfaces have become more visibly inconsistent.

On Mon, Feb 17, 2020 at 4:04 AM Ties Stuij via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Hi there,
>
> At the end of last week we saw a number of commits go in that were
> camelCasing batches of MCStreamer::Emit* and AsmPrinter::Emit* functions.
>
> For example:
> - https://reviews.llvm.org/rG549b436beb4129854e729a3e1398f03429149691
> - https://reviews.llvm.org/rGa55daa146166353236aa528546397226bee9363b
> - https://reviews.llvm.org/rG0bc77a0f0d1606520c7ad0ea72c434661786a956
>
> Unfortunately all these individual commits trigger the same merge
> conflicts over and over again with our downstream repo, which takes us some
> manual intervention every time.
>
> I understand uniformity is a nice to have, but:
> 1 - is it worth it to do this work right now? I can remember the casing
> debate a few months back, which seems unrelated to this work which seems
> manual, but I'm unsure of the outcome.
> 2 - If this work should be done, it would be nice if all of the work is
> done in one batch, to save us some of the downstream overhead.
>
> Thanks
> /Ties
> _______________________________________________
> 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/20200217/a6f38ab9/attachment.html>

My understanding is that LLVM's general policy is to not let
downstream slow down upstream development. The C++ API explicitly
unstable.

Note that we are even considering renaming variables globally:
https://lists.llvm.org/pipermail/llvm-dev/2019-September/134921.html

Michael

Am Mo., 17. Feb. 2020 um 06:04 Uhr schrieb Ties Stuij via llvm-dev
<llvm-dev at lists.llvm.org>:
>
> Hi there,
>
> At the end of last week we saw a number of commits go in that were
camelCasing batches of MCStreamer::Emit* and AsmPrinter::Emit* functions.
>
> For example:
> - https://reviews.llvm.org/rG549b436beb4129854e729a3e1398f03429149691
> - https://reviews.llvm.org/rGa55daa146166353236aa528546397226bee9363b
> - https://reviews.llvm.org/rG0bc77a0f0d1606520c7ad0ea72c434661786a956
>
> Unfortunately all these individual commits trigger the same merge conflicts
over and over again with our downstream repo, which takes us some manual
intervention every time.
>
> I understand uniformity is a nice to have, but:
> 1 - is it worth it to do this work right now? I can remember the casing
debate a few months back, which seems unrelated to this work which seems manual,
but I'm unsure of the outcome.
> 2 - If this work should be done, it would be nice if all of the work is
done in one batch, to save us some of the downstream overhead.
>
> Thanks
> /Ties
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

During that variable renaming debate, there was a discussion about discussion
about doing things all at once, piecemeal or not at all. An issue that
wasn't really resolved I think. I had the impression that the efforts
fizzled out a bit, and I thought this renaming was maybe related to that, but
I'm neutral on if we should do variable renaming.

All I'm asking as a kindness if we could be kind on poor downstream
maintainers not on the issue of variable renaming at large, but on the micro
level of not pushing 5/6 patches of this kind covering closely related
functionality in two days but collating them in 1. I don't think that would
slow down development, and I wanted to highlight the issue, as people might not
be aware that they could save some pain in a simple way. Especially if indeed
there is going to be a big renaming push and this would be a continuous thing.

Cheers,
/Ties

________________________________________
From: Michael Kruse <llvmdev at meinersbur.de>
Sent: 17 February 2020 21:16
To: Ties Stuij
Cc: llvm-dev
Subject: Re: [llvm-dev] amount of camelCase refactoring causing some downstream
overhead

My understanding is that LLVM's general policy is to not let
downstream slow down upstream development. The C++ API explicitly
unstable.

Note that we are even considering renaming variables globally:
https://lists.llvm.org/pipermail/llvm-dev/2019-September/134921.html

Michael

Am Mo., 17. Feb. 2020 um 06:04 Uhr schrieb Ties Stuij via llvm-dev
<llvm-dev at lists.llvm.org>:
>
> Hi there,
>
> At the end of last week we saw a number of commits go in that were
camelCasing batches of MCStreamer::Emit* and AsmPrinter::Emit* functions.
>
> For example:
> - https://reviews.llvm.org/rG549b436beb4129854e729a3e1398f03429149691
> - https://reviews.llvm.org/rGa55daa146166353236aa528546397226bee9363b
> - https://reviews.llvm.org/rG0bc77a0f0d1606520c7ad0ea72c434661786a956
>
> Unfortunately all these individual commits trigger the same merge conflicts
over and over again with our downstream repo, which takes us some manual
intervention every time.
>
> I understand uniformity is a nice to have, but:
> 1 - is it worth it to do this work right now? I can remember the casing
debate a few months back, which seems unrelated to this work which seems manual,
but I'm unsure of the outcome.
> 2 - If this work should be done, it would be nice if all of the work is
done in one batch, to save us some of the downstream overhead.
>
> Thanks
> /Ties
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

>On Mon, Feb 17, 2020 at 4:04 AM Ties Stuij via llvm-dev <
>llvm-dev at lists.llvm.org> wrote:
>
>> Hi there,
>>
>> At the end of last week we saw a number of commits go in that were
>> camelCasing batches of MCStreamer::Emit* and AsmPrinter::Emit*
functions.
>>
>> For example:
>> - https://reviews.llvm.org/rG549b436beb4129854e729a3e1398f03429149691
>> - https://reviews.llvm.org/rGa55daa146166353236aa528546397226bee9363b
>> - https://reviews.llvm.org/rG0bc77a0f0d1606520c7ad0ea72c434661786a956
>>
>> Unfortunately all these individual commits trigger the same merge
>> conflicts over and over again with our downstream repo, which takes us
some
>> manual intervention every time.
>>
>> I understand uniformity is a nice to have, but:
>> 1 - is it worth it to do this work right now? I can remember the casing
>> debate a few months back, which seems unrelated to this work which
seems
>> manual, but I'm unsure of the outcome.
>> 2 - If this work should be done, it would be nice if all of the work is
>> done in one batch, to save us some of the downstream overhead.
>>
>> Thanks
>> /Ties
On 2020-02-17, David Blaikie wrote:
>My usual take on this would be that it's within the LLVM project norms
to
>fix up naming on a case by case basis (independent of the recent discussion
>you mentioned) - especially if different subsets of a single
>interface/group of related interfaces have become more visibly inconsistent.
I want to mention a few points:

* These refactoring commits are made within the [20200213,20200215] time frame.
   It started on Thursday afternoon (Pacific Time). For many users, it is
   "oh, it started on Friday and ended in the weekend".
   They unlikely cause "over and over again" conflicts.
* MCStreamer is usually not inherited.
   AsmPrinter can be inherited by a downstream target (think
lib/Target/$target).
   However, for most downstream users, they should not observe anything.
   The changes just appear to be gigantic. Its impact is actually much
   smaller than an interface change of
DIBuilder::createGlobalVariableExpression,
   MaybeAlign, IRBuilderBase::CreateMemSet, or deleted implicit conversion
   from StringRef to std::string.
* Before the refactoring, AsmPrinter and MCStreamer were in a very
   unpleasant inconsistent status: many use Emit* while some use emit*.
   It was a pain to think "I probably need to use the uncommon Emit*
because
   AsmPrinter/MCStreamer have a different convention".