[clang-format] 'else if' clause not broken when using AlignAfterOpenBracket: BlockIndent

[yseymour]

clang-format version:
Debian clang-format version 14.0.0-++20220329040358+3f43d803382d-1~exp1~20220329160446.105

Code:

void foo() {
        if (thisIsRatherALongIfClause && thatIExpectToBeBroken || ontoMultipleLines && whenFormattedCorrectly) {
        }

        if (false) {
        }
        else if (thisIsRatherALongIfClause && thatIExpectToBeBroken || ontoMultipleLines && whenFormattedCorrectly) {
        }
}

Output with defaults:

❯ clang-format --style='{ColumnLimit: 60}' tmp.cc
void foo() {
  if (thisIsRatherALongIfClause && thatIExpectToBeBroken ||
      ontoMultipleLines && whenFormattedCorrectly) {
  }

  if (false) {
  } else if (thisIsRatherALongIfClause &&
                 thatIExpectToBeBroken ||
             ontoMultipleLines && whenFormattedCorrectly) {
  }
}

Output with AlignAfterOpenBracket: BlockIndent

❯ clang-format --style='{ColumnLimit: 60, AlignAfterOpenBracket: BlockIndent}' tmp.cc
void foo() {
  if (thisIsRatherALongIfClause && thatIExpectToBeBroken ||
      ontoMultipleLines && whenFormattedCorrectly) {
  }

  if (false) {
  } else if (thisIsRatherALongIfClause && thatIExpectToBeBroken || ontoMultipleLines && whenFormattedCorrectly) {
  }
}

Expected output:
First of all, I was intuitively expecting BlockIndent to break the if/else-if clause in a similar way to a function declaration:

void foo(
    int veryLongArgumentName1, int veryLongArgumentName2,
    int veryLongArgumentName3
) {}

and my hope was that there would be a BinPack... option to place each part of the clause on each line just as one can with arguments and parameters, but that doesn't seem to be the case.

If that's not possible and/or it's not a design goal for that option, then at the least I'd expect the else if clause to be broken, because currently it doesn't get wrapped at all when the BlockIndent alignment style is used, which seems buggy.

[llvmbot]

@llvm/issue-subscribers-clang-format

[luisrayas3]

I noticed this as well on v14.0.6.

I would have never guessed that BlockIndent was the culprit if it were not for this issue so thanks!

AlignAfterOpenBracket: AlwaysBreak seems to be the best workaround, of course at the cost of not having the closing parethesis on its own line which I want (though not as badly as I want else-if conditions that get broken). The example in the issue becomes:

void foo() {
  if (thisIsRatherALongIfClause && thatIExpectToBeBroken
      || ontoMultipleLines && whenFormattedCorrectly) {}

  if (false) {
  } else if (
      thisIsRatherALongIfClause && thatIExpectToBeBroken
      || ontoMultipleLines && whenFormattedCorrectly) {
  }
}

FWIW, +1 from me as well for a BinPack option to have each condition always on its own line when breaking is necessary.

[BraynStorm]

Bug is still present in clang-format version 15.0.1

[marco-antognini-sonarsource]

I've noticed ContinuationIndentWidth also impacts the behavior with if statement (not only else if).

For example:

> clang-format --style='{ColumnLimit: 60, AlignAfterOpenBracket: BlockIndent, ContinuationIndentWidth: 2}' tmp.cc
void foo() {
  if (thisIsRatherALongIfClause && thatIExpectToBeBroken || ontoMultipleLines && whenFormattedCorrectly) {
  }

  if (false) {
  } else if (thisIsRatherALongIfClause && thatIExpectToBeBroken || ontoMultipleLines && whenFormattedCorrectly) {
  }
}

I spent a bit of time exploring the implementation and came up with the following workaround. It's probably imperfect. Unfortunately, I don't have time to further improve it. Maybe it will trigger some ideas in the community.

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 5991cf23d5dc..97e3904156d8 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -4761,6 +4761,14 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
     return true;
   }
 
+  // We only break before r_paren if we're in a block indented context.
+  if (Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) {
+    if (Right.is(tok::r_paren))
+      return Right.MatchingParen;
+    if (Left.is(tok::l_paren))
+      return true;
+  }
+
   if (Right.is(tok::colon) &&
       !Right.isOneOf(TT_CtorInitializerColon, TT_InlineASMColon)) {
     return false;
@@ -4875,16 +4883,6 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
   if (Right.is(tok::r_brace))
     return Right.MatchingParen && Right.MatchingParen->is(BK_Block);
 
-  // We only break before r_paren if we're in a block indented context.
-  if (Right.is(tok::r_paren)) {
-    if (Style.AlignAfterOpenBracket != FormatStyle::BAS_BlockIndent ||
-        !Right.MatchingParen) {
-      return false;
-    }
-    const FormatToken *Previous = Right.MatchingParen->Previous;
-    return !(Previous && (Previous->is(tok::kw_for) || Previous->isIf()));
-  }
-
   // Allow breaking after a trailing annotation, e.g. after a method
   // declaration.
   if (Left.is(TT_TrailingAnnotation)) {

With it, the previous command produces:

void foo() {
  if (
    thisIsRatherALongIfClause && thatIExpectToBeBroken ||
    ontoMultipleLines && whenFormattedCorrectly
  ) {
  }

  if (false) {
  } else if (
    thisIsRatherALongIfClause && thatIExpectToBeBroken ||
    ontoMultipleLines && whenFormattedCorrectly
  ) {
  }
}

which is good for my purposes.

[nnethercote]

I have also seen this problem, involving ContinuationIndentWidth, with clang-format 16.0.3.

> cat b.c
void f() {
   if (   lastEvt->tag  ==  Ev_Dr &&
            lastEvt->Ev.Dr.szB==datasize &&    lastEvt->inode == inode && eqIRAtom(lastEvt.ea,ea) ) {
      return;
   }
}

> clang-format -style='{ColumnLimit: 80, AlignAfterOpenBracket: BlockIndent, IndentWidth: 4, ContinuationIndentWidth: 4}' b.c
void f() {
    if (lastEvt->tag == Ev_Dr && lastEvt->Ev.Dr.szB == datasize &&
        lastEvt->inode == inode && eqIRAtom(lastEvt.ea, ea)) {
        return;
    }
}

> clang-format -style='{ColumnLimit: 80, AlignAfterOpenBracket: BlockIndent, IndentWidth: 2, ContinuationIndentWidth: 2}' b.c
void f() {
  if (   lastEvt->tag  ==  Ev_Dr &&
            lastEvt->Ev.Dr.szB==datasize &&    lastEvt->inode == inode && eqIRAtom(lastEvt.ea,ea) ) {
    return;
  }
}

When the indent widths are 4, it works well, but when the indent widths are 2, things go badly wrong, with clang-format completely failing to format the if condition. But if I simplify the last expression in the condition a tiny bit (changing lastEvt.ea to lastEvt), I get this:

> clang-format -style='{ColumnLimit: 80, AlignAfterOpenBracket: BlockIndent, IndentWidth: 2, ContinuationIndentWidth: 2}' b.c
void f() {
  if (lastEvt->tag == Ev_Dr && lastEvt->Ev.Dr.szB == datasize && lastEvt->inode == inode && eqIRAtom(lastEvt, ea)) {
    return;
  }
}

The condition is formatted this time, but not wrapped, even though it exceeds 80 chars.

[gedare]

I have submitted a fix for this bug at https://reviews.llvm.org/D154755