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