vglavnyy updated this revision to Diff 170663.
vglavnyy added a comment.
A twice-format problem: the format of format isn't format.
This commit surface an instability problem in clang-format at unit-tests level.
The patch adds double-checking format method for all test and adds a stub for
tests with the detected TF-problem.
https://reviews.llvm.org/D53482
Files:
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -36,9 +36,9 @@
SC_DoNotCheck
};
- std::string format(llvm::StringRef Code,
- const FormatStyle &Style = getLLVMStyle(),
- StatusCheck CheckComplete = SC_ExpectComplete) {
+ std::string format_weak(llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle(),
+ StatusCheck CheckComplete = SC_ExpectComplete) {
LLVM_DEBUG(llvm::errs() << "---\n");
LLVM_DEBUG(llvm::errs() << Code << "\n\n");
std::vector<tooling::Range> Ranges(1, tooling::Range(0, Code.size()));
@@ -57,6 +57,33 @@
return *Result;
}
+ // Format method with invariant check: format(Code)==format(format(Code)).
+ std::string format(llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle(),
+ StatusCheck CheckComplete = SC_ExpectComplete) {
+ const auto Formatted = format_weak(Code, Style, CheckComplete);
+ if (CheckComplete != SC_DoNotCheck) {
+ auto SaveReplacementCount = ReplacementCount;
+ EXPECT_EQ(Formatted, format_weak(Formatted, Style, CheckComplete));
+ ReplacementCount = SaveReplacementCount;
+ }
+ return Formatted;
+ }
+
+ // A stub-method for tests with broken invariant: format(Code)!=format(format(Code)).
+ // Will be redundant when all tests become stable.
+ std::string format_unstable(llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle(),
+ StatusCheck CheckComplete = SC_ExpectComplete) {
+ const auto Formatted = format_weak(Code, Style, CheckComplete);
+ if (CheckComplete != SC_DoNotCheck) {
+ auto SaveReplacementCount = ReplacementCount;
+ EXPECT_NE(Formatted, format_weak(Formatted, Style, CheckComplete));
+ ReplacementCount = SaveReplacementCount;
+ }
+ return Formatted;
+ }
+
FormatStyle getStyleWithColumns(FormatStyle Style, unsigned ColumnLimit) {
Style.ColumnLimit = ColumnLimit;
return Style;
@@ -8147,12 +8174,14 @@
TEST_F(FormatTest, BreaksStringLiterals) {
- EXPECT_EQ("\"some text \"\n"
- "\"other\";",
- format("\"some text other\";", getLLVMStyleWithColumns(12)));
- EXPECT_EQ("\"some text \"\n"
- "\"other\";",
- format("\\\n\"some text other\";", getLLVMStyleWithColumns(12)));
+ EXPECT_EQ(
+ "\"some text \"\n"
+ "\"other\";",
+ format_unstable("\"some text other\";", getLLVMStyleWithColumns(12)));
+ EXPECT_EQ(
+ "\"some text \"\n"
+ "\"other\";",
+ format_unstable("\\\n\"some text other\";", getLLVMStyleWithColumns(12)));
EXPECT_EQ(
"#define A \\\n"
" \"some \" \\\n"
@@ -8172,22 +8201,22 @@
format("\"some text\"", getLLVMStyleWithColumns(11)));
EXPECT_EQ("\"some \"\n"
"\"text\"",
- format("\"some text\"", getLLVMStyleWithColumns(10)));
+ format_unstable("\"some text\"", getLLVMStyleWithColumns(10)));
EXPECT_EQ("\"some \"\n"
"\"text\"",
- format("\"some text\"", getLLVMStyleWithColumns(7)));
+ format_unstable("\"some text\"", getLLVMStyleWithColumns(7)));
EXPECT_EQ("\"some\"\n"
"\" tex\"\n"
"\"t\"",
- format("\"some text\"", getLLVMStyleWithColumns(6)));
+ format_unstable("\"some text\"", getLLVMStyleWithColumns(6)));
EXPECT_EQ("\"some\"\n"
"\" tex\"\n"
"\" and\"",
- format("\"some tex and\"", getLLVMStyleWithColumns(6)));
+ format_unstable("\"some tex and\"", getLLVMStyleWithColumns(6)));
EXPECT_EQ("\"some\"\n"
"\"/tex\"\n"
"\"/and\"",
- format("\"some/tex/and\"", getLLVMStyleWithColumns(6)));
+ format_unstable("\"some/tex/and\"", getLLVMStyleWithColumns(6)));
EXPECT_EQ("variable =\n"
" \"long string \"\n"
@@ -8237,34 +8266,32 @@
" aaaaaaaaaaaaaaaaaaaa,\n"
" aaaaaa(\"aaa aaaaa aaa aaa aaaaa aaa aaaaa aaa aaa aaaaaa\"));");
- EXPECT_EQ("\"splitmea\"\n"
- "\"trandomp\"\n"
- "\"oint\"",
- format("\"splitmeatrandompoint\"", getLLVMStyleWithColumns(10)));
+ EXPECT_EQ(
+ "\"splitmea\"\n"
+ "\"trandomp\"\n"
+ "\"oint\"",
+ format_unstable("\"splitmeatrandompoint\"", getLLVMStyleWithColumns(10)));
- EXPECT_EQ("\"split/\"\n"
- "\"pathat/\"\n"
- "\"slashes\"",
- format("\"split/pathat/slashes\"", getLLVMStyleWithColumns(10)));
+ EXPECT_EQ(
+ "\"split/\"\n"
+ "\"pathat/\"\n"
+ "\"slashes\"",
+ format_unstable("\"split/pathat/slashes\"", getLLVMStyleWithColumns(10)));
- EXPECT_EQ("\"split/\"\n"
- "\"pathat/\"\n"
- "\"slashes\"",
- format("\"split/pathat/slashes\"", getLLVMStyleWithColumns(10)));
EXPECT_EQ("\"split at \"\n"
"\"spaces/at/\"\n"
"\"slashes.at.any$\"\n"
"\"non-alphanumeric%\"\n"
"\"1111111111characte\"\n"
"\"rs\"",
- format("\"split at "
- "spaces/at/"
- "slashes.at."
- "any$non-"
- "alphanumeric%"
- "1111111111characte"
- "rs\"",
- getLLVMStyleWithColumns(20)));
+ format_unstable("\"split at "
+ "spaces/at/"
+ "slashes.at."
+ "any$non-"
+ "alphanumeric%"
+ "1111111111characte"
+ "rs\"",
+ getLLVMStyleWithColumns(20)));
// Verify that splitting the strings understands
// Style::AlwaysBreakBeforeMultilineStrings.
@@ -8325,27 +8352,29 @@
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
"(\n"
" \"x\t\");",
- format("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
- "aaaaaaa("
- "\"x\t\");"));
+ format_unstable(
+ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+ "aaaaaaa("
+ "\"x\t\");"));
}
TEST_F(FormatTest, BreaksWideAndNSStringLiterals) {
- EXPECT_EQ(
- "u8\"utf8 string \"\n"
- "u8\"literal\";",
- format("u8\"utf8 string literal\";", getGoogleStyleWithColumns(16)));
- EXPECT_EQ(
- "u\"utf16 string \"\n"
- "u\"literal\";",
- format("u\"utf16 string literal\";", getGoogleStyleWithColumns(16)));
- EXPECT_EQ(
- "U\"utf32 string \"\n"
- "U\"literal\";",
- format("U\"utf32 string literal\";", getGoogleStyleWithColumns(16)));
+ EXPECT_EQ("u8\"utf8 string \"\n"
+ "u8\"literal\";",
+ format_unstable("u8\"utf8 string literal\";",
+ getGoogleStyleWithColumns(16)));
+ EXPECT_EQ("u\"utf16 string \"\n"
+ "u\"literal\";",
+ format_unstable("u\"utf16 string literal\";",
+ getGoogleStyleWithColumns(16)));
+ EXPECT_EQ("U\"utf32 string \"\n"
+ "U\"literal\";",
+ format_unstable("U\"utf32 string literal\";",
+ getGoogleStyleWithColumns(16)));
EXPECT_EQ("L\"wide string \"\n"
"L\"literal\";",
- format("L\"wide string literal\";", getGoogleStyleWithColumns(16)));
+ format_unstable("L\"wide string literal\";",
+ getGoogleStyleWithColumns(16)));
EXPECT_EQ("@\"NSString \"\n"
"@\"literal\";",
format("@\"NSString literal\";", getGoogleStyleWithColumns(19)));
@@ -8370,11 +8399,11 @@
TEST_F(FormatTest, BreaksStringLiteralsWithin_TMacro) {
FormatStyle Style = getLLVMStyleWithColumns(20);
- EXPECT_EQ(
- "_T(\"aaaaaaaaaaaaaa\")\n"
- "_T(\"aaaaaaaaaaaaaa\")\n"
- "_T(\"aaaaaaaaaaaa\")",
- format(" _T(\"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\")", Style));
+ EXPECT_EQ("_T(\"aaaaaaaaaaaaaa\")\n"
+ "_T(\"aaaaaaaaaaaaaa\")\n"
+ "_T(\"aaaaaaaaaaaa\")",
+ format_unstable(
+ " _T(\"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\")", Style));
EXPECT_EQ("f(x,\n"
" _T(\"aaaaaaaaaaaa\")\n"
" _T(\"aaa\"),\n"
@@ -8624,22 +8653,22 @@
EXPECT_EQ("\"\\\"", format("\"\\\"", getLLVMStyleWithColumns(2)));
EXPECT_EQ("\"test\"\n"
"\"\\n\"",
- format("\"test\\n\"", getLLVMStyleWithColumns(7)));
+ format_unstable("\"test\\n\"", getLLVMStyleWithColumns(7)));
EXPECT_EQ("\"tes\\\\\"\n"
"\"n\"",
- format("\"tes\\\\n\"", getLLVMStyleWithColumns(7)));
+ format_unstable("\"tes\\\\n\"", getLLVMStyleWithColumns(7)));
EXPECT_EQ("\"\\\\\\\\\"\n"
"\"\\n\"",
- format("\"\\\\\\\\\\n\"", getLLVMStyleWithColumns(7)));
+ format_unstable("\"\\\\\\\\\\n\"", getLLVMStyleWithColumns(7)));
EXPECT_EQ("\"\\uff01\"", format("\"\\uff01\"", getLLVMStyleWithColumns(7)));
EXPECT_EQ("\"\\uff01\"\n"
"\"test\"",
- format("\"\\uff01test\"", getLLVMStyleWithColumns(8)));
+ format_unstable("\"\\uff01test\"", getLLVMStyleWithColumns(8)));
EXPECT_EQ("\"\\Uff01ff02\"",
format("\"\\Uff01ff02\"", getLLVMStyleWithColumns(11)));
EXPECT_EQ("\"\\x000000000001\"\n"
"\"next\"",
- format("\"\\x000000000001next\"", getLLVMStyleWithColumns(16)));
+ format_unstable("\"\\x000000000001next\"", getLLVMStyleWithColumns(16)));
EXPECT_EQ("\"\\x000000000001next\"",
format("\"\\x000000000001next\"", getLLVMStyleWithColumns(15)));
EXPECT_EQ("\"\\x000000000001\"",
@@ -8647,11 +8676,11 @@
EXPECT_EQ("\"test\"\n"
"\"\\000000\"\n"
"\"000001\"",
- format("\"test\\000000000001\"", getLLVMStyleWithColumns(9)));
+ format_unstable("\"test\\000000000001\"", getLLVMStyleWithColumns(9)));
EXPECT_EQ("\"test\\000\"\n"
"\"00000000\"\n"
"\"1\"",
- format("\"test\\000000000001\"", getLLVMStyleWithColumns(10)));
+ format_unstable("\"test\\000000000001\"", getLLVMStyleWithColumns(10)));
}
TEST_F(FormatTest, DoNotCreateUnreasonableUnwrappedLines) {
@@ -10702,18 +10731,18 @@
EXPECT_EQ("// foo bar baz bazfoo\n"
"// foo bar baz bazfoo\n"
"// bar foo bar\n",
- format("// foo bar baz bazfoo\n"
- "// foo bar baz bazfoo bar\n"
- "// foo bar\n",
- Style));
+ format_unstable("// foo bar baz bazfoo\n"
+ "// foo bar baz bazfoo bar\n"
+ "// foo bar\n",
+ Style));
EXPECT_EQ("// foo bar baz bazfoo\n"
"// foo bar baz bazfoo\n"
"// bar foo bar\n",
- format("// foo bar baz bazfoo\n"
- "// foo bar baz bazfoo bar\n"
- "// foo bar\n",
- Style));
+ format_unstable("// foo bar baz bazfoo\n"
+ "// foo bar baz bazfoo bar\n"
+ "// foo bar\n",
+ Style));
// Make sure we do not keep protruding characters if strict mode reflow is
// cheaper than keeping protruding characters.
@@ -11326,10 +11355,11 @@
"\"\xf1\xf2\xf3\xe4\xb8\xed\xf3\xfe \"\n"
"\"\xe7\xe8\xec\xed\xfe\xfe \"\n"
"\"\xef\xee\xf0\xf3...\"",
- format("\"\xce\xe4\xed\xe0\xe6\xe4\xfb \xe2 "
- "\xf1\xf2\xf3\xe4\xb8\xed\xf3\xfe \xe7\xe8\xec\xed\xfe\xfe "
- "\xef\xee\xf0\xf3...\"",
- getLLVMStyleWithColumns(12)));
+ format_unstable(
+ "\"\xce\xe4\xed\xe0\xe6\xe4\xfb \xe2 "
+ "\xf1\xf2\xf3\xe4\xb8\xed\xf3\xfe \xe7\xe8\xec\xed\xfe\xfe "
+ "\xef\xee\xf0\xf3...\"",
+ getLLVMStyleWithColumns(12)));
}
TEST_F(FormatTest, HandlesUTF8BOM) {
@@ -11364,22 +11394,23 @@
// (e.g. "<8d>"), so there's no single correct way to handle them.
EXPECT_EQ("\"aaaaÄ\"\n"
"\"\xc2\x8d\";",
- format("\"aaaaÄ\xc2\x8d\";", getLLVMStyleWithColumns(10)));
- EXPECT_EQ("\"aaaaaaaÄ\"\n"
- "\"\xc2\x8d\";",
- format("\"aaaaaaaÄ\xc2\x8d\";", getLLVMStyleWithColumns(10)));
+ format_unstable("\"aaaaÄ\xc2\x8d\";", getLLVMStyleWithColumns(10)));
+ EXPECT_EQ(
+ "\"aaaaaaaÄ\"\n"
+ "\"\xc2\x8d\";",
+ format_unstable("\"aaaaaaaÄ\xc2\x8d\";", getLLVMStyleWithColumns(10)));
EXPECT_EQ("\"Однажды, в \"\n"
"\"студёную \"\n"
"\"зимнюю \"\n"
"\"пору,\"",
- format("\"Однажды, в студёную зимнюю пору,\"",
- getLLVMStyleWithColumns(13)));
- EXPECT_EQ(
- "\"一 二 三 \"\n"
- "\"四 五六 \"\n"
- "\"七 八 九 \"\n"
- "\"十\"",
- format("\"一 二 三 四 五六 七 八 九 十\"", getLLVMStyleWithColumns(11)));
+ format_unstable("\"Однажды, в студёную зимнюю пору,\"",
+ getLLVMStyleWithColumns(13)));
+ EXPECT_EQ("\"一 二 三 \"\n"
+ "\"四 五六 \"\n"
+ "\"七 八 九 \"\n"
+ "\"十\"",
+ format_unstable("\"一 二 三 四 五六 七 八 九 十\"",
+ getLLVMStyleWithColumns(11)));
EXPECT_EQ("\"一\t\"\n"
"\"二 \t\"\n"
"\"三 四 \"\n"
@@ -11387,13 +11418,14 @@
"\"六 \t\"\n"
"\"七 \"\n"
"\"八九十\tqq\"",
- format("\"一\t二 \t三 四 五\t六 \t七 八九十\tqq\"",
- getLLVMStyleWithColumns(11)));
+ format_unstable("\"一\t二 \t三 四 五\t六 \t七 八九十\tqq\"",
+ getLLVMStyleWithColumns(11)));
// UTF8 character in an escape sequence.
- EXPECT_EQ("\"aaaaaa\"\n"
- "\"\\\xC2\x8D\"",
- format("\"aaaaaa\\\xC2\x8D\"", getLLVMStyleWithColumns(10)));
+ EXPECT_EQ(
+ "\"aaaaaa\"\n"
+ "\"\\\xC2\x8D\"",
+ format_unstable("\"aaaaaa\\\xC2\x8D\"", getLLVMStyleWithColumns(10)));
}
TEST_F(FormatTest, HandlesDoubleWidthCharsInMultiLineStrings) {
@@ -12086,7 +12118,7 @@
getLLVMStyle()));
EXPECT_EQ("\"aaaaaaa \"\r\n"
"\"bbbbbbb\";\r\n",
- format("\"aaaaaaa bbbbbbb\";\r\n", getLLVMStyleWithColumns(10)));
+ format_unstable("\"aaaaaaa bbbbbbb\";\r\n", getLLVMStyleWithColumns(10)));
EXPECT_EQ("#define A \\\r\n"
" b; \\\r\n"
" c; \\\r\n"
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits