sammccall added inline comments.
================
Comment at: clangd/CMakeLists.txt:50
add_subdirectory(tool)
+add_subdirectory(global-symbol-builder)
----------------
I think generally we run `check-clang-tools` before committing - I guess it's
OK not to have tests for this experimental tool, but we should at least keep it
building. Can you make check-clang-tools build it?
================
Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:106
+ std::error_code EC;
+ SymbolSlab Result;
+ std::mutex SymbolMutex;
----------------
FYI D41506 changes this API, adding `SymbolSlab::Builder`. Shouldn't be a
problem, but we'll need to merge.
================
Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:112
+ for (const auto &Symbol : NewSymbols) {
+ auto it = Result.find(Symbol.second.ID);
+ if (it == Result.end())
----------------
This seems like you might end up overwriting good declarations with bad ones
(e.g. forward decls). Picking one with a simple heuristic like "longest range"
would get classes right.
================
Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:175
+ *Executor->get()->getExecutionContext()));
+ auto Err = Executor->get()->execute(std::move(T));
+ if (Err) {
----------------
I don't understand why we need to plumb callbacks through SymbolCollector.
Can't we just write the slab after this line?
================
Comment at: clangd/global-symbol-builder/run-global-symbol-builder.py:1
+#!/usr/bin/env python
+#
----------------
So we now have three copies of this script...
I'm probably missing something, but what's here that's hard to do in a single
binary?
I'd have to think it'd be faster if we weren't constantly spawning new
processes and roundtripping all the data through YAML-on-disk.
(If this is just expedience because we have an existing template to copy, I'm
happy to try to combine it into one binary myself)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41491
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
