Using git-clang-format in the Swift compiler code base.

Development Compiler
1

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

2

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();

1b sounds good to me.

···

On Jan 26, 2017, at 2:07 AM, Andrew Trick via swift-dev <swift-dev@swift.org> wrote:

** 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@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

3

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.

It's funny you'd mention this! I often format code that way, not out of any great love of it, but from muscle-memory of living under an old coding guideline somewhere in the distant past claiming that the ugliness of trailing unfinished-binops draws the eye to them and makes the user pay attention. Doug Crockford recommends this style; but of course Don Knuth agrees with you. I don't feel strongly about them as such, but I feel ... anti-strongly, I guess? Like changing that one thing isn't worth a cross-codebase rewrite / merge collision.

That said ...

** Option 2: Contributors each tweak clang-format according to their (in my case strong) bias:

This option seems least-desirable to me. My preference would be that code stays formatted as uniformly and invariably as possible; I'm surprised to notice we have a .clang-format in the repo, given that it's not being enforced (and there's not even a build-system rule to use it, as far as I can see). How would people feel about automatic enforcement? i.e. testing a patch fails if clang-format has nonempty tree-reformatting to do.

-Graydon

(Who is happy to setq c-indentation-style to anything from Allman to Whitesmith)

···

On Jan 26, 2017, at 2:07 AM, Andrew Trick via swift-dev <swift-dev@swift.org> wrote:

4

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@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@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

5

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())

Well, I mean, I hope nobody mixes || and && like this. :)

Anyway, a vote:

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.

I agree with your point that we should discourage breaking after the logical
operators, and maybe also + and so on. Relational and assignment operators
I care less about, and I don't see a strong reason to force a choice, so I'm with
option #2.

John.

···

On Jan 26, 2017, at 5:07 AM, Andrew Trick via swift-dev <swift-dev@swift.org> wrote:

-Andy
_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

6

Counterargument: It's not an objective code quality degradation, it's a style choice that you don't like.

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.

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.

···

On Jan 26, 2017, at 2:07 AM, Andrew Trick via swift-dev <swift-dev@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.

--
Greg Parker gparker@apple.com Runtime Wrangler

7

I contradicted myself above. If you like the style shown in (1b), the configuration option is actually BreakBeforeBinaryOperators: All.

-Andy

···

On Jan 26, 2017, at 9:11 AM, Ben Langmuir <blangmuir@apple.com> wrote:

** 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();

1b sounds good to me.

8

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.

It's funny you'd mention this! I often format code that way, not out of any great love of it, but from muscle-memory of living under an old coding guideline somewhere in the distant past claiming that the ugliness of trailing unfinished-binops draws the eye to them and makes the user pay attention. Doug Crockford recommends this style; but of course Don Knuth agrees with you. I don't feel strongly about them as such, but I feel ... anti-strongly, I guess? Like changing that one thing isn't worth a cross-codebase rewrite / merge collision.

I’m not sure who’s recommending what. The above style obscures operators. Does anyone disagree with that? A lot of code has been written in that way; I think because developers value aesthetics over clarity, or just don’t think about it. I care because when something doesn’t stand out, my brain fills in the gaps with whatever it expects. For me, that leads to a bunch of silly logic errors.

I need to see it this way:

      if (some_expression->with_calls()
          >> another_expression->with(nested_calls())
          && an_even_longer_expression->making_your_eyes->glaze_over())

The need for parens now stands out. Sorry this isn’t a good example. Nested expressions would make it much more compelling.

That’s the coding convention we use for Swift code (at least in the stdlib). The compiler C++ code is just a hodge-podge.
If anyone actually thinks trailing operators are a good idea for our code base, I won’t argue, but I’ve never heard that argument.

BTW- I’m not interested at all in doing a mass reformatting or forcing a convention on people. I just don’t want to apply clang-format to every line of code I touch without knowing what settings to use.

-Andy

···

On Jan 26, 2017, at 11:38 AM, Graydon Hoare <ghoare@apple.com> wrote:

On Jan 26, 2017, at 2:07 AM, Andrew Trick via swift-dev <swift-dev@swift.org> wrote:

9

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

···

On Jan 26, 2017, at 4:48 PM, Greg Parker <gparker@apple.com> wrote:

On Jan 26, 2017, at 2:07 AM, Andrew Trick via swift-dev <swift-dev@swift.org> wrote:

10

** 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();

1b sounds good to me.

I contradicted myself above. If you like the style shown in (1b), the configuration option is actually BreakBeforeBinaryOperators: All.

Glad you mentioned it, because I prefer “NonAssignment”, but didn’t check your example code against the above description :-)

···

On Jan 26, 2017, at 9:14 AM, Andrew Trick <atrick@apple.com> wrote:

On Jan 26, 2017, at 9:11 AM, Ben Langmuir <blangmuir@apple.com <mailto:blangmuir@apple.com>> wrote:

-Andy

11

I've never had a problem with the trailing operators, and find them mildly more aesthetically pleasing (when not in an if, it makes it clearer what I plan to do with the next line), but I see how they put it in your face in an if. I can change my style.

Jordan

···

On Jan 26, 2017, at 12:55, Andrew Trick via swift-dev <swift-dev@swift.org> wrote:

On Jan 26, 2017, at 11:38 AM, Graydon Hoare <ghoare@apple.com> wrote:

On Jan 26, 2017, at 2:07 AM, Andrew Trick via swift-dev <swift-dev@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.

It's funny you'd mention this! I often format code that way, not out of any great love of it, but from muscle-memory of living under an old coding guideline somewhere in the distant past claiming that the ugliness of trailing unfinished-binops draws the eye to them and makes the user pay attention. Doug Crockford recommends this style; but of course Don Knuth agrees with you. I don't feel strongly about them as such, but I feel ... anti-strongly, I guess? Like changing that one thing isn't worth a cross-codebase rewrite / merge collision.

I’m not sure who’s recommending what. The above style obscures operators. Does anyone disagree with that? A lot of code has been written in that way; I think because developers value aesthetics over clarity, or just don’t think about it. I care because when something doesn’t stand out, my brain fills in the gaps with whatever it expects. For me, that leads to a bunch of silly logic errors.

I need to see it this way:

     if (some_expression->with_calls()
         >> another_expression->with(nested_calls())
         && an_even_longer_expression->making_your_eyes->glaze_over())

The need for parens now stands out. Sorry this isn’t a good example. Nested expressions would make it much more compelling.

That’s the coding convention we use for Swift code (at least in the stdlib). The compiler C++ code is just a hodge-podge.
If anyone actually thinks trailing operators are a good idea for our code base, I won’t argue, but I’ve never heard that argument.

BTW- I’m not interested at all in doing a mass reformatting or forcing a convention on people. I just don’t want to apply clang-format to every line of code I touch without knowing what settings to use.

12

Alright, I’l reformat my PR with that config, unless anyone else wants to weigh in.

Incidentally, I despise what clang-format does with asserts now:
  assert(condition
             && “text”)

It’s a consequence of us not using a legit assert package, so I don’t know if I want to push to get clang-format changed.

-Andy

···

On Jan 26, 2017, at 9:29 AM, Ben Langmuir <blangmuir@apple.com> wrote:

On Jan 26, 2017, at 9:14 AM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:

On Jan 26, 2017, at 9:11 AM, Ben Langmuir <blangmuir@apple.com <mailto:blangmuir@apple.com>> wrote:

** 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();

1b sounds good to me.

I contradicted myself above. If you like the style shown in (1b), the configuration option is actually BreakBeforeBinaryOperators: All.

Glad you mentioned it, because I prefer “NonAssignment”, but didn’t check your example code against the above description :-)

13

What do you want it to do with this style of assert?

···

On Jan 26, 2017, at 09:35, Andrew Trick via swift-dev <swift-dev@swift.org> wrote:

On Jan 26, 2017, at 9:29 AM, Ben Langmuir <blangmuir@apple.com <mailto:blangmuir@apple.com>> wrote:

On Jan 26, 2017, at 9:14 AM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:

On Jan 26, 2017, at 9:11 AM, Ben Langmuir <blangmuir@apple.com <mailto:blangmuir@apple.com>> wrote:

** 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();

1b sounds good to me.

I contradicted myself above. If you like the style shown in (1b), the configuration option is actually BreakBeforeBinaryOperators: All.

Glad you mentioned it, because I prefer “NonAssignment”, but didn’t check your example code against the above description :-)

Alright, I’l reformat my PR with that config, unless anyone else wants to weigh in.

Incidentally, I despise what clang-format does with asserts now:
  assert(condition
             && “text”)

It’s a consequence of us not using a legit assert package, so I don’t know if I want to push to get clang-format changed.

14

assert((precondition1 || predcondition2) &&
           “informative message”)

The boolean operator is filling in for a comma. That’s how it’s meant to be parsed by the reader. It’s presence in the code is purely distracting. From the tool’s point of view, it’s tautological so shouldn’t be given significance as a boolean operator.

-Andy

···

On Jan 26, 2017, at 10:25 AM, Jordan Rose <jordan_rose@apple.com> wrote:

On Jan 26, 2017, at 09:35, Andrew Trick via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:

On Jan 26, 2017, at 9:29 AM, Ben Langmuir <blangmuir@apple.com <mailto:blangmuir@apple.com>> wrote:

On Jan 26, 2017, at 9:14 AM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:

On Jan 26, 2017, at 9:11 AM, Ben Langmuir <blangmuir@apple.com <mailto:blangmuir@apple.com>> wrote:

** 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();

1b sounds good to me.

I contradicted myself above. If you like the style shown in (1b), the configuration option is actually BreakBeforeBinaryOperators: All.

Glad you mentioned it, because I prefer “NonAssignment”, but didn’t check your example code against the above description :-)

Alright, I’l reformat my PR with that config, unless anyone else wants to weigh in.

Incidentally, I despise what clang-format does with asserts now:
  assert(condition
             && “text”)

It’s a consequence of us not using a legit assert package, so I don’t know if I want to push to get clang-format changed.

What do you want it to do with this style of assert?

15

The idiomatic understanding is that the last && is "really" a comma.

John.

···

On Jan 26, 2017, at 1:25 PM, Jordan Rose via swift-dev <swift-dev@swift.org> wrote:

On Jan 26, 2017, at 09:35, Andrew Trick via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:

On Jan 26, 2017, at 9:29 AM, Ben Langmuir <blangmuir@apple.com <mailto:blangmuir@apple.com>> wrote:

On Jan 26, 2017, at 9:14 AM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:

On Jan 26, 2017, at 9:11 AM, Ben Langmuir <blangmuir@apple.com <mailto:blangmuir@apple.com>> wrote:

** 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();

1b sounds good to me.

I contradicted myself above. If you like the style shown in (1b), the configuration option is actually BreakBeforeBinaryOperators: All.

Glad you mentioned it, because I prefer “NonAssignment”, but didn’t check your example code against the above description :-)

Alright, I’l reformat my PR with that config, unless anyone else wants to weigh in.

Incidentally, I despise what clang-format does with asserts now:
  assert(condition
             && “text”)

It’s a consequence of us not using a legit assert package, so I don’t know if I want to push to get clang-format changed.

What do you want it to do with this style of assert?

16

clang-format bug filed to put this tangent to rest:
31772 – clang-format BreakBeforeBinaryOperators mishandles the standard assert idiom. <https://llvm.org/bugs/show_bug.cgi?id=31772&gt;

-Andy

···

On Jan 26, 2017, at 10:34 AM, Andrew Trick via swift-dev <swift-dev@swift.org> wrote:

On Jan 26, 2017, at 10:25 AM, Jordan Rose <jordan_rose@apple.com <mailto:jordan_rose@apple.com>> wrote:

On Jan 26, 2017, at 09:35, Andrew Trick via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:

On Jan 26, 2017, at 9:29 AM, Ben Langmuir <blangmuir@apple.com <mailto:blangmuir@apple.com>> wrote:

On Jan 26, 2017, at 9:14 AM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:

On Jan 26, 2017, at 9:11 AM, Ben Langmuir <blangmuir@apple.com <mailto:blangmuir@apple.com>> wrote:

** 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();

1b sounds good to me.

I contradicted myself above. If you like the style shown in (1b), the configuration option is actually BreakBeforeBinaryOperators: All.

Glad you mentioned it, because I prefer “NonAssignment”, but didn’t check your example code against the above description :-)

Alright, I’l reformat my PR with that config, unless anyone else wants to weigh in.

Incidentally, I despise what clang-format does with asserts now:
  assert(condition
             && “text”)

It’s a consequence of us not using a legit assert package, so I don’t know if I want to push to get clang-format changed.

What do you want it to do with this style of assert?

assert((precondition1 || predcondition2) &&
           “informative message”)

The boolean operator is filling in for a comma. That’s how it’s meant to be parsed by the reader. It’s presence in the code is purely distracting. From the tool’s point of view, it’s tautological so shouldn’t be given significance as a boolean operator.