[PATCH] D15639: [clang-format] Ensure Sort include is stable with negative Priority

[Jean-Philippe Dufraigne via cfe-commits]

jeanphilippeD created this revision.
jeanphilippeD added a reviewer: djasper.
jeanphilippeD added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

The new sort include with negative priority was not stable when running it again over the updated includes.

Extend the test NegativePriorities to have an extra header. Test pas still passing
Add a new assertion to test that the sorting is stable. This new assertion failed.

From this:
#include "important_os_header.h"
#include "c_main_header.h"
#include "a_other_header.h"

Before fix:
#include "important_os_header.h"
#include "a_other_header.h"
#include "c_main_header.h"

After fix:
#include "important_os_header.h"
#include "c_main_header.h"
#include "a_other_header.h"

http://reviews.llvm.org/D15639

Files:
  lib/Format/Format.cpp
  unittests/Format/SortIncludesTest.cpp

Index: unittests/Format/SortIncludesTest.cpp
===================================================================
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -188,9 +188,19 @@
 TEST_F(SortIncludesTest, NegativePriorities) {
   Style.IncludeCategories = {{".*important_os_header.*", -1}, {".*", 1}};
   EXPECT_EQ("#include \"important_os_header.h\"\n"
-            "#include \"a.h\"\n",
-            sort("#include \"a.h\"\n"
+            "#include \"c_main_header.h\"\n"
+            "#include \"a_other_header.h\"\n",
+            sort("#include \"c_main_header.h\"\n"
+                 "#include \"a_other_header.h\"\n"
                  "#include \"important_os_header.h\"\n"));
+
+  // check stable when re-run
+  EXPECT_EQ("#include \"important_os_header.h\"\n"
+            "#include \"c_main_header.h\"\n"
+            "#include \"a_other_header.h\"\n",
+            sort("#include \"important_os_header.h\"\n"
+                 "#include \"c_main_header.h\"\n"
+                 "#include \"a_other_header.h\"\n"));
 }
 
 TEST_F(SortIncludesTest, CalculatesCorrectCursorPosition) {
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1807,19 +1807,21 @@
     if (!FormattingOff && !Line.endswith("\\")) {
       if (IncludeRegex.match(Line, &Matches)) {
         StringRef IncludeName = Matches[2];
-        int Category;
-        if (LookForMainHeader && !IncludeName.startswith("<")) {
-          Category = 0;
-        } else {
-          Category = INT_MAX;
-          for (unsigned i = 0, e = CategoryRegexs.size(); i != e; ++i) {
+        int Category = INT_MAX;
+        for (unsigned i = 0, e = CategoryRegexs.size(); i != e; ++i) {
             if (CategoryRegexs[i].match(IncludeName)) {
-              Category = Style.IncludeCategories[i].Priority;
-              break;
+                Category = Style.IncludeCategories[i].Priority;
+                break;
             }
-          }
         }
-        LookForMainHeader = false;
+
+        if (Category >= 0 ) {
+            if (LookForMainHeader && !IncludeName.startswith("<")) {
+                Category = 0;
+            }
+            LookForMainHeader = false;
+        }
+
         IncludesInBlock.push_back({IncludeName, Line, Prev, Category});
       } else if (!IncludesInBlock.empty()) {
         sortIncludes(Style, IncludesInBlock, Ranges, FileName, Replaces,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D15639.43221.patch
Type: text/x-patch
Size: 2493 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151218/e470075a/attachment.bin>