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

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 17 08:20:33 PST 2018

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;
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.

  rCTE Clang Tools Extra


More information about the cfe-commits mailing list