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
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to