kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:91
+ // Whether a new CDB has been loaded but not broadcast yet.
+ bool Dirty = false;
+ // Last loaded CDB, meaningful if CachePopulated is set.
----------------
maybe rename this to `DidBroadcast` if we are not planning to add extra meaning
to `Dirty` in the near future?
================
Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:119
+ if (CachePopulated) {
+ ShouldBroadcast = Dirty;
+ Dirty = false;
----------------
what if caller is not willing to broadcast?
================
Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:253
+ for (DirectoryCache *Candidate : getDirectoryCaches(SearchDirs)) {
+ if ((CDB = Candidate->get(ShouldBroadcast))) {
+ DirCache = Candidate;
----------------
won't this overwrite `ShouldBroadcast` ?
for example if we get a query with `ShouldBroadcast` set to `true`, but first
searchdir doesn't have a CDB underneath, for all the consecutive calls this
will be `false`.
================
Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:263
- Result.CDB = Entry->CDB.get();
- Result.PI.SourceRoot = Entry->Path;
- }
+ // Mark CDB as broadcasted to make sure discovery is performed once.
+ CDBLookupResult Result;
----------------
stale comment.
================
Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.h:88
+ mutable llvm::StringMap<DirectoryCache> DirCaches;
+ mutable std::mutex Mutex;
+ mutable std::unique_ptr<DirectoryCache> OnlyDirCache;
----------------
it is kind of hard to figure out if this mutex is also meant for OnlyDirCache.
maybe separate it with a line or put some explicit `governed by` annotations?
================
Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.h:89
+ mutable std::mutex Mutex;
+ mutable std::unique_ptr<DirectoryCache> OnlyDirCache;
+
----------------
some comments about semantics of `OnlyDirCache` ? I suppose existing of it
implies `DirCaches` won't be used at all ?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92381/new/
https://reviews.llvm.org/D92381
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
