[clang] [clang-format] Fix a bug in aligning comments above finalized line (PR #202253)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-format

Author: owenca (owenca)

<details>
<summary>Changes</summary>

Don't change the indent level of the comments if they are already aligned with the finalized line below.

Fixes #<!-- -->200521

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


2 Files Affected:

- (modified) clang/lib/Format/TokenAnnotator.cpp (+17-10) 
- (modified) clang/unittests/Format/FormatTest.cpp (+31-5) 


``````````diff
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index c47a512454476..b2ccd411cbad1 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3657,22 +3657,29 @@ void TokenAnnotator::setCommentLineLevels(
 
     // If the comment is currently aligned with the line immediately following
     // it, that's probably intentional and we should keep it.
-    if (NextNonCommentLine && NextNonCommentLine->First->NewlinesBefore < 2 &&
+    if (const auto Column = Line->First->OriginalColumn;
+        NextNonCommentLine && NextNonCommentLine->First->NewlinesBefore < 2 &&
         Line->isComment() && !isClangFormatOff(Line->First->TokenText) &&
-        NextNonCommentLine->First->OriginalColumn ==
-            Line->First->OriginalColumn) {
+        NextNonCommentLine->First->OriginalColumn == Column) {
       const bool PPDirectiveOrImportStmt =
           NextNonCommentLine->Type == LT_PreprocessorDirective ||
           NextNonCommentLine->Type == LT_ImportStatement;
       if (PPDirectiveOrImportStmt)
         Line->Type = LT_CommentAbovePPDirective;
-      // Align comments for preprocessor lines with the # in column 0 if
-      // preprocessor lines are not indented. Otherwise, align with the next
-      // line.
-      Line->Level = Style.IndentPPDirectives < FormatStyle::PPDIS_BeforeHash &&
-                            PPDirectiveOrImportStmt
-                        ? 0
-                        : NextNonCommentLine->Level;
+      if (const auto IndentWidth = Style.IndentWidth;
+          NextNonCommentLine->First->Finalized && IndentWidth > 0 &&
+          Column % IndentWidth == 0) {
+        Line->Level = Column / IndentWidth;
+      } else {
+        // Align comments for preprocessor lines with the # in column 0 if
+        // preprocessor lines are not indented. Otherwise, align with the next
+        // line.
+        Line->Level =
+            Style.IndentPPDirectives < FormatStyle::PPDIS_BeforeHash &&
+                    PPDirectiveOrImportStmt
+                ? 0
+                : NextNonCommentLine->Level;
+      }
     } else {
       NextNonCommentLine = Line->First->isNot(tok::r_brace) ? Line : nullptr;
     }
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 1f243fe967fd1..689707886e0d1 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -22495,6 +22495,12 @@ TEST_F(FormatTest, OneLineFormatOffRegex) {
                  "   MACRO_TEST2( );",
                  Style);
 
+  Style.OneLineFormatOffRegex = "//(< clang-format off| NO_TRANSLATION)$";
+  verifyNoChange(
+      " int i ;  //< clang-format off\n"
+      " msg = sprintf(\"Long string with placeholders.\"); // NO_TRANSLATION",
+      Style);
+
   Style.ColumnLimit = 50;
   Style.OneLineFormatOffRegex = "^LogErrorPrint$";
   verifyFormat(" myproject::LogErrorPrint(logger, \"Don't split me!\");\n"
@@ -22504,11 +22510,31 @@ TEST_F(FormatTest, OneLineFormatOffRegex) {
                " myproject::MyLogErrorPrinter(myLogger, \"Split me!\");",
                Style);
 
-  Style.OneLineFormatOffRegex = "//(< clang-format off| NO_TRANSLATION)$";
-  verifyNoChange(
-      " int i ;  //< clang-format off\n"
-      " msg = sprintf(\"Long string with placeholders.\"); // NO_TRANSLATION",
-      Style);
+  Style.ColumnLimit = 20;
+  Style.OneLineFormatOffRegex = "^pragma$";
+  verifyFormat("void pragmas() {\n"
+               "  // a comment\n"
+               "  // that's too long\n"
+               "  #pragma omp\n"
+               "  my_pragma();\n"
+               "}",
+               "void pragmas () {\n"
+               "  // a comment that's\n"
+               "  // too long\n"
+               "  #pragma omp\n"
+               " my_pragma();\n"
+               "}",
+               Style);
+
+  Style.OneLineFormatOffRegex = "// FormatOff$";
+  verifyFormat("  // comment too\n"
+               "  // long\n"
+               "  f() ; // FormatOff\n"
+               "g();",
+               "  // comment too long\n"
+               "  f() ; // FormatOff\n"
+               " g();",
+               Style);
 }
 
 TEST_F(FormatTest, DoNotCrashOnInvalidInput) {

``````````

</details>


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