ilya-biryukov added a comment.
FWIW, the old implementation of the CDB looked simpler (which makes sense,
since it only allowed the in-memory compile commands, but the new
implementation also falls back to the base CDB, i.e. it's now doing two things).
However, if we can't avoid this protocol extension, the change LG and the model
behind it should hopefully make sense from the user's perspective.
Still not sure whether overriding the normal command (not the fallback command)
when we also read from the CDB is a useful feature.
================
Comment at: clangd/ClangdLSPServer.cpp:670
+ /*Output=*/"");
+ if (Old != New)
+ CDB->setCompileCommand(File, std::move(New));
----------------
Is this an optimization to not trigger compile command changes?
Can we perform it on the CDB level to make sure we hit it in the future if more
code calling `setCompileCommand` is added?
================
Comment at: clangd/ClangdLSPServer.h:42
ClangdLSPServer(Transport &Transp, const clangd::CodeCompleteOptions &CCOpts,
- llvm::Optional<Path> CompileCommandsDir,
- bool ShouldUseInMemoryCDB, const ClangdServer::Options
&Opts);
+ llvm::Optional<Path> CompileCommandsDir, bool NoReadCDB,
+ const ClangdServer::Options &Opts);
----------------
NIT: the negative variable names might cause confusion (i.e. the double
negations are hard to read). Maybe use the name of the corresponding field
(UseDirectoryCDB)?
================
Comment at: clangd/ClangdLSPServer.h:139
+ llvm::Optional<OverlayCDB> CDB;
+ std::unique_ptr<GlobalCompilationDatabase> BaseCDB;
// The ClangdServer is created by the "initialize" LSP method.
----------------
Maybe add a comment on how the two compilation databases are combined?
================
Comment at: clangd/GlobalCompilationDatabase.h:79
+/// using an in-memory mapping.
+class OverlayCDB : public GlobalCompilationDatabase {
public:
----------------
The name does not seem to fully capture what this class does, i.e. allowing to
override compile commands for some files.
Maybe use `OverridenCDB` or something similar?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53687
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
