[clangd][IncludeCleaner] Use a proper comparator for deduplicating fi…

…ndings Previous one didn't have strict weak ordering guarantees.
llvm · May 1, 2023 · 5e74a3d · 5e74a3d
1 parent 6875424
commit 5e74a3d
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 9 deletions.
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -422,13 +422,17 @@ IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) {
   // times.
   llvm::stable_sort(MissingIncludes, [](const MissingIncludeDiagInfo &LHS,
                                         const MissingIncludeDiagInfo &RHS) {
-    if (LHS.Symbol == RHS.Symbol) {
+    // First sort by reference location.
+    if (LHS.SymRefRange != RHS.SymRefRange) {
       // We can get away just by comparing the offsets as all the ranges are in
       // main file.
       return LHS.SymRefRange.beginOffset() < RHS.SymRefRange.beginOffset();
     }
-    // If symbols are different we don't care about the ordering.
-    return true;
+    // For the same location, break ties using the symbol. Note that this won't
+    // be stable across runs.
+    using MapInfo = llvm::DenseMapInfo<include_cleaner::Symbol>;
+    return MapInfo::getHashValue(LHS.Symbol) <
+           MapInfo::getHashValue(RHS.Symbol);
   });
   MissingIncludes.erase(llvm::unique(MissingIncludes), MissingIncludes.end());
   std::vector<const Inclusion *> UnusedIncludes =

diff --git a/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test b/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
@@ -33,14 +33,14 @@
 # CHECK-NEXT:     "diagnostics": [
 # CHECK-NEXT:       {
 # CHECK-NEXT:         "code": "missing-includes",
-# CHECK-NEXT:         "message": "No header providing \"Bar\" is directly included (fixes available)",
+# CHECK-NEXT:         "message": "No header providing \"Foo\" is directly included (fixes available)",
 # CHECK-NEXT:         "range": {
 # CHECK-NEXT:           "end": {
-# CHECK-NEXT:             "character": 14,
+# CHECK-NEXT:             "character": 4,
 # CHECK-NEXT:             "line": 2
 # CHECK-NEXT:           },
 # CHECK-NEXT:           "start": {
-# CHECK-NEXT:             "character": 11,
+# CHECK-NEXT:             "character": 1,
 # CHECK-NEXT:             "line": 2
 # CHECK-NEXT:           }
 # CHECK-NEXT:         },
@@ -49,14 +49,14 @@
 # CHECK-NEXT:       },
 # CHECK-NEXT:       {
 # CHECK-NEXT:         "code": "missing-includes",
-# CHECK-NEXT:         "message": "No header providing \"Foo\" is directly included (fixes available)",
+# CHECK-NEXT:         "message": "No header providing \"Bar\" is directly included (fixes available)",
 # CHECK-NEXT:         "range": {
 # CHECK-NEXT:           "end": {
-# CHECK-NEXT:             "character": 4,
+# CHECK-NEXT:             "character": 14,
 # CHECK-NEXT:             "line": 2
 # CHECK-NEXT:           },
 # CHECK-NEXT:           "start": {
-# CHECK-NEXT:             "character": 1,
+# CHECK-NEXT:             "character": 11,
 # CHECK-NEXT:             "line": 2
 # CHECK-NEXT:           }
 # CHECK-NEXT:         },