[llvm-dev] [RFC] Semi-Automatic clang-format of files with low frequency

Tue Jun 30 20:36:22 PDT 2020

On Tue, Jun 30, 2020 at 11:45 AM Hal Finkel via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

>
> On 6/30/20 1:19 PM, MyDeveloper Day via llvm-dev wrote:
>
> I 100% get that we might not like the decisions clang-format is making,
> but how does one overcome this when adding new code? The pre-merge checks
> enforce clang-formatting before commit and that's a common review comment
> anyway for those who didn't join the pre-merge checking group.
>
>
> Those checks are advisory. I do not recall a case where
> thoughtfully-hand-formatted code was proposed, adhering to the coding
> guidelines, and a reviewer asked for the file to be run through
> clang-format.
>
In general I'd ask for the magic comment that disables clang-format on the
particular section though, which keeps the file clean with respect to
clang-format.

> When the thoughtfully-formatted code does not match what clang-format
> would produce in such cases, it's generally obvious why. Normally, this
> request is made where the file is inconsistently formatted, or formatted in
> a way that obviously does not comply with our guidelines.
>
I've observed that there is a wide spectrum of developer preferences: some
> thoughtfully and deliberately format all of their code, others deal with
> formatting only when they feel like they absolutely must and are happy to
> have a tool that does anything consistent. Most people are probably
> somewhere in between.
>
>
> I'm just wondering are we not all following the same guidelines?
>
>
> We have specific guidelines that everyone should follow. clang-format
> implements a superset of those.
>
> Here's a counter-proposal: You should hook up a tool that automatically
> opens Phabricator reviews to clang-format the source code. It should do
> this at a very low rate. It may take years to get through everything. But,
> humans will look at the formatting changes, and where clang-format should
> be improved instead of changing the code, we'll discover and classify those
> things. Formatted files can be noted, however, to incrementally build the
> clang-format test suite
>

That seems like a great idea actually!

> .
>
>
>
> Concerns of clang-format not being good enough are fair enough, but that's
> the area I'm focusing my development efforts on (as that's where I've been
> trying to make a small contribution). This effort was driven out of a need
> in my view to have an extensive test suite to validate new changes to
> clang-format don't break the formatting of pre-formatted code. This isn't
> that possible with LLVM at the moment because large parts are not formatted.
>
> However checking a code base which is in high flux but one that maintains
> a 100% clang-format clean status, is a near perfect test-suite. Especially
> one that I assume will have unit tests for the latest C++ features.
>
> I'm not bored ;-) and whilst I understand this feels somewhat periphery to
> the seemingly much more pressing task of developing the next best compiler,
> I think we have to remember that clang-format is extensively used. (In my
> view by many more people than those actually using clang). GitHub reports
> 32,000 YAML files with the BasedOnStyle: LLVM alone that is present in the
> .clang-format file
>
>
> Independent of everything else, this is a great success. Maybe we should
> have a website with a tracker on it or something.
>
> Thanks again,
>
> Hal
>
>
>
> I realize it feels like unnecessary churn which is why I suggested this be
> in code which hasn't been touched in some time (I'd be fine if that time is
> 1+ years), but more often than not this is quite small basic style issues,
> similar to the ones below.
>
> MyDeveloperDay
>
> -  void HandleTranslationUnit(ASTContext& context) override {
> +  void HandleTranslationUnit(ASTContext &context) override {
>      if (!Instance.getLangOpts().DelayedTemplateParsing)
>        return;
>
> -      std::set<FunctionDecl*> LateParsedDecls;
> +      std::set<FunctionDecl *> LateParsedDecls;
>
> On Tue, Jun 30, 2020 at 6:55 PM Matt Arsenault <arsenm2 at gmail.com> wrote:
>
>>
>>
>> On Jun 28, 2020, at 11:30, MyDeveloper Day via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>> I’m a contributor to clang-format and would like to see LLVM 100% clang
>> formatted so we can use LLVM as a massive test-suite for clang-format when
>> we make changes.
>>
>>
>> My main issue with this would be that clang-format does things that I
>> don’t believe are stated in the LLVM style guide and I also disagree with.
>> There’s a whole set of cases where it makes unwelcome changes to put things
>> that were on separate lines on a single line. Whenever I run clang-format,
>> I typically git checkout -p to discard all of these.
>>
>> For example, it likes to take member initializer lists and pack them into
>> as few lines as possible. This is just asking for merge conflicts (e.g.
>> AMDGPUSubtarget has a ton of fields in it, and out of tree code is
>> constantly adding new fields for unreleased subtargets). It also mangles
>> BuildMI calls, where I believe every .add() should be on its own line, and
>> stringing this into BuildMI().addFoo().addBar() is way less readable.
>>
>> I also believe it’s 4 space indent on line wraps differs from the stated
>> 2 space indent level (and this also disagrees with emacs llvm-style)
>>
>> -Matt
>>
>
> _______________________________________________
> LLVM Developers mailing listllvm-dev at lists.llvm.orghttps://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
> --
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
>
> _______________________________________________
> 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/20200630/5c69d36b/attachment.html>