-
Notifications
You must be signed in to change notification settings - Fork 15k
[clang-format][NFC] Port FormatTestComments to verifyFormat #164310
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
And reduce the number of getLLVMStyleWithColumnLimit calls.
|
@llvm/pr-subscribers-clang-format Author: Björn Schäpers (HazardyKnusperkeks) ChangesAnd reduce the number of getLLVMStyleWithColumnLimit calls. Patch is 255.92 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/164310.diff 1 Files Affected:
diff --git a/clang/unittests/Format/FormatTestComments.cpp b/clang/unittests/Format/FormatTestComments.cpp
index fc80bf4024fd9..a9bd5d0cfa521 100644
--- a/clang/unittests/Format/FormatTestComments.cpp
+++ b/clang/unittests/Format/FormatTestComments.cpp
@@ -24,18 +24,20 @@ class FormatTestComments : public FormatTestBase {};
//===----------------------------------------------------------------------===//
TEST_F(FormatTestComments, UnderstandsSingleLineComments) {
+ const auto Style20 = getLLVMStyleWithColumns(20);
+
verifyFormat("//* */");
verifyFormat("// line 1\n"
"// line 2\n"
"void f() {}");
- EXPECT_EQ("// comment", format("//comment"));
- EXPECT_EQ("// #comment", format("//#comment"));
+ verifyFormat("// comment", "//comment");
+ verifyFormat("// #comment", "//#comment");
- EXPECT_EQ("// comment\n"
- "// clang-format on",
- format("//comment\n"
- "// clang-format on"));
+ verifyFormat("// comment\n"
+ "// clang-format on",
+ "//comment\n"
+ "// clang-format on");
verifyFormat("void f() {\n"
" // Doesn't do anything\n"
@@ -84,11 +86,11 @@ TEST_F(FormatTestComments, UnderstandsSingleLineComments) {
"#include \"a/b/c\" // comment");
verifyFormat("#include <a> // comment\n"
"#include <a/b/c> // comment");
- EXPECT_EQ("#include \"a\" // comment\n"
- "#include \"a/b/c\" // comment",
- format("#include \\\n"
- " \"a\" // comment\n"
- "#include \"a/b/c\" // comment"));
+ verifyFormat("#include \"a\" // comment\n"
+ "#include \"a/b/c\" // comment",
+ "#include \\\n"
+ " \"a\" // comment\n"
+ "#include \"a/b/c\" // comment");
verifyFormat("enum E {\n"
" // comment\n"
@@ -96,63 +98,63 @@ TEST_F(FormatTestComments, UnderstandsSingleLineComments) {
" VAL_B\n"
"};");
- EXPECT_EQ("enum A {\n"
- " // line a\n"
- " a,\n"
- " b, // line b\n"
- "\n"
- " // line c\n"
- " c\n"
- "};",
- format("enum A {\n"
- " // line a\n"
- " a,\n"
- " b, // line b\n"
- "\n"
- " // line c\n"
- " c\n"
- "};",
- getLLVMStyleWithColumns(20)));
- EXPECT_EQ("enum A {\n"
- " a, // line 1\n"
- " // line 2\n"
- "};",
- format("enum A {\n"
- " a, // line 1\n"
- " // line 2\n"
- "};",
- getLLVMStyleWithColumns(20)));
- EXPECT_EQ("enum A {\n"
- " a, // line 1\n"
- " // line 2\n"
- "};",
- format("enum A {\n"
- " a, // line 1\n"
- " // line 2\n"
- "};",
- getLLVMStyleWithColumns(20)));
- EXPECT_EQ("enum A {\n"
- " a, // line 1\n"
- " // line 2\n"
- " b\n"
- "};",
- format("enum A {\n"
- " a, // line 1\n"
- " // line 2\n"
- " b\n"
- "};",
- getLLVMStyleWithColumns(20)));
- EXPECT_EQ("enum A {\n"
- " a, // line 1\n"
- " // line 2\n"
- " b\n"
- "};",
- format("enum A {\n"
- " a, // line 1\n"
- " // line 2\n"
- " b\n"
- "};",
- 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"
+ "\n"
+ " // line c\n"
+ " 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);
+ verifyFormat("enum A {\n"
+ " a, // line 1\n"
+ " // line 2\n"
+ "};",
+ "enum A {\n"
+ " a, // line 1\n"
+ " // 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);
+ 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);
verifyFormat(
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa =\n"
" bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb; // Trailing comment");
@@ -172,28 +174,28 @@ TEST_F(FormatTestComments, UnderstandsSingleLineComments) {
verifyFormat("int aaaa; // aaaaa\n"
"int aa; // aaaaaaa",
- getLLVMStyleWithColumns(20));
+ Style20);
- EXPECT_EQ("void f() { // This does something ..\n"
- "}\n"
- "int a; // This is unrelated",
- format("void f() { // This does something ..\n"
- " }\n"
- "int a; // This is unrelated"));
- EXPECT_EQ("class C {\n"
- " void f() { // This does something ..\n"
- " } // awesome..\n"
- "\n"
- " int a; // This is unrelated\n"
- "};",
- format("class C{void f() { // This does something ..\n"
- " } // awesome..\n"
- " \n"
- "int a; // This is unrelated\n"
- "};"));
-
- EXPECT_EQ("int i; // single line trailing comment",
- format("int i;\\\n// single line trailing comment"));
+ verifyFormat("void f() { // This does something ..\n"
+ "}\n"
+ "int a; // This is unrelated",
+ "void f() { // This does something ..\n"
+ " }\n"
+ "int a; // This is unrelated");
+ verifyFormat("class C {\n"
+ " void f() { // This does something ..\n"
+ " } // awesome..\n"
+ "\n"
+ " int a; // This is unrelated\n"
+ "};",
+ "class C{void f() { // This does something ..\n"
+ " } // awesome..\n"
+ " \n"
+ "int a; // This is unrelated\n"
+ "};");
+
+ verifyFormat("int i; // single line trailing comment",
+ "int i;\\\n// single line trailing comment");
verifyGoogleFormat("int a; // Trailing comment.");
@@ -210,99 +212,99 @@ TEST_F(FormatTestComments, UnderstandsSingleLineComments) {
verifyGoogleFormat(
"aaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
" aaaaaaaaaaaaaaaaaaaaaa); // 81_cols_with_this_comment");
- EXPECT_EQ("D(a, {\n"
- " // test\n"
- " int a;\n"
- "});",
- format("D(a, {\n"
- "// test\n"
- "int a;\n"
- "});"));
-
- EXPECT_EQ("lineWith(); // comment\n"
- "// at start\n"
- "otherLine();",
- format("lineWith(); // comment\n"
- "// at start\n"
- "otherLine();"));
- EXPECT_EQ("lineWith(); // comment\n"
- "/*\n"
- " * at start */\n"
- "otherLine();",
- format("lineWith(); // comment\n"
- "/*\n"
- " * at start */\n"
- "otherLine();"));
- EXPECT_EQ("lineWith(); // comment\n"
- " // at start\n"
- "otherLine();",
- format("lineWith(); // comment\n"
- " // at start\n"
- "otherLine();"));
-
- EXPECT_EQ("lineWith(); // comment\n"
- "// at start\n"
- "otherLine(); // comment",
- format("lineWith(); // comment\n"
- "// at start\n"
- "otherLine(); // comment"));
- EXPECT_EQ("lineWith();\n"
- "// at start\n"
- "otherLine(); // comment",
- format("lineWith();\n"
- " // at start\n"
- "otherLine(); // comment"));
- EXPECT_EQ("// first\n"
- "// at start\n"
- "otherLine(); // comment",
- format("// first\n"
- " // at start\n"
- "otherLine(); // comment"));
- EXPECT_EQ("f();\n"
- "// first\n"
- "// at start\n"
- "otherLine(); // comment",
- format("f();\n"
- "// first\n"
- " // at start\n"
- "otherLine(); // comment"));
+ verifyFormat("D(a, {\n"
+ " // test\n"
+ " int a;\n"
+ "});",
+ "D(a, {\n"
+ "// test\n"
+ "int a;\n"
+ "});");
+
+ verifyFormat("lineWith(); // comment\n"
+ "// at start\n"
+ "otherLine();",
+ "lineWith(); // comment\n"
+ "// at start\n"
+ "otherLine();");
+ verifyFormat("lineWith(); // comment\n"
+ "/*\n"
+ " * at start */\n"
+ "otherLine();",
+ "lineWith(); // comment\n"
+ "/*\n"
+ " * at start */\n"
+ "otherLine();");
+ verifyFormat("lineWith(); // comment\n"
+ " // at start\n"
+ "otherLine();",
+ "lineWith(); // comment\n"
+ " // at start\n"
+ "otherLine();");
+
+ verifyFormat("lineWith(); // comment\n"
+ "// at start\n"
+ "otherLine(); // comment",
+ "lineWith(); // comment\n"
+ "// at start\n"
+ "otherLine(); // comment");
+ verifyFormat("lineWith();\n"
+ "// at start\n"
+ "otherLine(); // comment",
+ "lineWith();\n"
+ " // at start\n"
+ "otherLine(); // comment");
+ verifyFormat("// first\n"
+ "// at start\n"
+ "otherLine(); // comment",
+ "// first\n"
+ " // at start\n"
+ "otherLine(); // comment");
+ verifyFormat("f();\n"
+ "// first\n"
+ "// at start\n"
+ "otherLine(); // comment",
+ "f();\n"
+ "// first\n"
+ " // at start\n"
+ "otherLine(); // comment");
verifyFormat("f(); // comment\n"
"// first\n"
"// at start\n"
"otherLine();");
- EXPECT_EQ("f(); // comment\n"
- "// first\n"
- "// at start\n"
- "otherLine();",
- format("f(); // comment\n"
- "// first\n"
- " // at start\n"
- "otherLine();"));
- EXPECT_EQ("f(); // comment\n"
- " // first\n"
- "// at start\n"
- "otherLine();",
- format("f(); // comment\n"
- " // first\n"
- "// at start\n"
- "otherLine();"));
- EXPECT_EQ("void f() {\n"
- " lineWith(); // comment\n"
- " // at start\n"
- "}",
- format("void f() {\n"
- " lineWith(); // comment\n"
- " // at start\n"
- "}"));
- EXPECT_EQ("int xy; // a\n"
- "int z; // b",
- format("int xy; // a\n"
- "int z; //b"));
- EXPECT_EQ("int xy; // a\n"
- "int z; // bb",
- format("int xy; // a\n"
- "int z; //bb",
- getLLVMStyleWithColumns(12)));
+ verifyFormat("f(); // comment\n"
+ "// first\n"
+ "// at start\n"
+ "otherLine();",
+ "f(); // comment\n"
+ "// first\n"
+ " // at start\n"
+ "otherLine();");
+ verifyFormat("f(); // comment\n"
+ " // first\n"
+ "// at start\n"
+ "otherLine();",
+ "f(); // comment\n"
+ " // first\n"
+ "// at start\n"
+ "otherLine();");
+ verifyFormat("void f() {\n"
+ " lineWith(); // comment\n"
+ " // at start\n"
+ "}",
+ "void f() {\n"
+ " lineWith(); // comment\n"
+ " // at start\n"
+ "}");
+ verifyFormat("int xy; // a\n"
+ "int z; // b",
+ "int xy; // a\n"
+ "int z; //b");
+ verifyFormat("int xy; // a\n"
+ "int z; // bb",
+ "int xy; // a\n"
+ "int z; //bb",
+ getLLVMStyleWithColumns(12));
verifyFormat("#define A \\\n"
" int i; /* iiiiiiiiiiiiiiiiiiiii */ \\\n"
@@ -317,14 +319,14 @@ TEST_F(FormatTestComments, UnderstandsSingleLineComments) {
verifyFormat("if ( // This is some comment\n"
" x + 3) {\n"
"}");
- EXPECT_EQ("if ( // This is some comment\n"
- " // spanning two lines\n"
- " x + 3) {\n"
- "}",
- format("if( // This is some comment\n"
- " // spanning two lines\n"
- " x + 3) {\n"
- "}"));
+ verifyFormat("if ( // This is some comment\n"
+ " // spanning two lines\n"
+ " x + 3) {\n"
+ "}",
+ "if( // This is some comment\n"
+ " // spanning two lines\n"
+ " x + 3) {\n"
+ "}");
verifyNoCrash("/\\\n/");
verifyNoCrash("/\\\n* */");
@@ -333,35 +335,35 @@ TEST_F(FormatTestComments, UnderstandsSingleLineComments) {
}
TEST_F(FormatTestComments, KeepsParameterWithTrailingCommentsOnTheirOwnLine) {
- EXPECT_EQ("SomeFunction(a,\n"
- " b, // comment\n"
- " c);",
- format("SomeFunction(a,\n"
- " b, // comment\n"
- " c);"));
- EXPECT_EQ("SomeFunction(a, b,\n"
- " // comment\n"
- " c);",
- format("SomeFunction(a,\n"
- " b,\n"
- " // comment\n"
- " c);"));
- EXPECT_EQ("SomeFunction(a, b, // comment (unclear relation)\n"
- " c);",
- format("SomeFunction(a, b, // comment (unclear relation)\n"
- " c);"));
- EXPECT_EQ("SomeFunction(a, // comment\n"
- " b,\n"
- " c); // comment",
- format("SomeFunction(a, // comment\n"
- " b,\n"
- " c); // comment"));
- EXPECT_EQ("aaaaaaaaaa(aaaa(aaaa,\n"
- " aaaa), //\n"
- " aaaa, bbbbb);",
- format("aaaaaaaaaa(aaaa(aaaa,\n"
- "aaaa), //\n"
- "aaaa, bbbbb);"));
+ verifyFormat("SomeFunction(a,\n"
+ " b, // comment\n"
+ " c);",
+ "SomeFunction(a,\n"
+ " b, // comment\n"
+ " c);");
+ verifyFormat("SomeFunction(a, b,\n"
+ " // comment\n"
+ " c);",
+ "SomeFunction(a,\n"
+ " b,\n"
+ " // comment\n"
+ " c);");
+ verifyFormat("SomeFunction(a, b, // comment (unclear relation)\n"
+ " c);",
+ "SomeFunction(a, b, // comment (unclear relation)\n"
+ " c);");
+ verifyFormat("SomeFunction(a, // comment\n"
+ " b,\n"
+ " c); // comment",
+ "SomeFunction(a, // comment\n"
+ " b,\n"
+ " c); // comment");
+ verifyFormat("aaaaaaaaaa(aaaa(aaaa,\n"
+ " aaaa), //\n"
+ " aaaa, bbbbb);",
+ "aaaaaaaaaa(aaaa(aaaa,\n"
+ "aaaa), //\n"
+ "aaaa, bbbbb);");
FormatStyle BreakAlways = getLLVMStyle();
BreakAlways.BinPackParameters = FormatStyle::BPPS_AlwaysOnePerLine;
@@ -378,12 +380,12 @@ TEST_F(FormatTestComments, KeepsParameterWithTrailingCommentsOnTheirOwnLine) {
}
TEST_F(FormatTestComments, RemovesTrailingWhitespaceOfComments) {
- EXPECT_EQ("// comment", format("// comment "));
- EXPECT_EQ("int aaaaaaa, bbbbbbb; // comment",
- format("int aaaaaaa, bbbbbbb; // comment ",
- getLLVMStyleWithColumns(33)));
- EXPECT_EQ("// comment\\\n", format("// comment\\\n \t \v \f "));
- EXPECT_EQ("// comment \\\n", format("// comment \\\n \t \v \f "));
+ verifyFormat("// comment", "// comment ");
+ verifyFormat("int aaaaaaa, bbbbbbb; // comment",
+ "int aaaaaaa, bbbbbbb; // comment ",
+ getLLVMStyleWithColumns(33));
+ verifyFormat("// comment\\\n", "// comment\\\n \t \v \f ");
+ verifyFormat("// comment \\\n", "// comment \\\n \t \v \f ");
}
TEST_F(FormatTestComments, UnderstandsBlockComments) {
@@ -393,16 +395,15 @@ TEST_F(FormatTestComments, UnderstandsBlockComments) {
" /*qq_=*/move(q), [this, b](bar<void(uint32_t)> b) {},\n"
" c);",
getLLVMStyleWithColumns(60));
- EXPECT_EQ("f(aaaaaaaaaaaaaaaaaaaaaaaaa, /* Trailing comment for aa... */\n"
- " bbbbbbbbbbbbbbbbbbbbbbbbb);",
- format("f(aaaaaaaaaaaaaaaaaaaaaaaaa , \\\n"
- "/* Trailing comment for aa... */\n"
- " bbbbbbbbbbbbbbbbbbbbbbbbb);"));
- EXPECT_EQ(
- "f(aaaaaaaaaaaaaaaaaaaaaaaaa,\n"
- " /* Leading comment for bb... */ bbbbbbbbbbbbbbbbbbbbbbbbb);",
- format("f(aaaaaaaaaaaaaaaaaaaaaaaaa , \n"
- "/* Leading comment for bb... */ bbbbbbbbbbbbbbbbbbbbbbbbb);"));
+ verifyFormat("f(aaaaaaaaaaaaaaaaaaaaaaaaa, /* Trailing comment for aa... */\n"
+ " bbbbbbbbbbbbbbbbbbbbbbbbb);",
+ "f(aaaaaaaaaaaaaaaaaaaaaaaaa , \\\n"
+ "/* Trailing comment for aa... */\n"
+ " bbbbbbbbbbbbbbbbbbbbbbbbb);");
+ verifyFormat("f(aaaaaaaaaaaaaaaaaaaaaaaaa,\n"
+ " /* Leading comment for bb... */ bbbbbbbbbbbbbbbbbbbbbbbbb);",
+ "f(aaaaaaaaaaaaaaaaaaaaaaaaa , \n"
+ "/* Leading comment for bb... */ bbbbbbbbbbbbbbbbbbbbbbbbb);");
verifyFormat(
"void aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
@@ -445,77 +446,77 @@ TEST_F(FormatTestComments, UnderstandsBlockComments) {
}
TEST_F(FormatTestComments, AlignsBlockComments) {
- EXPECT_EQ("/*\n"
- " * Really multi-line\n"
- " * comment.\n"
- " */\n"
- "void f() {}",
- format(" /*\n"
- " * Really multi-line\n"
- " * comment.\n"
- " */\n"
- " void f() {}"));
- EXPECT_EQ("class C {\n"
- " /*\n"
- ...
[truncated]
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only reviewed the "reducing the number of get...StyleWithColumns calls" part, which should really be split to a separate patch. Oh well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change StringRef foo = ... to StringRef foo(...) as suggested?
463bab5 to
b4f2a6d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are probably 30+ two (or three if you count Style) parameter version of verifyFormat that can be replaced with the one-parameter version or with verifyNoChange, but they can be done in a follow-up patch.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/34180 Here is the relevant piece of the build log for the reference |
Done in #166029. |
And reduce the number of getLLVMStyleWithColumnLimit calls.