[PATCH] D50889: [clangd] Make FileIndex aware of the main file
Eric Liu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 20 03:11:59 PDT 2018
ioeric added inline comments.
================
Comment at: clangd/ClangdServer.cpp:70
+// FIXME(ibiryukov): this should be a generic helper instead.
+class NoopCallbacks : public ParsingCallbacks {
public:
----------------
Maybe provide noop implementations for `ParsingCallbacks::onPreambleAST()` and `ParsingCallbacks::onMainAST` by default?
================
Comment at: clangd/ClangdServer.h:213
+ /// from the file itself. Those have different lifetimes and we merge results from both
+ class DynamicIndex : public ParsingCallbacks {
+ public:
----------------
Can we move this into ClangdServer.cpp as an implementation helper?
================
Comment at: clangd/ClangdServer.h:246
+ /// If present, an up-to-date of symbols in open files. Read via Index.
+ std::unique_ptr<DynamicIndex> FileIdx;
// If present, a merged view of FileIdx and an external index. Read via Index.
----------------
nit: `s/FileIdx/DymIdx`? (also need to update the comment above)
================
Comment at: clangd/index/FileIndex.cpp:46
+ AST.getTranslationUnitDecl()->decls().end());
+ TopLevelDecls = Storage;
+ }
----------------
nit: I'd try avoiding modify the input argument if possible. Maybe set `Storage` properly according to `TopLevelDecls`?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50889
More information about the cfe-commits
mailing list