[clang] [clang-format] Fix bad comment indentation before ifdef after braceless if (PR #94776)
Erich Reitz via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 7 10:52:33 PDT 2024
https://github.com/Erich-Reitz created https://github.com/llvm/llvm-project/pull/94776
Addresses issue #45002.
When `readToken()` sees a preprocessor directive, it eagerly flushes the comments. This changes the behavior if it is within a braceless block.
Keeps behavior of
```
void f() {
if (foo)
// test
#ifdef
bax();
#endif
}
```
formatted as
```
void f() {
if (foo)
// test
#ifdef
bax();
#endif
}
```
for (specifically) comments within the braceless block
>From 6c910c8b40be79e3d573f6953860f60ebd27b39f Mon Sep 17 00:00:00 2001
From: Erich Reitz <erich.reitz at protonmail.com>
Date: Fri, 7 Jun 2024 13:04:33 -0400
Subject: [PATCH 1/3] delay flushing comments before ifdef after braceless if;
align with token that begins conditional
---
clang/lib/Format/UnwrappedLineParser.cpp | 19 +++++++++++++++----
clang/lib/Format/UnwrappedLineParser.h | 2 ++
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index b15a87327240b..7bc066787bf46 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -354,6 +354,7 @@ bool UnwrappedLineParser::precededByCommentOrPPDirective() const {
bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace,
IfStmtKind *IfKind,
FormatToken **IfLeftBrace) {
+
const bool InRequiresExpression =
OpeningBrace && OpeningBrace->is(TT_RequiresExpressionLBrace);
const bool IsPrecededByCommentOrPPDirective =
@@ -385,6 +386,7 @@ bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace,
};
switch (Kind) {
+
case tok::comment:
nextToken();
addUnwrappedLine();
@@ -1419,6 +1421,7 @@ void UnwrappedLineParser::readTokenWithJavaScriptASI() {
void UnwrappedLineParser::parseStructuralElement(
const FormatToken *OpeningBrace, IfStmtKind *IfKind,
FormatToken **IfLeftBrace, bool *HasDoWhile, bool *HasLabel) {
+
if (Style.Language == FormatStyle::LK_TableGen &&
FormatTok->is(tok::pp_include)) {
nextToken();
@@ -1696,6 +1699,7 @@ void UnwrappedLineParser::parseStructuralElement(
const bool InRequiresExpression =
OpeningBrace && OpeningBrace->is(TT_RequiresExpressionLBrace);
+
do {
const FormatToken *Previous = FormatTok->Previous;
switch (FormatTok->Tok.getKind()) {
@@ -2705,6 +2709,7 @@ void UnwrappedLineParser::parseUnbracedBody(bool CheckEOF) {
addUnwrappedLine();
++Line->Level;
+ ++UnBracedBodyDepth;
parseStructuralElement();
if (Tok) {
@@ -2719,11 +2724,11 @@ void UnwrappedLineParser::parseUnbracedBody(bool CheckEOF) {
assert(Tok);
++Tok->BraceCount;
}
-
if (CheckEOF && eof())
addUnwrappedLine();
--Line->Level;
+ --UnBracedBodyDepth;
}
static void markOptionalBraces(FormatToken *LeftBrace) {
@@ -4736,6 +4741,7 @@ void UnwrappedLineParser::distributeComments(
// the two lines about b form a maximal trail, so there are two sections, the
// first one consisting of the single comment "// line about a" and the
// second one consisting of the next two comments.
+
if (Comments.empty())
return;
bool ShouldPushCommentsInCurrentLine = true;
@@ -4811,8 +4817,10 @@ void UnwrappedLineParser::readToken(int LevelDifference) {
(!Style.isVerilog() ||
Keywords.isVerilogPPDirective(*Tokens->peekNextToken())) &&
FirstNonCommentOnLine) {
- distributeComments(Comments, FormatTok);
- Comments.clear();
+ if (!UnBracedBodyDepth) {
+ distributeComments(Comments, FormatTok);
+ Comments.clear();
+ }
// If there is an unfinished unwrapped line, we flush the preprocessor
// directives only after that unwrapped line was finished later.
bool SwitchToPreprocessorLines = !Line->Tokens.empty();
@@ -4828,7 +4836,10 @@ void UnwrappedLineParser::readToken(int LevelDifference) {
PPBranchLevel > 0) {
Line->Level += PPBranchLevel;
}
- flushComments(isOnNewLine(*FormatTok));
+ if (!UnBracedBodyDepth) {
+ flushComments(isOnNewLine(*FormatTok));
+ }
+
parsePPDirective();
PreviousWasComment = FormatTok->is(tok::comment);
FirstNonCommentOnLine = IsFirstNonCommentOnLine(
diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h
index d7963a4211bb9..4d87896870a3e 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -338,6 +338,8 @@ class UnwrappedLineParser {
// `decltype(auto)`.
bool IsDecltypeAutoFunction = false;
+ int UnBracedBodyDepth = 0;
+
// Represents preprocessor branch type, so we can find matching
// #if/#else/#endif directives.
enum PPBranchKind {
>From b9d52022e1caf314cb3f24f03775c8baf5da1c4a Mon Sep 17 00:00:00 2001
From: Erich Reitz <erich.reitz at protonmail.com>
Date: Fri, 7 Jun 2024 13:08:13 -0400
Subject: [PATCH 2/3] whitespace formatting
---
clang/lib/Format/UnwrappedLineParser.cpp | 7 +++----
clang/lib/Format/UnwrappedLineParser.h | 1 +
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 7bc066787bf46..b17ef33f95e98 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -354,7 +354,6 @@ bool UnwrappedLineParser::precededByCommentOrPPDirective() const {
bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace,
IfStmtKind *IfKind,
FormatToken **IfLeftBrace) {
-
const bool InRequiresExpression =
OpeningBrace && OpeningBrace->is(TT_RequiresExpressionLBrace);
const bool IsPrecededByCommentOrPPDirective =
@@ -386,7 +385,6 @@ bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace,
};
switch (Kind) {
-
case tok::comment:
nextToken();
addUnwrappedLine();
@@ -1421,7 +1419,6 @@ void UnwrappedLineParser::readTokenWithJavaScriptASI() {
void UnwrappedLineParser::parseStructuralElement(
const FormatToken *OpeningBrace, IfStmtKind *IfKind,
FormatToken **IfLeftBrace, bool *HasDoWhile, bool *HasLabel) {
-
if (Style.Language == FormatStyle::LK_TableGen &&
FormatTok->is(tok::pp_include)) {
nextToken();
@@ -1699,7 +1696,6 @@ void UnwrappedLineParser::parseStructuralElement(
const bool InRequiresExpression =
OpeningBrace && OpeningBrace->is(TT_RequiresExpressionLBrace);
-
do {
const FormatToken *Previous = FormatTok->Previous;
switch (FormatTok->Tok.getKind()) {
@@ -2724,6 +2720,7 @@ void UnwrappedLineParser::parseUnbracedBody(bool CheckEOF) {
assert(Tok);
++Tok->BraceCount;
}
+
if (CheckEOF && eof())
addUnwrappedLine();
@@ -4817,6 +4814,7 @@ void UnwrappedLineParser::readToken(int LevelDifference) {
(!Style.isVerilog() ||
Keywords.isVerilogPPDirective(*Tokens->peekNextToken())) &&
FirstNonCommentOnLine) {
+
if (!UnBracedBodyDepth) {
distributeComments(Comments, FormatTok);
Comments.clear();
@@ -4836,6 +4834,7 @@ void UnwrappedLineParser::readToken(int LevelDifference) {
PPBranchLevel > 0) {
Line->Level += PPBranchLevel;
}
+
if (!UnBracedBodyDepth) {
flushComments(isOnNewLine(*FormatTok));
}
diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h
index 4d87896870a3e..428c7dde182e1 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -338,6 +338,7 @@ class UnwrappedLineParser {
// `decltype(auto)`.
bool IsDecltypeAutoFunction = false;
+
int UnBracedBodyDepth = 0;
// Represents preprocessor branch type, so we can find matching
>From d5aca6de9f513ce049c337dc4c17ea32bb2721aa Mon Sep 17 00:00:00 2001
From: Erich Reitz <erich.reitz at protonmail.com>
Date: Fri, 7 Jun 2024 13:33:28 -0400
Subject: [PATCH 3/3] add basic unit test for change
---
clang/unittests/Format/FormatTestComments.cpp | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/clang/unittests/Format/FormatTestComments.cpp b/clang/unittests/Format/FormatTestComments.cpp
index d2baace6a7d80..e055a7c59f400 100644
--- a/clang/unittests/Format/FormatTestComments.cpp
+++ b/clang/unittests/Format/FormatTestComments.cpp
@@ -796,6 +796,24 @@ TEST_F(FormatTestComments, ParsesCommentsAdjacentToPPDirectives) {
format("namespace {}\n /* Test */ #define A"));
}
+
+TEST_F(FormatTestComments, DeIdentsCommentBeforeIfdefAfterBracelessIf) {
+ EXPECT_EQ("void f() {\n"
+ " if (true)\n"
+ " int i;\n"
+ " /* comment */\n"
+ "#ifdef A\n"
+ " int j;\n"
+ "}",
+ format("void f() {\n"
+ " if (true)\n"
+ " int i;\n"
+ " /* comment */\n"
+ "#ifdef A\n"
+ " int j;\n"
+ "}"));
+}
+
TEST_F(FormatTestComments, KeepsLevelOfCommentBeforePPDirective) {
// Keep the current level if the comment was originally not aligned with
// the preprocessor directive.
More information about the cfe-commits
mailing list