[PATCH] D129100: [clangd] Support external throttler for preamble builds

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 5 inline comments as done.
sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdServer.h:108
+    /// This throttler controls which preambles may be built at a given time.
+    clangd::PreambleThrottler *PreambleThrottler = nullptr;
+
----------------
kadircet wrote:
> why the qualification ?
same name is used for the type and the member name, an error in at least some compilers (I forget what the standard says).
I can't think of another good name


================
Comment at: clang-tools-extra/clangd/TUScheduler.h:114
+  // If it fails (e.g. races), resources are acquired and must be released.
+  virtual CancelFn acquire(llvm::StringRef Filename, AcquireCallback) = 0;
+
----------------
kadircet wrote:
> can we make the first parameter a struct instead? i feel like we might end up adding more parameters describing the reason for the request here.
My hypothesis is that *all* other parameters will be provided separately on unrelated codepaths (Throttler->setFoo(Filename, Foo)) and the name here is a join key rather than one parameter among many.

I might be wrong about this but would rather hold off on generalizing until we're sure.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129100/new/

https://reviews.llvm.org/D129100