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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
