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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
