Skip to content

Conversation

@owenca
Copy link
Contributor

@owenca owenca commented Nov 1, 2025

  • Replace verifyFormat(Foo, Bar, ...) with verifyFormat(Foo, ...) or verifyNoChange(Foo, ...) if Foo = Bar.

  • Other minor cleanups

- Replace verifyFormat(Foo, Bar, ...) with verifyFormat(Foo, ...) or
  verifyNoChange(Foo, ...) if Foo = Bar.

- Other minor cleanups
@llvmbot
Copy link
Member

llvmbot commented Nov 1, 2025

@llvm/pr-subscribers-clang-format

Author: owenca (owenca)

Changes
  • Replace verifyFormat(Foo, Bar, ...) with verifyFormat(Foo, ...) or verifyNoChange(Foo, ...) if Foo = Bar.

  • Other minor cleanups


Patch is 27.18 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/166029.diff

1 Files Affected:

  • (modified) clang/unittests/Format/FormatTestComments.cpp (+164-286)
diff --git a/clang/unittests/Format/FormatTestComments.cpp b/clang/unittests/Format/FormatTestComments.cpp
index 399f8357692ba..d7b2257605482 100644
--- a/clang/unittests/Format/FormatTestComments.cpp
+++ b/clang/unittests/Format/FormatTestComments.cpp
@@ -99,14 +99,6 @@ TEST_F(FormatTestComments, UnderstandsSingleLineComments) {
   const auto Style20 = getLLVMStyleWithColumns(20);
 
   verifyFormat("enum A {\n"
-               "  // line a\n"
-               "  a,\n"
-               "  b, // line b\n"
-               "\n"
-               "  // line c\n"
-               "  c\n"
-               "};",
-               "enum A {\n"
                "  // line a\n"
                "  a,\n"
                "  b, // line b\n"
@@ -115,15 +107,11 @@ TEST_F(FormatTestComments, UnderstandsSingleLineComments) {
                "  c\n"
                "};",
                Style20);
-  verifyFormat("enum A {\n"
-               "  a, // line 1\n"
-               "  // line 2\n"
-               "};",
-               "enum A {\n"
-               "  a, // line 1\n"
-               "  // line 2\n"
-               "};",
-               Style20);
+  verifyNoChange("enum A {\n"
+                 "  a, // line 1\n"
+                 "  // line 2\n"
+                 "};",
+                 Style20);
   verifyFormat("enum A {\n"
                "  a, // line 1\n"
                "     // line 2\n"
@@ -133,17 +121,12 @@ TEST_F(FormatTestComments, UnderstandsSingleLineComments) {
                "   // line 2\n"
                "};",
                Style20);
-  verifyFormat("enum A {\n"
-               "  a, // line 1\n"
-               "  // line 2\n"
-               "  b\n"
-               "};",
-               "enum A {\n"
-               "  a, // line 1\n"
-               "  // line 2\n"
-               "  b\n"
-               "};",
-               Style20);
+  verifyNoChange("enum A {\n"
+                 "  a, // line 1\n"
+                 "  // line 2\n"
+                 "  b\n"
+                 "};",
+                 Style20);
   verifyFormat("enum A {\n"
                "  a, // line 1\n"
                "     // line 2\n"
@@ -487,12 +470,9 @@ TEST_F(FormatTestComments, AlignsBlockComments) {
                " Don't try to outdent if there's not enough indentation.\n"
                " */");
 
-  verifyFormat("int i; /* Comment with empty...\n"
-               "        *\n"
-               "        * line. */",
-               "int i; /* Comment with empty...\n"
-               "        *\n"
-               "        * line. */");
+  verifyNoChange("int i; /* Comment with empty...\n"
+                 "        *\n"
+                 "        * line. */");
   verifyFormat("int foobar = 0; /* comment */\n"
                "int bar = 0;    /* multiline\n"
                "                   comment 1 */\n"
@@ -555,8 +535,6 @@ TEST_F(FormatTestComments, CommentReflowingCanApplyOnlyToIndents) {
 
 TEST_F(FormatTestComments, CorrectlyHandlesLengthOfBlockComments) {
   verifyFormat("double *x; /* aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
-               "              aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa */",
-               "double *x; /* aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
                "              aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa */");
   verifyFormat(
       "void ffffffffffff(\n"
@@ -680,17 +658,12 @@ TEST_F(FormatTestComments, SplitsLongCxxComments) {
   verifyFormat("#define XXX // q w e r\n"
                "            // t y u i",
                "#define XXX //q w e r t y u i", Style22);
-  verifyFormat("{\n"
-               "  //\n"
-               "  //\\\n"
-               "  // long 1 2 3 4 5\n"
-               "}",
-               "{\n"
-               "  //\n"
-               "  //\\\n"
-               "  // long 1 2 3 4 5\n"
-               "}",
-               Style20);
+  verifyNoChange("{\n"
+                 "  //\n"
+                 "  //\\\n"
+                 "  // long 1 2 3 4 5\n"
+                 "}",
+                 Style20);
   verifyFormat("{\n"
                "  //\n"
                "  //\\\n"
@@ -730,19 +703,13 @@ TEST_F(FormatTestComments, PreservesHangingIndentInCxxComments) {
 }
 
 TEST_F(FormatTestComments, DontSplitLineCommentsWithEscapedNewlines) {
-  verifyFormat("// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
-               "// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
-               "// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
-               "// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
-               "// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
-               "// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
-  verifyFormat("int a; // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
-               "       // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
-               "       // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
-               "int a; // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
-               "       // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
-               "       // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
-               getLLVMStyleWithColumns(50));
+  verifyNoChange("// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
+                 "// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
+                 "// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
+  verifyNoChange("int a; // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
+                 "       // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
+                 "       // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
+                 getLLVMStyleWithColumns(50));
   verifyFormat("double\n"
                "    a; // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
                "       // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
@@ -773,10 +740,8 @@ TEST_F(FormatTestComments, DontIntroduceMultilineComments) {
 TEST_F(FormatTestComments, DontSplitLineCommentsWithPragmas) {
   FormatStyle Pragmas = getLLVMStyleWithColumns(30);
   Pragmas.CommentPragmas = "^ IWYU pragma:";
-  verifyFormat("// IWYU pragma: aaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbb",
-               "// IWYU pragma: aaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbb", Pragmas);
-  verifyFormat("/* IWYU pragma: aaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbb */",
-               "/* IWYU pragma: aaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbb */", Pragmas);
+  verifyFormat("// IWYU pragma: aaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbb", Pragmas);
+  verifyFormat("/* IWYU pragma: aaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbb */", Pragmas);
 }
 
 TEST_F(FormatTestComments, PriorityOfCommentBreaking) {
@@ -812,26 +777,17 @@ TEST_F(FormatTestComments, PriorityOfCommentBreaking) {
 
 TEST_F(FormatTestComments, MultiLineCommentsInDefines) {
   const auto Style17 = getLLVMStyleWithColumns(17);
-  verifyFormat("#define A(x) /* \\\n"
-               "  a comment     \\\n"
-               "  inside */     \\\n"
-               "  f();",
-               "#define A(x) /* \\\n"
-               "  a comment     \\\n"
-               "  inside */     \\\n"
-               "  f();",
-               Style17);
-  verifyFormat("#define A(      \\\n"
-               "    x) /*       \\\n"
-               "  a comment     \\\n"
-               "  inside */     \\\n"
-               "  f();",
-               "#define A(      \\\n"
-               "    x) /*       \\\n"
-               "  a comment     \\\n"
-               "  inside */     \\\n"
-               "  f();",
-               Style17);
+  verifyNoChange("#define A(x) /* \\\n"
+                 "  a comment     \\\n"
+                 "  inside */     \\\n"
+                 "  f();",
+                 Style17);
+  verifyNoChange("#define A(      \\\n"
+                 "    x) /*       \\\n"
+                 "  a comment     \\\n"
+                 "  inside */     \\\n"
+                 "  f();",
+                 Style17);
 }
 
 TEST_F(FormatTestComments, LineCommentsInMacrosDoNotGetEscapedNewlines) {
@@ -865,33 +821,20 @@ TEST_F(FormatTestComments, ParsesCommentsAdjacentToPPDirectives) {
 TEST_F(FormatTestComments, KeepsLevelOfCommentBeforePPDirective) {
   // Keep the current level if the comment was originally not aligned with
   // the preprocessor directive.
-  verifyFormat("void f() {\n"
-               "  int i;\n"
-               "  /* comment */\n"
-               "#ifdef A\n"
-               "  int j;\n"
-               "}",
-               "void f() {\n"
-               "  int i;\n"
-               "  /* comment */\n"
-               "#ifdef A\n"
-               "  int j;\n"
-               "}");
+  verifyNoChange("void f() {\n"
+                 "  int i;\n"
+                 "  /* comment */\n"
+                 "#ifdef A\n"
+                 "  int j;\n"
+                 "}");
 
-  verifyFormat("void f() {\n"
-               "  int i;\n"
-               "  /* comment */\n"
-               "\n"
-               "#ifdef A\n"
-               "  int j;\n"
-               "}",
-               "void f() {\n"
-               "  int i;\n"
-               "  /* comment */\n"
-               "\n"
-               "#ifdef A\n"
-               "  int j;\n"
-               "}");
+  verifyNoChange("void f() {\n"
+                 "  int i;\n"
+                 "  /* comment */\n"
+                 "\n"
+                 "#ifdef A\n"
+                 "  int j;\n"
+                 "}");
 
   verifyFormat("int f(int i) {\n"
                "  if (true) {\n"
@@ -1060,18 +1003,12 @@ TEST_F(FormatTestComments, KeepsLevelOfCommentBeforePPDirective) {
   // Align with the preprocessor directive if the comment was originally aligned
   // with the preprocessor directive and there is no newline between the comment
   // and the preprocessor directive.
-  verifyFormat("void f() {\n"
-               "  int i;\n"
-               "/* comment */\n"
-               "#ifdef A\n"
-               "  int j;\n"
-               "}",
-               "void f() {\n"
-               "  int i;\n"
-               "/* comment */\n"
-               "#ifdef A\n"
-               "  int j;\n"
-               "}");
+  verifyNoChange("void f() {\n"
+                 "  int i;\n"
+                 "/* comment */\n"
+                 "#ifdef A\n"
+                 "  int j;\n"
+                 "}");
 
   verifyFormat("int f(int i) {\n"
                "  if (true) {\n"
@@ -1245,13 +1182,10 @@ TEST_F(FormatTestComments, SplitsLongLinesInComments) {
                "   wherever_a_space_occurs                             \n"
                " */",
                Style20);
-  verifyFormat("/*\n"
-               " *    This_comment_can_not_be_broken_into_lines\n"
-               " */",
-               "/*\n"
-               " *    This_comment_can_not_be_broken_into_lines\n"
-               " */",
-               Style20);
+  verifyNoChange("/*\n"
+                 " *    This_comment_can_not_be_broken_into_lines\n"
+                 " */",
+                 Style20);
   verifyFormat("{\n"
                "  /*\n"
                "  This is another\n"
@@ -1445,17 +1379,12 @@ TEST_F(FormatTestComments, AlignsPPElseEndifComments) {
                "int iiii; // CC\n"
                "#endif // B",
                Style20);
-  verifyFormat("#if A\n"
-               "#else  // A1\n"
-               "       // A2\n"
-               "int ii;\n"
-               "#endif // B",
-               "#if A\n"
-               "#else  // A1\n"
-               "       // A2\n"
-               "int ii;\n"
-               "#endif // B",
-               Style20);
+  verifyNoChange("#if A\n"
+                 "#else  // A1\n"
+                 "       // A2\n"
+                 "int ii;\n"
+                 "#endif // B",
+                 Style20);
 }
 
 TEST_F(FormatTestComments, CommentsInStaticInitializers) {
@@ -1526,10 +1455,6 @@ TEST_F(FormatTestComments, CommentsInStaticInitializers) {
 
 TEST_F(FormatTestComments, LineCommentsAfterRightBrace) {
   verifyFormat("if (true) { // comment about branch\n"
-               "  // comment about f\n"
-               "  f();\n"
-               "}",
-               "if (true) { // comment about branch\n"
                "  // comment about f\n"
                "  f();\n"
                "}");
@@ -1582,6 +1507,7 @@ TEST_F(FormatTestComments, LineCommentsAfterRightBrace) {
 TEST_F(FormatTestComments, ReflowsComments) {
   const auto Style20 = getLLVMStyleWithColumns(20);
   const auto Style22 = getLLVMStyleWithColumns(22);
+
   // Break a long line and reflow with the full next line.
   verifyFormat("// long long long\n"
                "// long long",
@@ -2149,11 +2075,9 @@ TEST_F(FormatTestComments, ReflowsComments) {
                Style20);
 
   // Don't break or reflow comments on import lines.
-  verifyFormat("#include \"t\" /* l l l\n"
-               "                * l */",
-               "#include \"t\" /* l l l\n"
-               "                * l */",
-               Style20);
+  verifyNoChange("#include \"t\" /* l l l\n"
+                 "                * l */",
+                 Style20);
 
   // Don't reflow between different trailing comment sections.
   verifyFormat("int i; // long long\n"
@@ -2209,7 +2133,9 @@ TEST_F(FormatTestComments, ReflowsCommentsPrecise) {
                "// some text that reflows\n"
                "// into   foo",
                Style);
+
   Style.ColumnLimit = 21;
+
   // Given one more column, "// reflows into   foo" does fit the limit, so we
   // do not compress the whitespace.
   verifyFormat("// some text that\n"
@@ -2228,6 +2154,7 @@ TEST_F(FormatTestComments, ReflowsCommentsPrecise) {
                "// some text that reflows\n"
                "// into1234567",
                Style);
+
   // Secondly, when the next line ends later, but the first word in that line
   // is precisely one column over the limit, do not reflow.
   verifyFormat("// some text that\n"
@@ -2240,6 +2167,7 @@ TEST_F(FormatTestComments, ReflowsCommentsPrecise) {
 
 TEST_F(FormatTestComments, ReflowsCommentsWithExtraWhitespace) {
   const auto Style16 = getLLVMStyleWithColumns(16);
+
   // Baseline.
   verifyFormat("// some text\n"
                "// that re flows",
@@ -2546,37 +2474,23 @@ TEST_F(FormatTestComments, BlockComments) {
 
   verifyFormat("void f(int * /* unused */) {}");
 
-  verifyFormat("/*\n"
-               " **\n"
-               " */",
-               "/*\n"
-               " **\n"
-               " */");
-  verifyFormat("/*\n"
-               " *q\n"
-               " */",
-               "/*\n"
-               " *q\n"
-               " */");
-  verifyFormat("/*\n"
-               " * q\n"
-               " */",
-               "/*\n"
-               " * q\n"
-               " */");
-  verifyFormat("/*\n"
-               " **/",
-               "/*\n"
-               " **/");
-  verifyFormat("/*\n"
-               " ***/",
-               "/*\n"
-               " ***/");
+  verifyNoChange("/*\n"
+                 " **\n"
+                 " */");
+  verifyNoChange("/*\n"
+                 " *q\n"
+                 " */");
+  verifyNoChange("/*\n"
+                 " * q\n"
+                 " */");
+  verifyNoChange("/*\n"
+                 " **/");
+  verifyNoChange("/*\n"
+                 " ***/");
 }
 
 TEST_F(FormatTestComments, BlockCommentsInMacros) {
   const auto Style20 = getLLVMStyleWithColumns(20);
-
   verifyFormat("#define A          \\\n"
                "  {                \\\n"
                "    /* one line */ \\\n"
@@ -2599,7 +2513,6 @@ TEST_F(FormatTestComments, BlockCommentsInMacros) {
 
 TEST_F(FormatTestComments, BlockCommentsAtEndOfLine) {
   const auto Style15 = getLLVMStyleWithColumns(15);
-
   verifyFormat("a = {\n"
                "    1111 /*    */\n"
                "};",
@@ -2770,11 +2683,6 @@ TEST_F(FormatTestComments, AlignTrailingComments) {
   // Align comment line sections aligned with the next token with the next
   // token.
   verifyFormat("class A {\n"
-               "public: // public comment\n"
-               "  // comment about a\n"
-               "  int a;\n"
-               "};",
-               "class A {\n"
                "public: // public comment\n"
                "  // comment about a\n"
                "  int a;\n"
@@ -3106,41 +3014,26 @@ TEST_F(FormatTestComments, AlignTrailingCommentsLeave) {
   FormatStyle Style = getLLVMStyle();
   Style.AlignTrailingComments.Kind = FormatStyle::TCAS_Leave;
 
-  verifyFormat("int a;// do not touch\n"
-               "int b; // any comments\n"
-               "int c;  // comment\n"
-               "int d;   // comment",
-               "int a;// do not touch\n"
-               "int b; // any comments\n"
-               "int c;  // comment\n"
-               "int d;   // comment",
-               Style);
+  verifyNoChange("int a;// do not touch\n"
+                 "int b; // any comments\n"
+                 "int c;  // comment\n"
+                 "int d;   // comment",
+                 Style);
 
-  verifyFormat("int a;   // do not touch\n"
-               "int b;  // any comments\n"
-               "int c; // comment\n"
-               "int d;// comment",
-               "int a;   // do not touch\n"
-               "int b;  // any comments\n"
-               "int c; // comment\n"
-               "int d;// comment",
-               Style);
+  verifyNoChange("int a;   // do not touch\n"
+                 "int b;  // any comments\n"
+                 "int c; // comment\n"
+                 "int d;// comment",
+                 Style);
 
-  verifyFormat("// do not touch\n"
-               "int a;  // any comments\n"
-               "\n"
-               "   // comment\n"
-               "// comment\n"
-               "\n"
-               "// comment",
-               "// do not touch\n"
-               "int a;  // any comments\n"
-               "\n"
-               "   // comment\n"
-               "// comment\n"
-               "\n"
-               "// comment",
-               Style);
+  verifyNoChange("// do not touch\n"
+                 "int a;  // any comments\n"
+                 "\n"
+                 "   // comment\n"
+                 "// comment\n"
+                 "\n"
+                 "// comment",
+                 Style);
 
   verifyFormat("// do not touch\n"
                "int a;  // any comments\n"
@@ -3178,23 +3071,15 @@ TEST_F(FormatTestComments, AlignTrailingCommentsLeave) {
 
   // Allow to keep 2 empty lines
   Style.MaxEmptyLinesToKeep = 2;
-  verifyFormat("// do not touch\n"
-               "int a;  // any comments\n"
-               "\n"
-               "\n"
-               "   // comment\n"
-               "// comment\n"
-               "\n"
-               "// comment",
-               "// do not touch\n"
-               "int a;  // any comments\n"
-               "\n"
-               "\n"
-               "   // comment\n"
-               "// comment\n"
-               "\n"
-               "// comment",
-               Style);
+  verifyNoChange("// do not touch\n"
+                 "int a;  // any comments\n"
+                 "\n"
+                 "\n"
+                 "   // comment\n"
+                 "// comment\n"
+                 "\n"
+                 "// comment",
+                 Style);
   Style.MaxEmptyLinesToKeep = 1;
 
   // Just format comments normally when leaving exceeds the column limit
@@ -3233,16 +3118,16 @@ TEST_F(FormatTestComments, DontAlignNamespaceComments) {
   Style.NamespaceMacros.push_back("TESTSUITE");
   Style.ShortNamespaceLines = 0;
 
-  StringRef Input = "namespace A {\n"
-                    "  TESTSUITE(B) {\n"
-                    "    namespace C {\n"
-                    "      namespace D { //\n"
-                    "      } // namespace D\n"
-                    "      std::string Foo = Bar; // Comment\n"
-                    "      std::string BazString = Baz;   // C2\n"
-                    "    }          // namespace C\n"
-                    "  }\n"
-                    "} // NaMeSpAcE A";
+  constexpr StringRef Input("namespace A {\n"
+                            "  TESTSUITE(B) {\n"
+                            "    namespace C {\n"
+                            "      namespace D { //\n"
+                            "      } // namespace D\n"
+                            "      std::string Foo = Bar; // Comment\n"
+                            "      std::string BazString = Baz;   // C2\n"
+                            "    }          // namespace C\n"
+                            "  }\n"
+                ...
[truncated]

Copy link
Contributor

@HazardyKnusperkeks HazardyKnusperkeks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just out of curiosity, with which tool did you check for the duplicated strings?

@owenca
Copy link
Contributor Author

owenca commented Nov 3, 2025

Just out of curiosity, with which tool did you check for the duplicated strings?

I wrote a quick and dirty Python script (when I started cleaning up FormatTest.cpp and other unittest files a few years ago). It basically uses regex.

@owenca owenca merged commit a9f0594 into llvm:main Nov 3, 2025
12 checks passed
@owenca owenca deleted the FormatTestComments branch November 3, 2025 06:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants