sstwcw updated this revision to Diff 420031.
sstwcw retitled this revision from "[clang-format] Clean up code looking for if
statements NFC" to "[clang-format] Clean up code looking for if statements".
sstwcw edited the summary of this revision.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121756/new/
https://reviews.llvm.org/D121756
Files:
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
@@ -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
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ 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"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ 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();
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 &&
@@ -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;
}
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) \
@@ -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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
