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

[Chandler Carruth via llvm-commits]

chandlerc added a comment.

As a few others have indicated, I really don't like the proposed rule as written. I very much want to continue to write:

  if (...)
    for (...)
      ...;

and

  if (...)
    for (...) {
      ...;
      ...;
    }

I don't think there is any interesting ambiguity in these cases, and the braces seem to add clutter with little value.

I'd like a rule that strikes at the heart of the issue: use braces when they make the structure more clear. But I understand that this is subjective and we really should have a rule that is relatively mechanical for consistency's sake and gets close enough to the spirit of this to be a good fit.

Here is my attempt:

----

Omit braces when there is no ambiguity due to nesting and `else` statements.

Common cases of this are flagged with compiler warnings:

  if (...)
    if (...)
      ...;
    else // warning: ambiguous else!
      ...;

Even if the ambiguity happens to dodge the compiler warnings or other checking mechanism, you should still use braces:

  if (...)
    for (...)
      if (...)
        ...;
      else // Still bad!
        ...;

----

I like this (or something like this) formulation because:

1. It doesn't talk about number of lines. Do comments count as lines? what if the *condition* is multiple lines but the statement isn't? I don't like using line count here.

2. It precisely matches what compilers are trying to warn on already.

3. It gives grounds for a reviewer to argue that a highly confusing pattern, even if it doesn't literally result in ambiguity, is still sufficiently opaque as to require curlies. We could add some examples here if this actually comes up.

The primary alternative I see is tweaking the above to also require braces on all surrounding constructs once they are required for one. From discussions with Richard Smith, I think this is more common in Clang's code whereas the formulation I suggested is more common in LLVM code.

The reason I (quite strongly) prefer the formulation I proposed is in no small part because clang-format has completely removed the failure mode where I mis-indent things. As a consequence, I prefer the more minimal & brief approach to braces. I feel like it balances clarity and brevity well.

If we can't agree on a single consistent style between these two alternatives, then perhaps we end up saying both are acceptable and letting code reviewers choose, but I would generally prefer having clear guidance here.

My two cents.
-Chandler


https://reviews.llvm.org/D26943