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

Fangrui Song via llvm-dev llvm-dev at lists.llvm.org
Tue Feb 18 16:28:27 PST 2020


>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".


More information about the llvm-dev mailing list