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

[Andrew Trick]

> On Jan 26, 2017, at 4:48 PM, Greg Parker <gparker at apple.com> wrote:
> 
> 
>> 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.
> 
> Counterargument: It's not an objective code quality degradation, it's a style choice that you don't like. 

It’s not about what I’m used to or what I find visually pleasing. It’s been a source of bugs in my experience. If my experience is unique, and people want to standardize on a different style, then I’ll just have to try to be more careful reading the code and write fewer complicated if-conditions.

> The first rule of code style is "do as the rest of the code does". 
> 
> Trailing binary operators are fine, as long as they are consistent. Mixing leading and trailing operators in the same code base is a greater harm to code comprehension than either one is on its own. Any difference between consistently leading and consistently trailing is small, and not worth the pain of a mass rewrite.

That’s why I haven’t brought this up with LLVM project. The style has already been standardized (although my old code still wraps operators the right way :). I’m not sure anyone thought about this and didn’t want to repeat the same mistake in Swift.

I’ve seen both styles in the Swift compiler, although looking around the code now my preferred convention seems pretty clearly out of style. I also spent time in the standard library code, which has very carefully thought out conventions and wraps operators the way I’m recommending. Moving toward that style seemed reasonable if most developers agreed.

If I am the only person that feels strongly about it, then we’re probably better of standardizing on LLVM’s defaults just because most of the code has already done that. So here’s the option I intentionally left out:

Option 4: Standardize Swift style based purely on LLVM.

  Let's standardize the Swift compiler using LLVM’s default style configuration.

> I consider this LLVM and Swift project style to be an objective code quality degradation:
>    if (condition)
>        single_line_body();
> 
> But the first rule of code style is "do as the rest of the code does", so I write my LLVM and Swift code like that.

I agree. If auto-indenting editors hadn’t solved that problem I’d be arguing about that too!

-Andy