krasimir added inline comments.
================
Comment at: clangd/CMakeLists.txt:12
ProtocolHandlers.cpp
)
----------------
nit: we might want to keep these sorted
================
Comment at: clangd/ClangdMain.cpp:41
// dispatching.
- DocumentStore Store;
- ASTManager AST(Out, Store, RunSynchronously);
- Store.addListener(&AST);
+ ClangdLSPServer AST(Out, RunSynchronously);
JSONRPCDispatcher Dispatcher(llvm::make_unique<Handler>(Out));
----------------
The combination of the name and the type of this variable makes no sense.
================
Comment at: clangd/ClangdServer.h:41
- void cacheFixIts(DiagnosticToReplacementMap FixIts);
- std::vector<clang::tooling::Replacement>
- getFixIts(const clangd::Diagnostic &D) const;
-
-private:
- std::unique_ptr<ASTUnit> AST;
- DiagnosticToReplacementMap FixIts;
+ virtual bool shouldBuildDiagnosticsForFile(PathRef File) = 0;
+ virtual void onDiagnosticsReady(PathRef File,
----------------
I feel that `shouldBuildDiagnosticsForFile` doesn't belong to this interface.
================
Comment at: clangd/ClangdServer.h:49
/// A request to the worker thread
-class ASTManagerRequest {
+class ServerRequest {
public:
----------------
If this models a request to the worker thread, why not call it `WorkerRequest`
or something? The current name is confusing, because it implies that also other
requests might be handled in this way.
================
Comment at: clangd/ClangdServer.h:62
+/// Handles scheduling on a different thread for ClangdServer
+class ClangdScheduler {
+public:
----------------
Maybe explicitly mention `Worker` in the name of this class?
================
Comment at: clangd/ClangdServer.h:68
+ /// Enqueue ServerRequest to be run on a worker thread
+ void Enqueue(ClangdServer &Server, ServerRequest Request);
----------------
The constructor already gets a `ClangdServer`, why pass it here?
https://reviews.llvm.org/D33047
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
