kadircet added a comment.
I believe we should log some warning at startup if user has provided
`--compile-commands-dir`. Saying that "CDB search customizations through config
is disabled". (only emitting the warning when we hit a config with CDB search
customization would be nicer, but plumbing and making ConfigCompiler aware of
such things sounds terrifying :/)
mostly LG, didn't get a change to look at the tests yet (will do so soon)
================
Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:267
+ llvm::Optional<Config::CDBSearchSpec> Spec;
+ if (**F.CompilationDatabase == "Ancestors") {
+ Spec.emplace();
----------------
i hope no one tries to put their CDBs in a directory named Ancestors/None :)))
(it is definitely better than breaking any project with a top-level directory
named Ancestors/None)
================
Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:278
+ // Drop trailing slash to put the path in canonical form.
+ // Should makeAbsolute do this?
+ llvm::StringRef Rel = llvm::sys::path::relative_path(*Path);
----------------
+1 i think it should.
================
Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:617
+ Child->Parent = &Info;
+ // Keep walking, whether we inserted or not, if parent link is missing.
+ // (If it's present, parent links must be present up to the root, so
stop)
----------------
missing a `Child = &Info;` ?
================
Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:650
+ DirValues[I]->Cache = DirCaches[I];
+ if (DirCaches[I] == ThisCache)
+ DirValues[I]->State = DirInfo::TargetCDB;
----------------
nit: `DirValues[I]->State = DirCaches[I] == ThisCache ? DirInfo::TargetCDB :
DirInfo::Unknown;`, to reduce branching.
================
Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:680
+ // Walk up to the next parent.
+ return shouldInclude(SearchPath(Info->Parent, P.getInt()));
}
----------------
s/P.getInt()/1
as we'll bail out otherwise.
================
Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:717
+ case Config::CDBSearchSpec::Ancestors:
+ SearchPaths[I].setInt(/*Recursive*/ 1);
+ SearchPaths[I].setPointer(addParents(AllFiles[I]));
----------------
s/Recursive/Recursive=/
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95057/new/
https://reviews.llvm.org/D95057
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
