[LLVMdev] running clang format on the Mips target

[Alp Toker]

On 13/01/2014 11:18, Daniel Sanders wrote:
>
> 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
>

Hi Daniel,

Without weighing in on either side of that discussion, I'd like to point 
out a technical solution to merging out-of-tree changes following 
upstream reformatting.

It should be possible to run something like the following on any 
out-of-tree branch to reformat each commit individually:

# Reformat divergent commits in history
git filter-branch --tree-filter "git diff -U0 | clang-format-diff.py -p1 
-i"
# Rebase to ToT
git rebase origin/master

With these commands, version history will be rewritten to match upstream 
formatting and the individual commits will apply without conflicts 
regardless of when the branch was forked. We've used variations of this 
to great effect.

So if you're absolutely decided that you want to reformat the whole 
tree, I don't see a strong reason to wait three weeks.

Alp.


> 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.
>
>
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

-- 
http://www.nuanti.com
the browser experts