[clang] [clang-format] Fix bad comment indentation before ifdef after braceless if (PR #94776)

via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 7 10:53:22 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-format

Author: Erich Reitz (Erich-Reitz)

<details>
<summary>Changes</summary>

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 

---
Full diff: https://github.com/llvm/llvm-project/pull/94776.diff


3 Files Affected:

- (modified) clang/lib/Format/UnwrappedLineParser.cpp (+13-3) 
- (modified) clang/lib/Format/UnwrappedLineParser.h (+3) 
- (modified) clang/unittests/Format/FormatTestComments.cpp (+18) 


``````````diff
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index b15a87327240b..b17ef33f95e98 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2705,6 +2705,7 @@ void UnwrappedLineParser::parseUnbracedBody(bool CheckEOF) {
 
   addUnwrappedLine();
   ++Line->Level;
+  ++UnBracedBodyDepth;
   parseStructuralElement();
 
   if (Tok) {
@@ -2724,6 +2725,7 @@ void UnwrappedLineParser::parseUnbracedBody(bool CheckEOF) {
     addUnwrappedLine();
 
   --Line->Level;
+  --UnBracedBodyDepth;
 }
 
 static void markOptionalBraces(FormatToken *LeftBrace) {
@@ -4736,6 +4738,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 +4814,11 @@ 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 +4834,11 @@ 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..428c7dde182e1 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -338,6 +338,9 @@ 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 {
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.

``````````

</details>


https://github.com/llvm/llvm-project/pull/94776


More information about the cfe-commits mailing list