[clang-tools-extra] 5e74a3d - [clangd][IncludeCleaner] Use a proper comparator for deduplicating findings

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2023-05-01T12:19:33+02:00
New Revision: 5e74a3dc2da879d98204f2360e2e33571b93b91b

URL: https://github.com/llvm/llvm-project/commit/5e74a3dc2da879d98204f2360e2e33571b93b91b
DIFF: https://github.com/llvm/llvm-project/commit/5e74a3dc2da879d98204f2360e2e33571b93b91b.diff

LOG: [clangd][IncludeCleaner] Use a proper comparator for deduplicating findings

Previous one didn't have strict weak ordering guarantees.

Added: 
    

Modified: 
    clang-tools-extra/clangd/IncludeCleaner.cpp
    clang-tools-extra/clangd/test/include-cleaner-batch-fix.test

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index d3754de689eb0..3b913d851abe2 100644
--- 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 
diff erent 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
index 4b2208ba3e146..b7e9661b79b08 100644
--- 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:         },