[PATCH] D42113: [clangd] Deduplicate symbols collected in global-symbol-builder tool.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
Herald added a reviewer: jkorous-apple.


================
Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:109
+  // Deduplicate the result by key.
+  // FIXME(ioeric): we need a better way to merge symbols with the same key. For
+  // example, class forward-declarations can have the same key as the class
----------------
nit: this comment could be tightened up a bit:

  // FIXME(ioeric): Merge occurrences, rather than just dropping all but one.
  // Definitions and forward declarations have the same key and may both have information.
  // Usage count will need to be aggregated across occurrences, too.


================
Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:113
+  // would also need to aggregate signals like usage count when they are added.
+  llvm::StringMap<llvm::StringRef> ReducedSymbols;
   Executor->get()->getToolResults()->forEachResult(
----------------
nit: UniqueSymbols? "Reduce" makes sense from an MR perspective but there's not enough context.


================
Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:116
+      [&ReducedSymbols](llvm::StringRef Key, llvm::StringRef Value) {
+        ReducedSymbols[Key] = Value;
+      });
----------------
picking the longest string might be better than random, but I'm not sure if it's enough to be worthwhile.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42113