r212591 - clang-format: Revamp function declaration/definition indentation.
djasper at google.com
Wed Jul 9 02:33:19 PDT 2014
On Wed, Jul 9, 2014 at 11:08 AM, Tobias Grosser <tobias at grosser.es> wrote:
> On 09/07/2014 10:47, Daniel Jasper wrote:
>> Yes, this is intended and I actually don't think that it is such a big
>> difference. Multiline variable declaration that are broken after the type
>> are actually reasonably rare.
> In Polly alone it changes 180 lines (43 declarations)
> LLVM has 276 command line declarations that are currently broken
> after the type (grep -R 'cl::opt<.*>$' lib/ | wc):
> static cl::opt<bool>
> VarName("options, ...")
> and 232 that are formatted
a) That is not a lot (<1% of the codebase).
b) I think volume of change is actually not all that relevant when deciding
on what formatting should be.
> static cl::opt<std::string> OnlyFunction(
> If you look at the last five commit messages that explicitly mention
> clang-format three contain formatting changes of exactly these constructs:
> -static cl::list<std::string> ArchFlags("arch",
> - cl::desc("architecture(s) from a Mach-O file to dump"),
> - cl::ZeroOrMore);
> +static cl::list<std::string>
> +ArchFlags("arch", cl::desc("architecture(s) from a Mach-O file to dump"),
> + cl::ZeroOrMore);
> -static cl::opt<OutputFormatTy>
> - OutputFormatShort(cl::desc("Specify output format"),
> - cl::values(clEnumValN(sysv, "A", "System V format"),
> - clEnumValN(berkeley, "B", "Berkeley format"),
> - clEnumValN(darwin, "m", "Darwin -m format"),
> - clEnumValEnd),
> - cl::init(berkeley));
> +OutputFormat("format", cl::desc("Specify output format"),
> + cl::values(clEnumVal(sysv, "System V format"),
> + clEnumVal(berkeley, "Berkeley format"),
> + clEnumVal(darwin, "Darwin -m format"),
> + cl::init(berkeley));
> -static cl::opt<bool>
> -EnableAArch64UnscaledMemOp("aarch64-unscaled-mem-op", cl::Hidden,
> - cl::desc("Allow AArch64 unscaled load/store
> - cl::init(true));
> +static cl::opt<bool> EnableAArch64UnscaledMemOp(
> + "aarch64-unscaled-mem-op", cl::Hidden,
> + cl::desc("Allow AArch64 unscaled load/store combining"),
> Happy, to take a vote on this though..
> Sure, would it be OK to revert this change meanwhile to get the build bot
> clean again.
No. I think it is highly likely, we are going to stick with this change.
Moreover, I think the issue here is that we have a buildbot enforcing a
format based on clang-format's HEAD version. Lets do one of:
- Remove formatting test from buildbot. I think formatting is important,
but it is good enough to clean this up every now and then, e.g. in a
manually invoked step. Having incorrect indentation for a few days doesn't
do any harm. That would also give room to separate formatting-only commits
from other commits.
- Use a released version (or other known good version) of clang-format for
these tests. We need to decouple clang-format commits from the cleanup of
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the cfe-commits