[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 14:44:11 PDT 2024
https://github.com/Erich-Reitz updated https://github.com/llvm/llvm-project/pull/94776
>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/4] 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/4] 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/4] 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.
>From a9724e3ba9aa136111701d69619ae3635991b632 Mon Sep 17 00:00:00 2001
From: Erich Reitz <erich.reitz at protonmail.com>
Date: Fri, 7 Jun 2024 17:43:46 -0400
Subject: [PATCH 4/4] add second test; fix formatting
---
clang/lib/Format/UnwrappedLineParser.cpp | 4 +-
clang/unittests/Format/FormatTestComments.cpp | 50 +++++++++++++------
2 files changed, 36 insertions(+), 18 deletions(-)
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index b17ef33f95e98..7c571907acb1e 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -4738,7 +4738,6 @@ 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;
@@ -4835,9 +4834,8 @@ void UnwrappedLineParser::readToken(int LevelDifference) {
Line->Level += PPBranchLevel;
}
- if (!UnBracedBodyDepth) {
+ if (!UnBracedBodyDepth)
flushComments(isOnNewLine(*FormatTok));
- }
parsePPDirective();
PreviousWasComment = FormatTok->is(tok::comment);
diff --git a/clang/unittests/Format/FormatTestComments.cpp b/clang/unittests/Format/FormatTestComments.cpp
index e055a7c59f400..c1db28b53f88f 100644
--- a/clang/unittests/Format/FormatTestComments.cpp
+++ b/clang/unittests/Format/FormatTestComments.cpp
@@ -796,22 +796,42 @@ 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"
- "}"));
+ verifyFormat("void f() {\n"
+ " if (true)\n"
+ " int i;\n"
+ " /* comment */\n"
+ "#ifdef A\n"
+ " int j;\n"
+ "#endif\n"
+ "}",
+ "void f() {\n"
+ " if (true)\n"
+ " int i;\n"
+ " /* comment */\n"
+ "#ifdef A\n"
+ " int j;\n"
+ "#endif\n"
+ "}");
+
+ verifyFormat("void f() {\n"
+ " if (true)\n"
+ " int i;\n"
+ " /* comment */\n"
+ "\n"
+ "#ifdef A\n"
+ " int j;\n"
+ "#endif\n"
+ "}",
+ "void f() {\n"
+ " if (true)\n"
+ " int i;\n"
+ " /* comment */\n"
+ "\n"
+ "#ifdef A\n"
+ " int j;\n"
+ "#endif\n"
+ "}");
}
TEST_F(FormatTestComments, KeepsLevelOfCommentBeforePPDirective) {
More information about the cfe-commits
mailing list