[PATCH] D46519: [clang-format] Respect BreakBeforeClosingBrace while calculating length

[Daniel Jasper via Phabricator via cfe-commits]

djasper added inline comments.


================
Comment at: lib/Format/ContinuationIndenter.cpp:44
+  int MatchingStackIndex = Stack.size() - 1;
+  while (MatchingStackIndex >= 0 && Stack[MatchingStackIndex].Tok != &Tok)
+    --MatchingStackIndex;
----------------
I think this needs a long explanatory comment, possibly with an example of the problem it is actually solving.

Also, I am somewhat skeptical as we are using this logic for all paren kinds, not just braces. That seems to be unnecessary work and also might be unexpected at some point (although I cannot come up with a test case where that would be wrong).


================
Comment at: lib/Format/ContinuationIndenter.cpp:49
+      break;
+    while (MatchingStackIndex >= 0 &&
+           Stack[MatchingStackIndex].Tok != End->Next->MatchingParen)
----------------
Maybe pull out a lambda:

  auto FindParenState = [&](const FormatToken *Tok) {
    while (MatchingStackIndex >= 0 && Stack[MatchingStackIndex].Tok != &Tok)
      --MatchingStackIndex;
    return MatchingStackIndex >= 0 ? &Stack[MatchingStackIndex] : nullptr;
  };

Then you could write the rest as:

  ...
  FindParenState(&Tok);
  const auto* End = Tok.MatchingParen
  for (; End->Next; End = End->Next) {
    if (End->Next->CanBreakBefore || !End->Next->closesScope())
      break;
    auto ParenState = FindParenState(End->Next->MatchingParen);
    if (ParenState && ParenState.BreakBeforeClosingBrace)
      break;
  }
  return End->TotalLength - Tok.TotalLength + 1;


(not entirely sure it's better)


================
Comment at: lib/Format/ContinuationIndenter.h:215
+  /// \brief The token opening this parenthesis level, or nullptr if this level
+  /// is opened by fake parenthesis.
+  const FormatToken *Tok;
----------------
The comment should include that this is not considered for memoization as the same state will always be represented by the same token (similar to other below). I wonder whether we should actually move the fields that don't affect memoization out to a different structure at some point.


Repository:
  rC Clang

https://reviews.llvm.org/D46519