diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h --- a/clang/lib/Format/FormatToken.h +++ b/clang/lib/Format/FormatToken.h @@ -39,7 +39,11 @@ 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) \ @@ -520,6 +524,30 @@ (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; diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -63,13 +63,6 @@ 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 @@ -130,10 +123,9 @@ // 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))) + CurrentToken->getStartOfNonWhitespace() == + CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset( + -1)) return false; Left->MatchingParen = CurrentToken; CurrentToken->MatchingParen = Left; @@ -257,12 +249,11 @@ // 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 && @@ -3118,15 +3109,11 @@ (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)))) + return true; // auto{x} auto(x) if (Left.is(tok::kw_auto) && Right.isOneOf(tok::l_paren, tok::l_brace)) @@ -3380,11 +3367,7 @@ 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); @@ -4539,10 +4522,7 @@ 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; } diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -2422,8 +2422,10 @@ } 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(); @@ -2714,14 +2716,20 @@ 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)) + 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(); @@ -2773,6 +2781,9 @@ ++Line->Level; nextToken(); + // FIXME: Add error handling. + if (FormatTok->is(tok::l_paren)) + FormatTok->setFinalizedType(TT_ConditionLParen); parseStructuralElement(); } @@ -2827,8 +2838,10 @@ 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(); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -2628,6 +2628,10 @@ "[[likely]] case 2:\n" " return;\n" "}"); + verifyFormat("switch (x * x) { //\n" + "}"); + verifyFormat("switch (x & x) { //\n" + "}"); FormatStyle Attributes = getLLVMStyle(); Attributes.AttributeMacros.push_back("LIKELY"); Attributes.AttributeMacros.push_back("OTHER_LIKELY"); @@ -3177,6 +3181,15 @@ 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" diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp --- a/clang/unittests/Format/TokenAnnotatorTest.cpp +++ b/clang/unittests/Format/TokenAnnotatorTest.cpp @@ -646,6 +646,55 @@ 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); + Tokens = annotate("switch (x * x) {\n}"); + ASSERT_EQ(Tokens.size(), 9u) << Tokens; + EXPECT_TOKEN(Tokens[3], tok::star, TT_BinaryOperator); + Tokens = annotate("switch (x & x) {\n}"); + ASSERT_EQ(Tokens.size(), 9u) << Tokens; + EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator); +} + } // namespace } // namespace format } // namespace clang