[PATCH] D54475: [clangd] Allow observation of changes to global CDBs.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clangd/Function.h:90
+class Event {
+public:
+  // A Listener is the callback through which events are delivered.
----------------
I assume the `Event` is supposed to be used only with non-reference and non-const qualified types.
Maybe add a static assert for that? Something like:
`static_assert(std::is_same_v<std::decay_t<T>, T>)` 


================
Comment at: clangd/Function.h:135
+    for (const auto &L : Listeners)
+      L.first(V);
+  }
----------------
As discussed offline, running the callbacks under the lock might cause deadlocks.
We probably won't hit this case in near future, but it would be nice to find a way to diagnose this situation. Not sure what we can do non-intrusively, though.


================
Comment at: clangd/GlobalCompilationDatabase.h:72
   tooling::CompilationDatabase *getCDBForFile(PathRef File) const;
-  tooling::CompilationDatabase *getCDBInDirLocked(PathRef File) const;
+  std::pair<tooling::CompilationDatabase *, bool>
+  getCDBInDirLocked(PathRef File) const;
----------------
Maybe add a `/*Cached*/`  comment here too?


================
Comment at: unittests/clangd/GlobalCompilationDatabaseTests.cpp:98
+    Changes.push_back(ChangedFiles);
+  });
+}
----------------
Maybe add an actual test?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54475