sstwcw updated this revision to Diff 418073. sstwcw marked an inline comment as done.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121756/new/ https://reviews.llvm.org/D121756 Files: clang/lib/Format/ContinuationIndenter.cpp clang/lib/Format/FormatToken.h clang/lib/Format/TokenAnnotator.cpp clang/lib/Format/UnwrappedLineParser.cpp clang/unittests/Format/FormatTest.cpp clang/unittests/Format/TokenAnnotatorTest.cpp
Index: clang/unittests/Format/TokenAnnotatorTest.cpp =================================================================== --- clang/unittests/Format/TokenAnnotatorTest.cpp +++ 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 Index: clang/unittests/Format/FormatTest.cpp =================================================================== --- clang/unittests/Format/FormatTest.cpp +++ clang/unittests/Format/FormatTest.cpp @@ -3131,6 +3131,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" Index: clang/lib/Format/UnwrappedLineParser.cpp =================================================================== --- clang/lib/Format/UnwrappedLineParser.cpp +++ clang/lib/Format/UnwrappedLineParser.cpp @@ -2408,8 +2408,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(); @@ -2700,14 +2702,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(); @@ -2759,6 +2767,9 @@ ++Line->Level; nextToken(); + // FIXME: Add error handling. + if (FormatTok->is(tok::l_paren)) + FormatTok->setFinalizedType(TT_ConditionLParen); parseStructuralElement(); } @@ -2813,8 +2824,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(); Index: clang/lib/Format/TokenAnnotator.cpp =================================================================== --- clang/lib/Format/TokenAnnotator.cpp +++ 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 && @@ -3019,8 +3010,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(/*IncludeFor=*/true)) return 1000; if (Left.is(tok::equal) && InFunctionDecl) return 110; @@ -3114,15 +3104,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)) @@ -3376,11 +3362,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); @@ -4535,10 +4517,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; } Index: clang/lib/Format/FormatToken.h =================================================================== --- clang/lib/Format/FormatToken.h +++ 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,28 @@ } 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 { Index: clang/lib/Format/ContinuationIndenter.cpp =================================================================== --- clang/lib/Format/ContinuationIndenter.cpp +++ 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(/*IncludeFor=*/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;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits