[PATCH] D51438: [clangd] Run SignatureHelp using an up-to-date preamble, waiting if needed.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

The implementation is really simple. LG!
To get proper operation order in tests, we can wait in the diagnostics callback that runs on the worker thread (IIRC, we do that in some of the other tests too).



================
Comment at: clangd/TUScheduler.cpp:188
+  /// immediately, or later on the worker thread if it's not yet ready.
+  /// Threadsafe, but should not be called from the worker thread.
+  void getCurrentPreamble(
----------------
As discussed offline, it looks like it shouldn't be a problem to call it from the working thread, so we could probably remove this restriction from the comment.
Not that it's the right thing to do or that we have a use-case for that yet.


================
Comment at: clangd/TUScheduler.cpp:474
+    llvm::unique_function<void(std::shared_ptr<const PreambleData>)> Callback) {
+  if (RunSync)
+    return Callback(getPossiblyStalePreamble());
----------------
It seems we could remove the special-casing of `RunSync` and still get correct results (the Requests queue is always empty).
But feel free to keep it for clarity.


================
Comment at: clangd/TUScheduler.h:123
+  /// Controls whether preamble reads wait for the preamble to be up-to-date.
+  enum PreambleConsistency {
+    /// The preamble is generated from the current version of the file.
----------------
Maybe use a strongly-typed enum outside the class?
`TUScheduler::Stale` will turn into `PreambleConsistency::Stale` at the call-site. The main upside is that it does not pollute completions inside the class with enumerators.

Just a suggestion, feel free to ignore.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51438