[PATCH] D26943: [CodingStandards] Add style guide rule about "if" statements and loops.

[Krzysztof Parzyszek via llvm-commits]

kparzysz added a comment.

In https://reviews.llvm.org/D26943#605082, @mehdi_amini wrote:

> Then I have a strong disagreement on this:
>
> 1. clang-format totally changed the way I deal with formatting, and having rules that can be taught to an automatic tool removes any subjectivity and discussion about the formatting.
> 2. What makes the code "clear" and "readable" beyond the various tastes of contributors, *is* consistency across the codebase. Less brain power used to adapt to different styles is a non-negligible productivity gain to me.


My concern about automatic formatting is that it may damage context-specific formatting. Consider this example:

  unsigned FlagAB = FlagAlpha | FlagBravo;
  unsigned FlagAC = FlagAlpha             | FlagCharlie;
  unsigned FlagBC =             FlagBravo | FlagCharlie;

Here the spacing is intended to separate the individual flags in columns, and doing do may improve readability of this piece of code, which could otherwise appear cluttered. Automatic formatting could remove all of it, since it is not aware of the author's intent.  I know this is a trivial example, but I hope that it illustrates the concept.

In my experience, readability is affected to different degrees by different formatting elements.  Some factors are highly important (like spacing, e.g `(((x-1)|(x+1)*(x-2))+x)+(x-1)*(x+2)` vs `(((x-1) | (x+1)*(x-2)) + x) + (x-1)*(x+2)`, which makes a big difference in visual parsing), others are less so.  While I agree with the insistence on the important ones, I prefer to be lax about the others.  Small local differences are a natural consequence of having a large number of contributors and the evolution of the formatting guidelines themselves.  If at one point we decided that single-line if/for/while statements should also be bracketed, would we require that in a new code added to a file that consistently didn't have them? If so, it would be inconsistent with the existing code, if not, it would contravene the guidelines. Sweeping changes throughout the code base are a real pain for out-of-tree developers, so hopefully that wouldn't be considered.  IMO, it's best to allow a degree of flexibility and just live with it.


https://reviews.llvm.org/D26943