[PATCH] D51438: [clangd] Run SignatureHelp using an up-to-date preamble, waiting if needed.
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 30 08:15:47 PDT 2018
sammccall added inline comments.
================
Comment at: clangd/TUScheduler.cpp:474
+ llvm::unique_function<void(std::shared_ptr<const PreambleData>)> Callback) {
+ if (RunSync)
+ return Callback(getPossiblyStalePreamble());
----------------
ilya-biryukov wrote:
> 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.
Right, of course :-)
Replaced this with an assert before we write to the queue.
================
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.
----------------
ilya-biryukov wrote:
> 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.
Yeah, the downside to that is that it *does* pollute the clangd:: namespace. So both are a bit sad.
This header is fairly widely visible (since it's included by clangdserver) and this API is fairly narrowly interesting, so as far as completion goes I think I prefer it being hidden in TUScheduler.
Repository:
rL LLVM
https://reviews.llvm.org/D51438
More information about the cfe-commits
mailing list