diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -745,9 +745,8 @@ } State.Column += Spaces; - if (Current.isNot(tok::comment) && Previous.is(tok::l_paren) && - Previous.Previous && - (Previous.Previous->is(tok::kw_for) || Previous.Previous->isIf())) { + if (Current.isNot(tok::comment) && + Previous.isConditionLParen(/*IncludeSpecial=*/true)) { // Treat the condition inside an if as if it was a second function // parameter, i.e. let nested calls have a continuation indent. CurrentState.LastSpace = State.Column; 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) \ @@ -515,9 +519,24 @@ } template 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 IncludeSpecial - 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 IncludeSpecial) const { + if (!is(tok::l_paren)) + return false; + if (is(TT_ConditionLParen)) + return true; + const FormatToken *Prev = getPreviousNonComment(); + // `for` and `catch` special handling. + return Prev && + ((IncludeSpecial && Prev->isOneOf(TT_ForEachMacro, TT_ObjCForIn, + tok::kw_for, tok::kw_catch)) || + Prev->isOneOf(tok::kw_if, tok::kw_while, tok::kw_switch, + tok::kw_case, tok::kw_constexpr)); } bool closesScopeAfterBlock() const { 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(/*IncludeSpecial=*/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 && @@ -2994,8 +2985,7 @@ if (Left.is(tok::l_paren) && InFunctionDecl && Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign) return 100; - if (Left.is(tok::l_paren) && Left.Previous && - (Left.Previous->is(tok::kw_for) || Left.Previous->isIf())) + if (Left.isConditionLParen(/*IncludeSpecial=*/true)) return 1000; if (Left.is(tok::equal) && InFunctionDecl) return 110; @@ -3489,15 +3479,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(/*IncludeSpecial=*/true) || + (Right.is(tok::r_paren) && Right.MatchingParen && + Right.MatchingParen->isConditionLParen(/*IncludeSpecial=*/true)))) + return true; // auto{x} auto(x) if (Left.is(tok::kw_auto) && Right.isOneOf(tok::l_paren, tok::l_brace)) @@ -3751,11 +3737,8 @@ 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(/*IncludeSpecial=*/true) || + Left.is(tok::pp_elif)) return Style.SpaceBeforeParensOptions.AfterControlStatements || spaceRequiredBeforeParens(Right); @@ -4502,13 +4485,9 @@ // 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) { + 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(/*IncludeSpecial=*/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->Tok.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"); + // Those that begin with a for require special treatment because inside the + // parentheses is not an expression. + bool IsFor = FormatTok->is(tok::kw_for) || FormatTok->is(TT_ForEachMacro); 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)) { + if (!IsFor) + 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/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp --- a/clang/unittests/Format/TokenAnnotatorTest.cpp +++ b/clang/unittests/Format/TokenAnnotatorTest.cpp @@ -660,6 +660,49 @@ 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