sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Thanks for this cleanup!
The way we deal with ResourceDir still doesn't feel ideal, but that's no worse
here and everything else is better!
================
Comment at: clangd/ClangdServer.cpp:211
+ // Clear the cached CompileCommand for File, this ensures the new one will be
+ // requested for the next reparse.
----------------
nit: the comment and whitespace seems unneccesary, this is exactly what
invalidate() is documented to do.
(A comment saying *why* might be useful, but we don't really know why here)
================
Comment at: clangd/ClangdServer.cpp:285
std::shared_ptr<PCHContainerOperations> PCHs = this->PCHs;
+ tooling::CompileCommand CompileCommand = CompileArgs.getCompileCommand(File);
----------------
nit: remove blank line?
================
Comment at: clangd/ClangdServer.h:339
- GlobalCompilationDatabase &CDB;
+ // Stores compile commands produced by GlobalCompilationDatabase.
+ class CompileArgsCache {
----------------
nit: can we hide this in the cpp file or move to GCD.h to reduce clutter?
================
Comment at: clangd/ClangdServer.h:339
- GlobalCompilationDatabase &CDB;
+ // Stores compile commands produced by GlobalCompilationDatabase.
+ class CompileArgsCache {
----------------
sammccall wrote:
> nit: can we hide this in the cpp file or move to GCD.h to reduce clutter?
Can the comment give some motivation here? It's clear enough from the name what
it does, but why?
================
Comment at: clangd/ClangdServer.h:340
+ // Stores compile commands produced by GlobalCompilationDatabase.
+ class CompileArgsCache {
+ public:
----------------
sorry for naive question - this doesn't need to be threadsafe, because
clangdserver itself isn't, right?
I never really looked at the threading model of clangdserver - it looks like
most stuff is called externally on one thread, and any async logic avoids
accessing most of the members?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42429
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
