[LLVMdev] running clang format on the Mips target

[Daniel Sanders]

Thanks for the recommendations.

The MIPS team itself is unanimously agreed that an initial formatting is worth the cost. I've had an objection from David Chisnall which I'm trying to accommodate by waiting for him to upstream the relevant parts of his out-of-tree modifications before going ahead. I haven't had any other objections but I intend to send an email giving three weeks' notice calling for comments/objections in case there are others.

At the moment, the MIPS backend is generally consistent inside each file but not between them. When I ran clang-format on lib/Target/Mips/*.{cpp,h} in my working copy, the diff was 555KB, so I'd say it's generally inconsistent with clang-format.

From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] On Behalf Of Daniel Jasper
Sent: 09 January 2014 11:19
To: Manuel Klimek
Cc: Reed Kotler; Tobias Grosser; LLVMdev at cs.uiuc.edu
Subject: Re: [LLVMdev] running clang format on the Mips target

I would slightly alter this recommendation depending on whether the codebase is currently in a reasonably consistent state (both internally and wrt. what clang-format would do).

The basic assumption is that code is easier to read if it is more consistent.

If the code base is currently inconsistently formatted, then a full reformatting might provide benefit. If it is significantly different from clang-format, then an incremental approach has the cost of making the code more inconsistent (at least temporarily).

On Thu, Jan 9, 2014 at 12:03 PM, Manuel Klimek <klimek at google.com<mailto:klimek at google.com>> wrote:
FYI, our usual recommendation is to use clang-format only on changed code, unless there's strong evidence that the cost of the initial reformatting is basically zero (which usually means a team / community is pretty unanimous about the decision).

On Thu, Jan 9, 2014 at 11:53 AM, Tobias Grosser <tobias at grosser.es<mailto:tobias at grosser.es>> wrote:
Manuel, Daniel,

I forgot to CC you in this mail. The thread here may be interesting to you, also in the light of the last discussion we had.

Tobias


On 12/21/2013 12:00 AM, Tobias Grosser wrote:
On 12/20/2013 11:18 PM, Rafael Espíndola wrote:
On 20 December 2013 15:52, reed kotler <rkotler at mips.com<mailto: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.




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140113/9f7646f6/attachment.html>