Skip to content

Commit

Permalink
[clang-format] Skip PP directives when determining brace kind (#69473)
Browse files Browse the repository at this point in the history
Pull request #65409 changed the brace kind heuristic to not treat a
preprocessor if directive as a in statement, however, this caused some
regressions.

If the contents of a brace don't immediately determine the brace kind,
the heuristic will look at the characters immediately before and after
the closing brace to determine the brace type.

Unfortunately, if the closing brace was preceded by a preprocessor
directive, for example `#endif`, the preceding token was `endif`, seen
as just an identifier, so the braces were understood as a braced list.

This patch fixes this behaviour by skipping all preprocessor directives
when calculating brace types. Comments were already being skipped, so
now preprocessor lines are skipped alongside comments.

Fixes #68404
  • Loading branch information
rymiel committed Nov 3, 2023
1 parent b5b251a commit 063e3fe
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 5 deletions.
14 changes: 9 additions & 5 deletions clang/lib/Format/UnwrappedLineParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -491,11 +491,18 @@ void UnwrappedLineParser::calculateBraceTypes(bool ExpectClassBody) {
SmallVector<StackEntry, 8> LBraceStack;
assert(Tok->is(tok::l_brace));
do {
// Get next non-comment token.
// Get next non-comment, non-preprocessor token.
FormatToken *NextTok;
do {
NextTok = Tokens->getNextToken();
} while (NextTok->is(tok::comment));
while (NextTok->is(tok::hash)) {
NextTok = Tokens->getNextToken();
do {
NextTok = Tokens->getNextToken();
} while (NextTok->is(tok::comment) ||
(NextTok->NewlinesBefore == 0 && NextTok->isNot(tok::eof)));
}

switch (Tok->Tok.getKind()) {
case tok::l_brace:
Expand Down Expand Up @@ -611,12 +618,9 @@ void UnwrappedLineParser::calculateBraceTypes(bool ExpectClassBody) {
if (Tok->isNot(TT_StatementMacro))
break;
[[fallthrough]];
case tok::kw_if:
if (PrevTok->is(tok::hash))
break;
[[fallthrough]];
case tok::at:
case tok::semi:
case tok::kw_if:
case tok::kw_while:
case tok::kw_for:
case tok::kw_switch:
Expand Down
15 changes: 15 additions & 0 deletions clang/unittests/Format/TokenAnnotatorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2152,6 +2152,7 @@ TEST_F(TokenAnnotatorTest, UnderstandConstructors) {
EXPECT_TOKEN(Tokens[11], tok::l_paren, TT_Unknown);
EXPECT_TOKEN(Tokens[12], tok::r_paren, TT_Unknown);
EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_FunctionLBrace);
EXPECT_BRACE_KIND(Tokens[13], BK_Block);

Tokens = annotate("Class::Class() : BaseClass{}, Member{} {}");
ASSERT_EQ(Tokens.size(), 16u) << Tokens;
Expand All @@ -2164,6 +2165,20 @@ TEST_F(TokenAnnotatorTest, UnderstandConstructors) {
EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_Unknown);
EXPECT_TOKEN(Tokens[12], tok::r_brace, TT_Unknown);
EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_FunctionLBrace);
EXPECT_BRACE_KIND(Tokens[13], BK_Block);

Tokens = annotate("class Class {\n"
" Class() : BaseClass() {\n"
"#if 0\n"
" // comment\n"
"#endif\n"
" }\n"
" Class f();\n"
"}");
ASSERT_EQ(Tokens.size(), 25u) << Tokens;
EXPECT_TOKEN(Tokens[6], tok::colon, TT_CtorInitializerColon);
EXPECT_TOKEN(Tokens[10], tok::l_brace, TT_FunctionLBrace);
EXPECT_BRACE_KIND(Tokens[10], BK_Block);
}

TEST_F(TokenAnnotatorTest, UnderstandsConditionParens) {
Expand Down

0 comments on commit 063e3fe

Please sign in to comment.