Skip to content

Commit

Permalink
clang-format: Support if CONSTEXPR if CONSTEXPR is a macro.
Browse files Browse the repository at this point in the history 
This is like r305666 (which added support for `if constexpr`) except
that it allows a macro name after the if.

This is slightly tricky for two reasons:

1. r305666 didn't add test coverage for all cases where it added a
   kw_constexpr, so I had to figure out what all the added cases were
   for. I now added tests for all `if constexpr` bits that didn't have
   tests. (This took a while, see e.g. https://reviews.llvm.org/D65223)

2. Parsing `if <ident> (` as an if means that `#if defined(` and
   `#if __has_include(` parse as ifs too. Add some special-case code
   to prevent this from happening where it's incorrect.

Fixes PR39248.

Differential Revision: https://reviews.llvm.org/D65227

llvm-svn: 367167
  • Loading branch information
@nico
nico committed Jul 27, 2019
1 parent 92a2e1b commit 1361a4c
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 10 deletions.
3 changes: 1 addition & 2 deletions clang/lib/Format/ContinuationIndenter.cpp
View file
Expand Up @@ -676,8 +676,7 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
State.Column += Spaces;
if (Current.isNot(tok::comment) && Previous.is(tok::l_paren) &&
Previous.Previous &&
(Previous.Previous->isOneOf(tok::kw_if, tok::kw_for) ||
Previous.Previous->endsSequence(tok::kw_constexpr, tok::kw_if))) {
(Previous.Previous->is(tok::kw_for) || Previous.Previous->isIf())) {
// Treat the condition inside an if as if it was a second function
// parameter, i.e. let nested calls have a continuation indent.
State.Stack.back().LastSpace = State.Column;
Expand Down
9 changes: 9 additions & 0 deletions clang/lib/Format/FormatToken.h
View file
Expand Up @@ -327,6 +327,11 @@ struct FormatToken {
}
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);
}

bool closesScopeAfterBlock() const {
if (BlockKind == BK_Block)
return true;
Expand All @@ -344,6 +349,10 @@ struct FormatToken {

/// \c true if this token ends a sequence with the given tokens in order,
/// following the ``Previous`` pointers, ignoring comments.
/// For example, given tokens [T1, T2, T3], the function returns true if
/// 3 tokens ending at this (ignoring comments) are [T3, T2, T1]. In other
/// words, the tokens passed to this function need to the reverse of the
/// order the tokens appear in code.
template <typename A, typename... Ts>
bool endsSequence(A K1, Ts... Tokens) const {
return endsSequenceInternal(K1, Tokens...);
Expand Down
15 changes: 8 additions & 7 deletions clang/lib/Format/TokenAnnotator.cpp
View file
Expand Up @@ -175,9 +175,9 @@ class AnnotatingParser {
Contexts.back().IsExpression = false;
} else if (Left->Previous &&
(Left->Previous->isOneOf(tok::kw_static_assert, tok::kw_decltype,
tok::kw_if, tok::kw_while, tok::l_paren,
tok::kw_while, tok::l_paren,
tok::comma) ||
Left->Previous->endsSequence(tok::kw_constexpr, tok::kw_if) ||
Left->Previous->isIf() ||
Left->Previous->is(TT_BinaryOperator))) {
// static_assert, if and while usually contain expressions.
Contexts.back().IsExpression = true;
Expand Down Expand Up @@ -825,8 +825,9 @@ class AnnotatingParser {
break;
case tok::kw_if:
case tok::kw_while:
assert(!Line.startsWith(tok::hash));
if (Tok->is(tok::kw_if) && CurrentToken &&
CurrentToken->is(tok::kw_constexpr))
CurrentToken->isOneOf(tok::kw_constexpr, tok::identifier))
next();
if (CurrentToken && CurrentToken->is(tok::l_paren)) {
next();
Expand Down Expand Up @@ -1078,6 +1079,7 @@ class AnnotatingParser {
case tok::pp_if:
case tok::pp_elif:
Contexts.back().IsExpression = true;
next();
parseLine();
break;
default:
Expand Down Expand Up @@ -2409,8 +2411,7 @@ unsigned TokenAnnotator::splitPenalty(const AnnotatedLine &Line,
Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign)
return 100;
if (Left.is(tok::l_paren) && Left.Previous &&
(Left.Previous->isOneOf(tok::kw_if, tok::kw_for) ||
Left.Previous->endsSequence(tok::kw_constexpr, tok::kw_if)))
(Left.Previous->is(tok::kw_for) || Left.Previous->isIf()))
return 1000;
if (Left.is(tok::equal) && InFunctionDecl)
return 110;
Expand Down Expand Up @@ -2611,10 +2612,10 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
return true;
return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
(Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok::kw_while,
(Left.isOneOf(tok::pp_elif, tok::kw_for, tok::kw_while,
tok::kw_switch, tok::kw_case, TT_ForEachMacro,
TT_ObjCForIn) ||
Left.endsSequence(tok::kw_constexpr, tok::kw_if) ||
Left.isIf(Line.Type != LT_PreprocessorDirective) ||
(Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,
tok::kw_new, tok::kw_delete) &&
(!Left.Previous || Left.Previous->isNot(tok::period))))) ||
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Format/UnwrappedLineParser.cpp
View file
Expand Up @@ -1756,7 +1756,7 @@ void UnwrappedLineParser::parseSquare(bool LambdaIntroducer) {
void UnwrappedLineParser::parseIfThenElse() {
assert(FormatTok->Tok.is(tok::kw_if) && "'if' expected");
nextToken();
if (FormatTok->Tok.is(tok::kw_constexpr))
if (FormatTok->Tok.isOneOf(tok::kw_constexpr, tok::identifier))
nextToken();
if (FormatTok->Tok.is(tok::l_paren))
parseParens();
Expand Down
89 changes: 89 additions & 0 deletions clang/unittests/Format/FormatTest.cpp
View file
Expand Up @@ -426,16 +426,28 @@ TEST_F(FormatTest, FormatIfWithoutCompoundStatement) {
verifyFormat("if (a)\n if (b) {\n f();\n }\ng();");
verifyFormat("if constexpr (true)\n"
" f();\ng();");
verifyFormat("if CONSTEXPR (true)\n"
" f();\ng();");
verifyFormat("if constexpr (a)\n"
" if constexpr (b)\n"
" if constexpr (c)\n"
" g();\n"
"h();");
verifyFormat("if CONSTEXPR (a)\n"
" if CONSTEXPR (b)\n"
" if CONSTEXPR (c)\n"
" g();\n"
"h();");
verifyFormat("if constexpr (a)\n"
" if constexpr (b) {\n"
" f();\n"
" }\n"
"g();");
verifyFormat("if CONSTEXPR (a)\n"
" if CONSTEXPR (b) {\n"
" f();\n"
" }\n"
"g();");

FormatStyle AllowsMergedIf = getLLVMStyle();
AllowsMergedIf.AlignEscapedNewlines = FormatStyle::ENAS_Left;
Expand Down Expand Up @@ -561,10 +573,12 @@ TEST_F(FormatTest, FormatShortBracedStatements) {

verifyFormat("if (true) {}", AllowSimpleBracedStatements);
verifyFormat("if constexpr (true) {}", AllowSimpleBracedStatements);
verifyFormat("if CONSTEXPR (true) {}", AllowSimpleBracedStatements);
verifyFormat("while (true) {}", AllowSimpleBracedStatements);
verifyFormat("for (;;) {}", AllowSimpleBracedStatements);
verifyFormat("if (true) { f(); }", AllowSimpleBracedStatements);
verifyFormat("if constexpr (true) { f(); }", AllowSimpleBracedStatements);
verifyFormat("if CONSTEXPR (true) { f(); }", AllowSimpleBracedStatements);
verifyFormat("while (true) { f(); }", AllowSimpleBracedStatements);
verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
verifyFormat("if (true) {\n"
Expand Down Expand Up @@ -633,10 +647,12 @@ TEST_F(FormatTest, FormatShortBracedStatements) {

verifyFormat("if (true) {}", AllowSimpleBracedStatements);
verifyFormat("if constexpr (true) {}", AllowSimpleBracedStatements);
verifyFormat("if CONSTEXPR (true) {}", AllowSimpleBracedStatements);
verifyFormat("while (true) {}", AllowSimpleBracedStatements);
verifyFormat("for (;;) {}", AllowSimpleBracedStatements);
verifyFormat("if (true) { f(); }", AllowSimpleBracedStatements);
verifyFormat("if constexpr (true) { f(); }", AllowSimpleBracedStatements);
verifyFormat("if CONSTEXPR (true) { f(); }", AllowSimpleBracedStatements);
verifyFormat("while (true) { f(); }", AllowSimpleBracedStatements);
verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
verifyFormat("if (true)\n"
Expand Down Expand Up @@ -750,6 +766,19 @@ TEST_F(FormatTest, ParseIfElse) {
"else {\n"
" i();\n"
"}");
verifyFormat("if (true)\n"
" if CONSTEXPR (true)\n"
" if (true) {\n"
" if CONSTEXPR (true)\n"
" f();\n"
" } else {\n"
" g();\n"
" }\n"
" else\n"
" h();\n"
"else {\n"
" i();\n"
"}");
verifyFormat("void f() {\n"
" if (a) {\n"
" } else {\n"
Expand All @@ -771,6 +800,12 @@ TEST_F(FormatTest, ElseIf) {
" g();\n"
"else\n"
" h();");
verifyFormat("if CONSTEXPR (a)\n"
" f();\n"
"else if CONSTEXPR (b)\n"
" g();\n"
"else\n"
" h();");
verifyFormat("if (a) {\n"
" f();\n"
"}\n"
Expand All @@ -783,6 +818,14 @@ TEST_F(FormatTest, ElseIf) {
"} else if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
" aaaaaaaaaaaaaaaaaaaaaaaaaaaa)) {\n"
"}");
verifyFormat("if (a) {\n"
"} else if constexpr (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
" aaaaaaaaaaaaaaaaaaaaaaaaaaaa)) {\n"
"}");
verifyFormat("if (a) {\n"
"} else if CONSTEXPR (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
" aaaaaaaaaaaaaaaaaaaaaaaaaaaa)) {\n"
"}");
verifyFormat("if (a) {\n"
"} else if (\n"
" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {\n"
Expand All @@ -793,6 +836,11 @@ TEST_F(FormatTest, ElseIf) {
" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {\n"
"}",
getLLVMStyleWithColumns(62));
verifyFormat("if (a) {\n"
"} else if CONSTEXPR (\n"
" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {\n"
"}",
getLLVMStyleWithColumns(62));
}

TEST_F(FormatTest, FormatsForLoop) {
Expand Down Expand Up @@ -3725,6 +3773,9 @@ TEST_F(FormatTest, LineBreakingInBinaryExpressions) {
verifyFormat("if constexpr ((aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ||\n"
" bbbbbbbbbbbbbbbbbb) && // aaaaaaaaaaa\n"
" cccccc) {\n}");
verifyFormat("if CONSTEXPR ((aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ||\n"
" bbbbbbbbbbbbbbbbbb) && // aaaaaaaaaaa\n"
" cccccc) {\n}");
verifyFormat("b = a &&\n"
" // Comment\n"
" b.c && d;");
Expand Down Expand Up @@ -3812,6 +3863,14 @@ TEST_F(FormatTest, ExpressionIndentation) {
"} else if (aaaaa && bbbbb > // break\n"
" ccccc) {\n"
"}");
verifyFormat("if () {\n"
"} else if constexpr (aaaaa && bbbbb > // break\n"
" ccccc) {\n"
"}");
verifyFormat("if () {\n"
"} else if CONSTEXPR (aaaaa && bbbbb > // break\n"
" ccccc) {\n"
"}");
verifyFormat("if () {\n"
"} else if (aaaaa &&\n"
" bbbbb > // break\n"
Expand Down Expand Up @@ -6913,7 +6972,11 @@ TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
verifyIndependentOfContext("if (int *a = &b)");
verifyIndependentOfContext("if (int &a = *b)");
verifyIndependentOfContext("if (a & b[i])");
verifyIndependentOfContext("if constexpr (a & b[i])");
verifyIndependentOfContext("if CONSTEXPR (a & b[i])");
verifyIndependentOfContext("if (a * (b * c))");
verifyIndependentOfContext("if constexpr (a * (b * c))");
verifyIndependentOfContext("if CONSTEXPR (a * (b * c))");
verifyIndependentOfContext("if (a::b::c::d & b[i])");
verifyIndependentOfContext("if (*b[i])");
verifyIndependentOfContext("if (int *a = (&b))");
Expand Down Expand Up @@ -8602,6 +8665,9 @@ TEST_F(FormatTest, MergeHandlingInTheFaceOfPreprocessorDirectives) {
verifyFormat("#define A \\\n"
" if constexpr (true) return 42;",
ShortMergedIf);
verifyFormat("#define A \\\n"
" if CONSTEXPR (true) return 42;",
ShortMergedIf);
ShortMergedIf.ColumnLimit = 29;
verifyFormat("#define A \\\n"
" if (aaaaaaaaaa) return 1; \\\n"
Expand All @@ -8618,6 +8684,11 @@ TEST_F(FormatTest, MergeHandlingInTheFaceOfPreprocessorDirectives) {
" return 1; \\\n"
" return 2;",
ShortMergedIf);
verifyFormat("#define A \\\n"
" if CONSTEXPR (aaaaaaa) \\\n"
" return 1; \\\n"
" return 2;",
ShortMergedIf);
}

TEST_F(FormatTest, FormatStarDependingOnContext) {
Expand Down Expand Up @@ -11181,6 +11252,14 @@ TEST_F(FormatTest, AllmanBraceBreaking) {
" }\n"
"}\n",
BreakBeforeBraceShortIfs);
verifyFormat("void f(bool b)\n"
"{\n"
" if CONSTEXPR (b)\n"
" {\n"
" return;\n"
" }\n"
"}\n",
BreakBeforeBraceShortIfs);
verifyFormat("void f(bool b)\n"
"{\n"
" if (b) return;\n"
Expand All @@ -11191,6 +11270,11 @@ TEST_F(FormatTest, AllmanBraceBreaking) {
" if constexpr (b) return;\n"
"}\n",
BreakBeforeBraceShortIfs);
verifyFormat("void f(bool b)\n"
"{\n"
" if CONSTEXPR (b) return;\n"
"}\n",
BreakBeforeBraceShortIfs);
verifyFormat("void f(bool b)\n"
"{\n"
" while (b)\n"
Expand Down Expand Up @@ -12858,6 +12942,11 @@ TEST_F(FormatTest, FormatsLambdas) {
" doo_dah();\n"
" })) {\n"
"}");
verifyFormat("if CONSTEXPR (blah_blah(whatever, whatever, [] {\n"
" doo_dah();\n"
" doo_dah();\n"
" })) {\n"
"}");
verifyFormat("auto lambda = []() {\n"
" int a = 2\n"
"#if A\n"
Expand Down

0 comments on commit 1361a4c

Please sign in to comment.