[llvm-branch-commits] [clang] 392b77d - [clang-format] Fix misannotations of `<` in ternary expressions (#100980)
Tobias Hieta via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Tue Jul 30 01:40:40 PDT 2024
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);
More information about the llvm-branch-commits
mailing list