Author: Owen Pan Date: 2024-07-30T10:40:15+02:00 New Revision: 392b77d58a91049a155f3390ec16941a848aa766
URL: https://github.com/llvm/llvm-project/commit/392b77d58a91049a155f3390ec16941a848aa766 DIFF: https://github.com/llvm/llvm-project/commit/392b77d58a91049a155f3390ec16941a848aa766.diff LOG: [clang-format] Fix misannotations of `<` in ternary expressions (#100980) Fixes #100300. (cherry picked from commit 73c961a3345c697f40e2148318f34f5f347701c1) Added: Modified: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/TokenAnnotatorTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index 5c11f3cb1a874..63c8699fd62d1 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -154,8 +154,8 @@ class AnnotatingParser { if (NonTemplateLess.count(CurrentToken->Previous) > 0) return false; - const FormatToken &Previous = *CurrentToken->Previous; // The '<'. - if (Previous.Previous) { + if (const auto &Previous = *CurrentToken->Previous; // The '<'. + Previous.Previous) { if (Previous.Previous->Tok.isLiteral()) return false; if (Previous.Previous->is(tok::r_brace)) @@ -175,11 +175,13 @@ class AnnotatingParser { FormatToken *Left = CurrentToken->Previous; Left->ParentBracket = Contexts.back().ContextKind; ScopedContextCreator ContextCreator(*this, tok::less, 12); - Contexts.back().IsExpression = false; + + const auto *BeforeLess = Left->Previous; + // If there's a template keyword before the opening angle bracket, this is a // template parameter, not an argument. - if (Left->Previous && Left->Previous->isNot(tok::kw_template)) + if (BeforeLess && BeforeLess->isNot(tok::kw_template)) Contexts.back().ContextType = Context::TemplateArgument; if (Style.Language == FormatStyle::LK_Java && @@ -187,19 +189,24 @@ class AnnotatingParser { next(); } - while (CurrentToken) { + for (bool SeenTernaryOperator = false; CurrentToken;) { + const bool InExpr = Contexts[Contexts.size() - 2].IsExpression; if (CurrentToken->is(tok::greater)) { + const auto *Next = CurrentToken->Next; // 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) && + if (Next && Next->is(tok::greater) && Left->ParentBracket != tok::less && CurrentToken->getStartOfNonWhitespace() == - CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset( - -1)) { + Next->getStartOfNonWhitespace().getLocWithOffset(-1)) { + return false; + } + if (InExpr && SeenTernaryOperator && + (!Next || !Next->isOneOf(tok::l_paren, tok::l_brace))) { return false; } Left->MatchingParen = CurrentToken; @@ -210,14 +217,14 @@ class AnnotatingParser { // msg: < item: data > // In TT_TextProto, map<key, value> does not occur. if (Style.Language == FormatStyle::LK_TextProto || - (Style.Language == FormatStyle::LK_Proto && Left->Previous && - Left->Previous->isOneOf(TT_SelectorName, TT_DictLiteral))) { + (Style.Language == FormatStyle::LK_Proto && BeforeLess && + BeforeLess->isOneOf(TT_SelectorName, TT_DictLiteral))) { CurrentToken->setType(TT_DictLiteral); } else { CurrentToken->setType(TT_TemplateCloser); CurrentToken->Tok.setLength(1); } - if (CurrentToken->Next && CurrentToken->Next->Tok.isLiteral()) + if (Next && Next->Tok.isLiteral()) return false; next(); return true; @@ -229,18 +236,21 @@ class AnnotatingParser { } if (CurrentToken->isOneOf(tok::r_paren, tok::r_square, tok::r_brace)) return false; + const auto &Prev = *CurrentToken->Previous; // If a && or || is found and interpreted as a binary operator, this set // of angles is likely part of something like "a < b && c > d". If the // angles are inside an expression, the ||/&& might also be a binary // operator that was misinterpreted because we are parsing template // parameters. // FIXME: This is getting out of hand, write a decent parser. - if (CurrentToken->Previous->isOneOf(tok::pipepipe, tok::ampamp) && - CurrentToken->Previous->is(TT_BinaryOperator) && - Contexts[Contexts.size() - 2].IsExpression && - !Line.startsWith(tok::kw_template)) { - return false; + if (InExpr && !Line.startsWith(tok::kw_template) && + Prev.is(TT_BinaryOperator)) { + const auto Precedence = Prev.getPrecedence(); + if (Precedence > prec::Conditional && Precedence < prec::Relational) + return false; } + if (Prev.is(TT_ConditionalExpr)) + SeenTernaryOperator = true; updateParameterCount(Left, CurrentToken); if (Style.Language == FormatStyle::LK_Proto) { if (FormatToken *Previous = CurrentToken->getPreviousNonComment()) { diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp index 51810ad047a26..386649bb6679f 100644 --- a/clang/unittests/Format/TokenAnnotatorTest.cpp +++ b/clang/unittests/Format/TokenAnnotatorTest.cpp @@ -577,12 +577,20 @@ TEST_F(TokenAnnotatorTest, UnderstandsTernaryInTemplate) { EXPECT_TOKEN(Tokens[7], tok::greater, TT_TemplateCloser); // IsExpression = true + Tokens = annotate("return foo<true ? 1 : 2>();"); ASSERT_EQ(Tokens.size(), 13u) << Tokens; EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener); EXPECT_TOKEN(Tokens[4], tok::question, TT_ConditionalExpr); EXPECT_TOKEN(Tokens[6], tok::colon, TT_ConditionalExpr); EXPECT_TOKEN(Tokens[8], tok::greater, TT_TemplateCloser); + + Tokens = annotate("return foo<true ? 1 : 2>{};"); + ASSERT_EQ(Tokens.size(), 13u) << Tokens; + EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener); + EXPECT_TOKEN(Tokens[4], tok::question, TT_ConditionalExpr); + EXPECT_TOKEN(Tokens[6], tok::colon, TT_ConditionalExpr); + EXPECT_TOKEN(Tokens[8], tok::greater, TT_TemplateCloser); } TEST_F(TokenAnnotatorTest, UnderstandsNonTemplateAngleBrackets) { @@ -596,6 +604,21 @@ TEST_F(TokenAnnotatorTest, UnderstandsNonTemplateAngleBrackets) { EXPECT_TOKEN(Tokens[1], tok::less, TT_BinaryOperator); EXPECT_TOKEN(Tokens[7], tok::greater, TT_BinaryOperator); + Tokens = annotate("return A < B ? true : A > B;"); + ASSERT_EQ(Tokens.size(), 12u) << Tokens; + EXPECT_TOKEN(Tokens[2], tok::less, TT_BinaryOperator); + EXPECT_TOKEN(Tokens[8], tok::greater, TT_BinaryOperator); + + Tokens = annotate("return A < B ? true : A > B ? false : false;"); + ASSERT_EQ(Tokens.size(), 16u) << Tokens; + EXPECT_TOKEN(Tokens[2], tok::less, TT_BinaryOperator); + EXPECT_TOKEN(Tokens[8], tok::greater, TT_BinaryOperator); + + Tokens = annotate("return A < B ^ A > B;"); + ASSERT_EQ(Tokens.size(), 10u) << Tokens; + EXPECT_TOKEN(Tokens[2], tok::less, TT_BinaryOperator); + EXPECT_TOKEN(Tokens[6], tok::greater, TT_BinaryOperator); + Tokens = annotate("ratio{-1, 2} < ratio{-1, 3} == -1 / 3 > -1 / 2;"); ASSERT_EQ(Tokens.size(), 27u) << Tokens; EXPECT_TOKEN(Tokens[7], tok::less, TT_BinaryOperator); _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
