llvm dev - Dec 2013 - [LLVMdev] running clang format on the Mips target

We are considering running clang format on the whole Mips target.

Is there any rule against this?

Is there any good argument against doing this even if there is no rule 
against it?

TIA.

Reed

On 20 December 2013 15:52, reed kotler <rkotler at mips.com>
wrote:
> We are considering running clang format on the whole Mips target.
>
> Is there any rule against this?
I don't think there is a rule, but even simpler things like "remove
all trailing spaces" were not common in the past
> Is there any good argument against doing this even if there is no rule
> against it?
It think the argument was that it makes history harder to follow. A
git blame lands you on the reformatting patch and you have to manually
skip them.

I don't personally consider that a strong argument and I think polly
did just that (run clang-format on everything), so it might be OK
these days.

Cheers,
Rafael

On 12/20/2013 11:18 PM, Rafael Espíndola wrote:
> On 20 December 2013 15:52, reed kotler <rkotler at mips.com> wrote:
>> We are considering running clang format on the whole Mips target.
>>
>> Is there any rule against this?
>
> I don't think there is a rule, but even simpler things like
"remove
> all trailing spaces" were not common in the past
>
>> Is there any good argument against doing this even if there is no rule
>> against it?
>
> It think the argument was that it makes history harder to follow. A
> git blame lands you on the reformatting patch and you have to manually
> skip them.
>
> I don't personally consider that a strong argument and I think polly
> did just that (run clang-format on everything), so it might be OK
> these days.
Unfortunately for Polly it was not as clean as we started using 
clang-format early, so we had to format piece by piece the sections that 
clang-format could already handle.

However, we now have an automatic clang-format buildbot so formatting 
became (in some way) a 'solved problem'.

One benefit of fully formatted source code is that there is no need for 
any new patches to include 'formatting fixes', which previously 
sometimes obfuscated submitted patches and commits. In fact, patch 
reviews are now free of formatting discussions. If the patch passes
make polly-check-format, it is ready to be discussed. (If it does not,
make polly-update-format needs to be run). Also, having a single 
formatting commit to skip might arguably be easier, than to have a mixed 
history of formatting and patches.

One reason that may hold us back from formatting all LLVM code is the 
risk of clang-format changing its formatting behavior. The only thing 
worse than formatting all code is formatting all code twice. ;)

 From my experience with Polly, clang-format's formatting has been 
largely stable the last months. Changes in its formatting behavior have 
normally been fixed quickly. So it may make sense to format more code.
I would still be afraid of formatting all LLVM, but maybe a backend is
a reasonable sized chunk.

Maybe Manual and Daniel have some experience in formatting a larger set 
of files.

Cheers,
Tobias

As someone with an  lot of out-of-tree changes to the MIPS back end, I would
consider this to be somewhat impolite.  I hope to upstream the changes that are
relevant to users of other MIPS-derived processors in the new year, and having
large numbers of formatting changes would make rebasing a lot more painful.

David

On 20 Dec 2013, at 20:52, reed kotler <rkotler at mips.com> wrote:
> We are considering running clang format on the whole Mips target.
> 
> Is there any rule against this?
> 
> Is there any good argument against doing this even if there is no rule
against it?
> 
> TIA.
> 
> Reed
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

Hi David,

What kind of  "a lot of out-of-tree changes"?

You should push changes incrementally as you do work. Holding onto changes means
that many things,
not just reformatting, can make them need to be redone.  We frequently clean up
and rewrite
code to make it cleaner and easier to maintain.

We are moving to a more strict internal review and pushing of changes and post
commit reviewing.

It takes time to review and respond to comments on formatting issues; time that
would be better spent doing new work.

So we would like to have robots, i.e. clang format, do this checking and such.

I would recommend that you start to submit your patches for review.


Reed

________________________________________
From: Dr D. Chisnall [dc552 at hermes.cam.ac.uk] on behalf of David Chisnall
[David.Chisnall at cl.cam.ac.uk]
Sent: Saturday, December 21, 2013 6:43 AM
To: Reed Kotler
Cc: LLVMdev at cs.uiuc.edu
Subject: Re: [LLVMdev] running clang format on the Mips target

As someone with an  lot of out-of-tree changes to the MIPS back end, I would
consider this to be somewhat impolite.  I hope to upstream the changes that are
relevant to users of other MIPS-derived processors in the new year, and having
large numbers of formatting changes would make rebasing a lot more painful.

David

On 20 Dec 2013, at 20:52, reed kotler <rkotler at mips.com> wrote:
> We are considering running clang format on the whole Mips target.
>
> Is there any rule against this?
>
> Is there any good argument against doing this even if there is no rule
against it?
>
> TIA.
>
> Reed
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev