[llvm-dev] amount of camelCase refactoring causing some downstream overhead (original) (raw)
Fangrui Song via llvm-dev llvm-dev at lists.llvm.org
Tue Feb 18 16:28:27 PST 2020
- Previous message: [llvm-dev] amount of camelCase refactoring causing some downstream overhead
- Next message: [llvm-dev] amount of camelCase refactoring causing some downstream overhead
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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".
- Previous message: [llvm-dev] amount of camelCase refactoring causing some downstream overhead
- Next message: [llvm-dev] amount of camelCase refactoring causing some downstream overhead
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]