[swift-dev] Using git-clang-format in the Swift compiler code base.

Michael Gottesman mgottesman at apple.com
Thu Jan 26 13:43:49 CST 2017


Can I make a more radical suggestion. Maybe the thing to do here is to enforce git-clang-format so that everyone uses it. It would be really simple to do. You would require people to put it in a pre commit hook. Then swift-ci would have a job that ran git-clang-format and would require it to come through clean.

Michael

> On Jan 26, 2017, at 2:07 AM, Andrew Trick via swift-dev <swift-dev at swift.org> wrote:
> 
> Before I pull in a large PR that "accidentally" reformats a bunch of code, I want to get feedback on how Swift compiler devs plan to use `clang-format`. (BTW, here's the PR https://github.com/apple/swift/pull/6922).
> 
> During the code review, I ran `git clang-format` as part of being a good citizen. There's only one problem with the tool. It rewraps long boolean expressions, hiding those unsightly operators at the end of lines (after all who wants to see those?).
> 
>        if (some_expression->with_calls() ||
>            another_expression->with(nested_calls()) &&
>            an_even_longer_expression->making_your_eyes->glaze_over())
> 
> I don't get involved in style wars, but this is not a style change, it's an objective code quality degradation. It's a demonstrably bug-prone thing to do. It's hurt me too many times in the past, and I'm not happy using a formatting tool that injects future bugs and harms code comprehension.
> 
> When the LLVM coding style was originally ratified, this aspect was left up to individual preference and didn't get any discussion AFAIK. I think
> clang-format then ended up with a `BreakBeforeBinaryOperators: None` style out of sheer inertia. 
> 
> So, how should I use clang-format on Swift compiler code? Vote please.
> 
> ** Option 1: Add a simple configuration option to swift/.clang-format:
> 
> 1a. BreakBeforeBinaryOperators: All
> 
> 1b. BreakBeforeBinaryOperators: NonAssignment
> 
> I have absolutely no preference between 1a and 1b. It's purely style.
> 
> 1a:
> SomeLongTypeName someLongVariableName =
>  someLongExpression();
> 
> 1b:
> SomeLongTypeName someLongVariableName
>  = someLongExpression();
> 
> ** Option 2: Contributors each tweak clang-format according to their (in my case strong) bias:
> 
> My own command line:
> 2a. " --style "{BasedOnStyle: LLVM, BreakBeforeBinaryOperators: All}"
> 2b. " --style "{BasedOnStyle: LLVM, BreakBeforeBinaryOperators: NonAssignment}"
> 
> ** Option 3: Don't bother using clang-format.
> 
> Let emacs do its indentation thing. Embrace idiosyncrasies in the code base.
> 
> -Andy
> _______________________________________________
> swift-dev mailing list
> swift-dev at swift.org
> https://lists.swift.org/mailman/listinfo/swift-dev



More information about the swift-dev mailing list