[clang-format] Take out common code for parsing blocks NFC

[HazardyKnusperkeks]

Looks basically okay.

clang/lib/Format/UnwrappedLineParser.cpp

2841

This is completely missing. Didn't it affect anything?

clang/lib/Format/UnwrappedLineParser.h

128

I'm no fan of default arguments.
But we have them all around...

[MyDeveloperDay]

Can you add a summary as to why you are making this change? the code doesn't look the same before/after so whats changing and why? is there no unit tests that perhaps need to be added?

[sstwcw]

clang/lib/Format/UnwrappedLineParser.cpp

2841

This chunk of code is already in parseUnbracedBody.

[owenpan]

Because this patch would impact inserting/removing braces, we must test it against a large codebase. Before I landed RemoveBracesLLVM and InsertBraces, I had tested them with make check-clang and compared the result with that of running the same test without the options. I'm not sure if this patch would be worth the effort as the new function only refactored code at two places.

clang/lib/Format/UnwrappedLineParser.cpp

2716–2717

The suggested names are merely my preference.

2718–2723

2725

2736

2757

2764–2765

2854

We can't refactor here as parseSwitch() doesn't call parseUnbracedBody(), which handles option InsertBraces.

clang/lib/Format/UnwrappedLineParser.h

128

There is no need to have a default for the first parameter now. See line 2839 below.

[sstwcw]

This patch is only intended to reduce the number of times the functionality gets implemented separately. Any change in behavior would be unintended. And we also use the parseIndentedBlock in Verilog stuff, so it's not just two places. I will wait for the check-clang result to see what to do.

[MyDeveloperDay]

Just an observation, I have this in my Day Job all the time..

1. Developer A develops a piece of code
2. Over the years the developers in our group learn that code and become familiar with it
3. Developer B arrives in our group, telling us we are doing it all wrong we don't understand, but they are going to refactor it for us and correct the error of our ways.
4. Now now one in our group understands the code, except Developer B
5. Developer B gets annoyed at our lack of understanding of his/her more masterful refactoring skills and disappears...
6. No one understands the code and everyone has to relearn the code. (Developer B's name is now mud!)

Refactoring is an important step, but in my view it shouldn't be the first contribution someone makes. I understand it you think its better, it possibly is.. but those of us who have been in here for years now no longer recognize the code and have to read it with a fine tooth combe again.

I personally don't see the code as being functional equal before or after, and I've kicked myself for not being more thorough in review when someone does this..that's probably my lack of understanding.

If you are fixing a bug, then ok, lets look at it, but just because you don't like the way it looks or you think there is repetition. If your an experienced contributor then maybe you'd have our trust...but otherwise we need to express caution here in my view. Just incase you chose to up and disappear.

Refactoring yes, but if its "Drive by" Refactoring, I'm not so keen.

[sstwcw]

For the new stuff I have the option of still adding the function parseIndentedBlock but only using it in new code. Please be more blunt about whether I should close this revision and do it that way. I guess I might have misunderstood you before from how you reacted when I closed the large patch.

[MyDeveloperDay]

In D121757#3389218, @sstwcw wrote:

For the new stuff I have the option of still adding the function parseIndentedBlock but only using it in new code. Please be more blunt about whether I should close this revision and do it that way. I guess I might have misunderstood you before from how you reacted when I closed the large patch.

Ok being blunt.. the things you are asking to do, look noble, I just don't always understand the motivation? you are seemingly not fixing anything, you are not adding new tests, its just moving things around, but I don't know your commitment yet? are you hanging around? are you going to fix bugs?

I feel like the guidance for new developers is do a "good first issue", the reviewers have triaged the clang-format issues to try and identify what makes a good first issue for a new contributor

https://github.com/llvm/llvm-project/issues?q=is%3Aopen+label%3A%22good+first+issue%22+label%3Aclang-format

Ultimately if you don't have commit access your going to ask one of us to land this for you, as such we might need persuading, Certainly for me to do that for you I need to see a benefit otherwise its just change in my view.

[owenpan]

In D121757#3389092, @sstwcw wrote:

This patch is only intended to reduce the number of times the functionality gets implemented separately. Any change in behavior would be unintended. And we also use the parseIndentedBlock in Verilog stuff, so it's not just two places. I will wait for the check-clang result to see what to do.

In that case, I can accept this patch if the review comments are addressed. We can add inserting braces to switch in a separate patch. Not sure what name we should use for the function though as it would not be parsing the body of loop statements anymore.

[sstwcw]

I tried formatting the files in clang-formatted-files.txt. Besides the files in the list that get changed when formatted with the program built from main, none gets changed when I format them with the program built from this patch, whether or not parseSwitch is modified.

[owenpan]

clang/lib/Format/UnwrappedLineParser.cpp

2721–2722

You swapped these two lines. We want to remember the l_brace before calling other functions in case they update FormatTok.

2757–2758

Or just the following as they are not default arguments now:

parseLoopBody(Style.RemoveBracesLLVM, true);

2764

Or just the following:

parseLoopBody(false, Style.BraceWrapping.BeforeWhile);

[owenpan]

In D121757#3406814, @sstwcw wrote:

I tried formatting the files in clang-formatted-files.txt. Besides the files in the list that get changed when formatted with the program built from main, none gets changed when I format them with the program built from this patch, whether or not parseSwitch is modified.

To test a token-changing patch (e.g. inserting/removing braces) on a large codebase like clang, I would go through the following steps:

1. Build a debug version of clang-format from scratch without applying the patch.
2. Run the built clang-format on every .(c|cpp|h) file under clang/.
3. Build clang.
4. Run check-clang and save the output in a log file.
5. Run git reset --hard and apply the patch.
6. Build clang-format again.
7. Add InsertBraces: true to clang/.clang-format.
8. Repeat steps 2-4 above.
9. Compare the new log file with the pre-patch one. They should have exactly the same Unresolved/Failed tests. They should also have the same numbers of Skipped/Unsupported/Passed/Expectedly Failed tests.

[owenpan]

In D121757#3407539, @owenpan wrote:

In D121757#3406814, @sstwcw wrote:

I tried formatting the files in clang-formatted-files.txt. Besides the files in the list that get changed when formatted with the program built from main, none gets changed when I format them with the program built from this patch, whether or not parseSwitch is modified.

To test a token-changing patch (e.g. inserting/removing braces) on a large codebase like clang, I would go through the following steps:

1. Build a debug version of clang-format from scratch without applying the patch.
2. Run the built clang-format on every .(c|cpp|h) file under clang/.
3. Build clang.
4. Run check-clang and save the output in a log file.
5. Run git reset --hard and apply the patch.
6. Build clang-format again.

And then run git reset --hard again before step 7 below.

7. Add InsertBraces: true to clang/.clang-format.
8. Repeat steps 2-4 above.
9. Compare the new log file with the pre-patch one. They should have exactly the same Unresolved/Failed tests. They should also have the same numbers of Skipped/Unsupported/Passed/Expectedly Failed tests.

[sstwcw]

I ran check-clang after formatting the entire code base with the new version. It turned out it did break some tests. It seems to be because it messed up these comments.

diff --git a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.fallthrough/p1.cpp b/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.fallthrough/p1.cpp
index 7a8d756b09fd..53b2ddd12885 100644
--- a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.fallthrough/p1.cpp
+++ b/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.fallthrough/p1.cpp
@@ -33,8 +33,8 @@ void f(int n) {
   case 6:
     do {
       [[fallthrough]];
-    } while (
-        false); // expected-error {{does not directly precede switch label}}
+    } while (false); // expected-error {{does not directly precede switch
+                     // label}}
   case 7:
     switch (n) {
     case 0:

Now I have undone the parts that changed this. Formatting with the new version yields the same result as formatting with the program built from the main branch, whether with or without the InsertBraces option, except for this one line:

diff --git a/clang/test/SemaCXX/constant-expression-cxx11.cpp b/clang/test/SemaCXX/constant-expression-cxx11.cpp
index 4c6b6da9e415..4d063378f37c 100644
--- a/clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -1115,7 +1115,7 @@ static_assert(check(S(5), 11), "");
 namespace PR14171 {
 
 struct X {
-  constexpr(operator int)() const { return 0; }
+  constexpr (operator int)() const { return 0; }
 };
 static_assert(X() == 0, "");

[owenpan]

In D121757#3415402, @sstwcw wrote:

I ran check-clang after formatting the entire code base with the new version. It turned out it did break some tests. It seems to be because it messed up these comments.

diff --git a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.fallthrough/p1.cpp b/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.fallthrough/p1.cpp
index 7a8d756b09fd..53b2ddd12885 100644
--- a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.fallthrough/p1.cpp
+++ b/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.fallthrough/p1.cpp
@@ -33,8 +33,8 @@ void f(int n) {
   case 6:
     do {
       [[fallthrough]];
-    } while (
-        false); // expected-error {{does not directly precede switch label}}
+    } while (false); // expected-error {{does not directly precede switch
+                     // label}}
   case 7:
     switch (n) {
     case 0:

Now I have undone the parts that changed this. Formatting with the new version yields the same result as formatting with the program built from the main branch, whether with or without the InsertBraces option, except for this one line:

diff --git a/clang/test/SemaCXX/constant-expression-cxx11.cpp b/clang/test/SemaCXX/constant-expression-cxx11.cpp
index 4c6b6da9e415..4d063378f37c 100644
--- a/clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -1115,7 +1115,7 @@ static_assert(check(S(5), 11), "");
 namespace PR14171 {
 
 struct X {
-  constexpr(operator int)() const { return 0; }
+  constexpr (operator int)() const { return 0; }
 };
 static_assert(X() == 0, "");

Thanks for running check-clang. Can you address the review comments so that we can accept this patch? After you land it, I will add handling switch to InsertBraces and run check-clang again.

[owenpan]

LGTM

[sstwcw]

Would you have a look at the parent revision namely D121756?

clang/lib/Format/FormatToken.h

#define LIST_TOKEN_TYPES                                                       \
   TYPE(AttributeSquare)                                                        \
   TYPE(BinaryOperator)                                                         \
   TYPE(BitFieldColon)                                                          \
   TYPE(BlockComment)                                                           \
   TYPE(BracedListLBrace)                                                       \
   TYPE(CastRParen)                                                             \
   TYPE(ClassLBrace)                                                            \
   TYPE(CompoundRequirementLBrace)                                              \
-  TYPE(ConditionalExpr)                                                        \
+  TYPE(ConditionLParen) /* The condition in an if, while, or switch statement. \
+                           The parenthesis in a for statement is not labeled   \
+                           this type, because inside is not simply an          \
+                           expression and thus special treatment is needed. */ \
+  TYPE(ConditionalExpr) /* ternary ?: expression */                            \
   TYPE(ConflictAlternative)                                                    \
   TYPE(ConflictEnd)                                                            \
   TYPE(ConflictStart)                                                          \
   TYPE(CppCastLParen)                                                          \
   TYPE(CtorInitializerColon)                                                   \
   TYPE(CtorInitializerComma)                                                   \
   TYPE(DesignatedInitializerLSquare)                                           \
   TYPE(DesignatedInitializerPeriod)                                            \
public:
   }
   template <typename T> bool isNot(T Kind) const { return !is(Kind); }
 
   bool isIf(bool AllowConstexprMacro = true) const {
     return is(tok::kw_if) || endsSequence(tok::kw_constexpr, tok::kw_if) ||
            (endsSequence(tok::identifier, tok::kw_if) && AllowConstexprMacro);
   }
 
+  /// Returns \c true if the token is the open parenthesis of a
+  /// statement's condition like if or while.
+  /// @param IncludeFor - Whether to return true if the parenthesis is in some
+  /// special construct including for, catch, and foreach. Because inside the
+  /// parentheses of these constructs is not simply an expression, they need to
+  /// be handled separately.
+  bool isConditionLParen(bool IncludeFor) const {
+    if (isNot(tok::l_paren))
+      return false;
+    if (is(TT_ConditionLParen))
+      return true;
+    const FormatToken *Prev = getPreviousNonComment();
+    if (!Prev)
+      return false;
+    if (Prev->isOneOf(tok::kw_if, tok::kw_while, tok::kw_switch, tok::kw_case,
+                      tok::kw_constexpr))
+      return true;
+    // `for` and `catch` special handling.
+    if (IncludeFor && Prev->isOneOf(TT_ForEachMacro, TT_ObjCForIn, tok::kw_for,
+                                    tok::kw_catch))
+      return true;
+    return false;
+  }
+
   bool closesScopeAfterBlock() const {
     if (getBlockKind() == BK_Block)
       return true;
     if (closesScope())
       return Previous->closesScopeAfterBlock();
     return false;
   }
 

clang/lib/Format/TokenAnnotator.cpp

if (Left->Previous && Left->Previous->is(tok::greater) &&
     Left = Left->Previous->MatchingParen;
 
   // Check for `[...]`.
   return Left->Previous && Left->Previous->is(tok::r_square) &&
          Left->Previous->MatchingParen &&
          Left->Previous->MatchingParen->is(TT_LambdaLSquare);
 }
 
-/// Returns \c true if the token is followed by a boolean condition, \c false
-/// otherwise.
-static bool isKeywordWithCondition(const FormatToken &Tok) {
-  return Tok.isOneOf(tok::kw_if, tok::kw_for, tok::kw_while, tok::kw_switch,
-                     tok::kw_constexpr, tok::kw_catch);
-}
-
 /// A parser that gathers additional information about tokens.
 ///
 /// The \c TokenAnnotator tries to match parenthesis and square brakets and
 /// store a parenthesis levels. It also tries to resolve matching "<" and ">"
 /// into template parameter lists.
 class AnnotatingParser {
 public:
   AnnotatingParser(const FormatStyle &Style, AnnotatedLine &Line,
while (CurrentToken) {
         // Try to do a better job at looking for ">>" within the condition of
         // a statement. Conservatively insert spaces between consecutive ">"
         // tokens to prevent splitting right bitshift operators and potentially
         // altering program semantics. This check is overly conservative and
         // will prevent spaces from being inserted in select nested template
         // parameter cases, but should not alter program semantics.
         if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
             Left->ParentBracket != tok::less &&
-            (isKeywordWithCondition(*Line.First) ||
-             CurrentToken->getStartOfNonWhitespace() ==
-                 CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
-                     -1)))
+                    -1))
           return false;
         Left->MatchingParen = CurrentToken;
         CurrentToken->MatchingParen = Left;
         // In TT_Proto, we must distignuish between:
         //   map<key, value>
         //   msg < item: data >
         //   msg: < item: data >
         // In TT_TextProto, map<key, value> does not occur.
if (OpeningParen.is(TT_OverloadedOperatorLParen)) {
       Contexts.back().IsExpression = OperatorCalledAsMemberFunction;
     } else if (Style.isJavaScript() &&
                (Line.startsWith(Keywords.kw_type, tok::identifier) ||
                 Line.startsWith(tok::kw_export, Keywords.kw_type,
                                 tok::identifier))) {
       // type X = (...);
       // export type X = (...);
       Contexts.back().IsExpression = false;
-    } else if (OpeningParen.Previous &&
-               (OpeningParen.Previous->isOneOf(tok::kw_static_assert,
-                                               tok::kw_while, tok::l_paren,
-                                               tok::comma) ||
-                OpeningParen.Previous->isIf() ||
-                OpeningParen.Previous->is(TT_BinaryOperator))) {
+    } else if (OpeningParen.isConditionLParen(/*IncludeFor=*/false) ||
+               (OpeningParen.Previous &&
+                OpeningParen.Previous->isOneOf(TT_BinaryOperator, tok::l_paren,
+                                               tok::comma,
+                                               tok::kw_static_assert))) {
       // static_assert, if and while usually contain expressions.
       Contexts.back().IsExpression = true;
     } else if (Style.isJavaScript() && OpeningParen.Previous &&
                (OpeningParen.Previous->is(Keywords.kw_function) ||
                 (OpeningParen.Previous->endsSequence(tok::identifier,
                                                      Keywords.kw_function)))) {
       // function(...) or function f(...)
       Contexts.back().IsExpression = false;
bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
   if (Right.is(tok::hashhash))
     return Left.is(tok::hash);
   if (Left.isOneOf(tok::hashhash, tok::hash))
     return Right.is(tok::hash);
   if ((Left.is(tok::l_paren) && Right.is(tok::r_paren)) ||
       (Left.is(tok::l_brace) && Left.isNot(BK_Block) &&
        Right.is(tok::r_brace) && Right.isNot(BK_Block)))
     return Style.SpaceInEmptyParentheses;
-  if (Style.SpacesInConditionalStatement) {
-    if (Left.is(tok::l_paren) && Left.Previous &&
-        isKeywordWithCondition(*Left.Previous))
-      return true;
-    if (Right.is(tok::r_paren) && Right.MatchingParen &&
-        Right.MatchingParen->Previous &&
-        isKeywordWithCondition(*Right.MatchingParen->Previous))
-      return true;
-  }
+  if (Style.SpacesInConditionalStatement &&
+      (Left.isConditionLParen(/*IncludeFor=*/true) ||
+       (Right.is(tok::r_paren) && Right.MatchingParen &&
+        Right.MatchingParen->isConditionLParen(/*IncludeFor=*/true))))
 
   // auto{x} auto(x)
   if (Left.is(tok::kw_auto) && Right.isOneOf(tok::l_paren, tok::l_brace))
     return false;
 
   // operator co_await(x)
   if (Right.is(tok::l_paren) && Left.is(tok::kw_co_await) && Left.Previous &&
       Left.Previous->is(tok::kw_operator))
if (Left.is(TT_ForEachMacro))
              spaceRequiredBeforeParens(Right);
     if (Left.is(TT_IfMacro))
       return Style.SpaceBeforeParensOptions.AfterIfMacros ||
              spaceRequiredBeforeParens(Right);
     if (Line.Type == LT_ObjCDecl)
       return true;
     if (Left.is(tok::semi))
       return true;
-    if (Left.isOneOf(tok::pp_elif, tok::kw_for, tok::kw_while, tok::kw_switch,
-                     tok::kw_case, TT_ForEachMacro, TT_ObjCForIn))
-      return Style.SpaceBeforeParensOptions.AfterControlStatements ||
-             spaceRequiredBeforeParens(Right);
-    if (Left.isIf(Line.Type != LT_PreprocessorDirective))
+    if (Right.isConditionLParen(/*IncludeFor=*/true) || Left.is(tok::pp_elif))
       return Style.SpaceBeforeParensOptions.AfterControlStatements ||
              spaceRequiredBeforeParens(Right);
 
     // TODO add Operator overloading specific Options to
     // SpaceBeforeParensOptions
     if (Right.is(TT_OverloadedOperatorLParen))
       return spaceRequiredBeforeParens(Right);
     // Function declaration or definition
bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
   // the l_brace, which is tracked by BreakBeforeClosingBrace.
   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)
       return Right.MatchingParen &&
-             !(Right.MatchingParen->Previous &&
-               (Right.MatchingParen->Previous->is(tok::kw_for) ||
-                Right.MatchingParen->Previous->isIf()));
-
+             !Right.MatchingParen->isConditionLParen(/*IncludeFor=*/true);
     return false;
   }
 
   // Allow breaking after a trailing annotation, e.g. after a method
   // declaration.
   if (Left.is(TT_TrailingAnnotation))
     return !Right.isOneOf(tok::l_brace, tok::semi, tok::equal, tok::l_paren,
                           tok::less, tok::coloncolon);

clang/lib/Format/UnwrappedLineParser.h

private:
   void parseParens(TokenType AmpAmpTokenType = TT_Unknown);
   void parseSquare(bool LambdaIntroducer = false);
   void keepAncestorBraces();
   void parseUnbracedBody(bool CheckEOF = false);
   void handleAttributes();
   bool handleCppAttributes();
   FormatToken *parseIfThenElse(IfStmtKind *IfKind, bool KeepBraces = false);
   void parseTryCatch();
+  void parseLoopBody(bool TryRemoveBraces, bool WrapRightBrace);
   void parseForOrWhileLoop();
   void parseDoWhile();
   void parseLabel(bool LeftAlignLabel = false);
   void parseCaseLabel();
   void parseSwitch();
   void parseNamespace();
   void parseModuleImport();
   void parseNew();

clang/lib/Format/UnwrappedLineParser.cpp

FormatToken *UnwrappedLineParser::parseIfThenElse(IfStmtKind *IfKind,
   nextToken();
   if (FormatTok->is(tok::exclaim))
     nextToken();
   if (FormatTok->is(tok::kw_consteval)) {
     nextToken();
   } else {
     if (FormatTok->isOneOf(tok::kw_constexpr, tok::identifier))
       nextToken();
-    if (FormatTok->is(tok::l_paren))
+    if (FormatTok->is(tok::l_paren)) {
+      FormatTok->setFinalizedType(TT_ConditionLParen);
       parseParens();
     }
+  }
   handleAttributes();
 
   bool NeedsUnwrappedLine = false;
   keepAncestorBraces();
 
   FormatToken *IfLeftBrace = nullptr;
   IfStmtKind IfBlockKind = IfStmtKind::NotIf;
 
if (FormatTok->is(tok::l_paren)) {
       if (FormatTok->is(tok::l_brace))
         parseChildBlock();
       return;
     }
     nextToken();
   } while (!eof());
 }
 
-void UnwrappedLineParser::parseForOrWhileLoop() {
-  assert(FormatTok->isOneOf(tok::kw_for, tok::kw_while, TT_ForEachMacro) &&
-         "'for', 'while' or foreach macro expected");
-  nextToken();
-  // JS' for await ( ...
-  if (Style.isJavaScript() && FormatTok->is(Keywords.kw_await))
-    nextToken();
-  if (Style.isCpp() && FormatTok->is(tok::kw_co_await))
-    nextToken();
-  if (FormatTok->is(tok::l_paren))
-    parseParens();
-
+void UnwrappedLineParser::parseLoopBody(bool TryRemoveBraces,
+                                        bool WrapRightBrace) {
   keepAncestorBraces();
 
   if (FormatTok->is(tok::l_brace)) {
     FormatToken *LeftBrace = FormatTok;
     CompoundStatementIndenter Indenter(this, Style, Line->Level);
     parseBlock();
-    if (Style.RemoveBracesLLVM) {
+    if (TryRemoveBraces) {
       assert(!NestedTooDeep.empty());
       if (!NestedTooDeep.back())
         markOptionalBraces(LeftBrace);
     }
+    if (WrapRightBrace)
       addUnwrappedLine();
   } else {
     parseUnbracedBody();
   }
 
-  if (Style.RemoveBracesLLVM)
+  if (TryRemoveBraces)
     NestedTooDeep.pop_back();
 }
 
-void UnwrappedLineParser::parseDoWhile() {
-  assert(FormatTok->is(tok::kw_do) && "'do' expected");
+void UnwrappedLineParser::parseForOrWhileLoop() {
+  assert(FormatTok->isOneOf(tok::kw_for, tok::kw_while, TT_ForEachMacro) &&
+         "'for', 'while' or foreach macro expected");
+  const FormatToken &FirstTok = *FormatTok;
   nextToken();
+  // JS' for await ( ...
+  if (Style.isJavaScript() && FormatTok->is(Keywords.kw_await))
+    nextToken();
+  if (Style.isCpp() && FormatTok->is(tok::kw_co_await))
+    nextToken();
+  if (FormatTok->is(tok::l_paren)) {
+    // Those that begin with a for require special treatment because inside the
+    // parentheses is not an expression.
+    if (FirstTok.is(tok::kw_while))
+      FormatTok->setFinalizedType(TT_ConditionLParen);
+    parseParens();
+  }
 
-  keepAncestorBraces();
-
-  if (FormatTok->is(tok::l_brace)) {
-    CompoundStatementIndenter Indenter(this, Style, Line->Level);
-    parseBlock();
-    if (Style.BraceWrapping.BeforeWhile)
-      addUnwrappedLine();
-  } else {
-    parseUnbracedBody();
-  }
+  parseLoopBody(Style.RemoveBracesLLVM, true);
 
-  if (Style.RemoveBracesLLVM)
-    NestedTooDeep.pop_back();
+void UnwrappedLineParser::parseDoWhile() {
+  assert(FormatTok->is(tok::kw_do) && "'do' expected");
+  nextToken();
+
+  parseLoopBody(false, Style.BraceWrapping.BeforeWhile);
 
   // FIXME: Add error handling.
   if (!FormatTok->is(tok::kw_while)) {
     addUnwrappedLine();
     return;
   }
 
   // If in Whitesmiths mode, the line with the while() needs to be indented
   // to the same level as the block.
   if (Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths)
     ++Line->Level;
 
   nextToken();
+  // FIXME: Add error handling.
+  if (FormatTok->is(tok::l_paren))
+    FormatTok->setFinalizedType(TT_ConditionLParen);
   parseStructuralElement();
 }
 
 void UnwrappedLineParser::parseLabel(bool LeftAlignLabel) {
   nextToken();
   unsigned OldLineLevel = Line->Level;
   if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))
     --Line->Level;
do {
     nextToken();
   } while (!eof() && !FormatTok->is(tok::colon));
   parseLabel();
 }
 
 void UnwrappedLineParser::parseSwitch() {
   assert(FormatTok->is(tok::kw_switch) && "'switch' expected");
   nextToken();
-  if (FormatTok->is(tok::l_paren))
+  if (FormatTok->is(tok::l_paren)) {
+    FormatTok->setFinalizedType(TT_ConditionLParen);
     parseParens();
+  }
 
   keepAncestorBraces();
 
   if (FormatTok->is(tok::l_brace)) {
     CompoundStatementIndenter Indenter(this, Style, Line->Level);
     parseBlock();
     addUnwrappedLine();
   } else {
     addUnwrappedLine();
     ++Line->Level;
     parseStructuralElement();
     --Line->Level;
   }
 
   if (Style.RemoveBracesLLVM)
     NestedTooDeep.pop_back();
 }
 
 // Operators that can follow a C variable.
 static bool isCOperatorFollowingVar(tok::TokenKind kind) {
   switch (kind) {
   case tok::ampamp:
   case tok::ampequal:
   case tok::arrow:

clang/unittests/Format/FormatTest.cpp

EXPECT_EQ("try {\n"
             "  foo();\n"
             "} catch (Exception &bar) {\n"
             "  baz();\n"
             "}",
             format("try{foo();}catch(Exception&bar){baz();}", Style));
   Style.ColumnLimit =
       20; // to concentrate at brace wrapping, not line wrap due to column limit
 
+  verifyFormat("if (xxxxxxxxxx\n"
+               "        .xxxxxx)\n"
+               "  continue;",
+               Style);
+  verifyFormat("while (xxxxxxxxxx\n"
+               "           .xxxxxx)\n"
+               "  continue;",
+               Style);
+
   Style.BraceWrapping.BeforeElse = true;
   EXPECT_EQ(
       "if (foo) {\n"
       "  bar();\n"
       "}\n"
       "else if (baz ||\n"
       "         quux)\n"
       "{\n"

clang/unittests/Format/TokenAnnotatorTest.cpp

TEST_F(TokenAnnotatorTest, UnderstandsObjCBlock) {
 
   Tokens = annotate("int *p = ^int*(){ //\n"
                     "  return nullptr;\n"
                     "}();");
   ASSERT_EQ(Tokens.size(), 19u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::l_brace, TT_ObjCBlockLBrace);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsConditionParen) {
+  auto Tokens = annotate("if (true) {\n}");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_ConditionLParen);
+  Tokens = annotate("if constexpr (true) {\n}");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_ConditionLParen);
+  Tokens = annotate("if CONSTEXPR (true) {\n}");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_ConditionLParen);
+
+  // The parentheses following for is not TT_ConditionLParen, because inside is
+  // not just a condition.
+  Tokens = annotate("for (;;) {\n}");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_Unknown);
+  Tokens = annotate("foreach (Item *item, itemlist) {\n}");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_Unknown);
+  Tokens = annotate("Q_FOREACH (Item *item, itemlist) {\n}");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_Unknown);
+  Tokens = annotate("BOOST_FOREACH (Item *item, itemlist) {\n}");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_Unknown);
+  Tokens = annotate("try {\n"
+                    "} catch (Exception &bar) {\n"
+                    "}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_Unknown);
+
+  Tokens = annotate("while (true) {\n}");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_ConditionLParen);
+  Tokens = annotate("do {\n} while (true);");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_ConditionLParen);
+
+  Tokens = annotate("switch (true) {\n}");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_ConditionLParen);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang