[PATCH] D29886: [clangd] Wire up ASTUnit and publish diagnostics with it.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.


================
Comment at: clangd/ASTManager.cpp:148
+  std::string Error;
+  I = tooling::CompilationDatabase::autoDetectFromSource(Uri, Error);
+  return I.get();
----------------
Do you have an idea about a proper error handling if Error?


================
Comment at: clangd/ASTManager.cpp:162
+    Commands = CDB->getCompileCommands(Uri);
+    // chdir. This is thread hostile.
+    if (!Commands.empty())
----------------
Then maybe we need a big disclaimer comment at the declaration of `createASTUnitForFile`?


================
Comment at: clangd/ClangDMain.cpp:32
+  auto *AST = new ASTManager(Out, Store);
+  Store.addListener(std::unique_ptr<ASTManager>(AST));
   JSONRPCDispatcher Dispatcher(llvm::make_unique<Handler>(Out));
----------------
Why not directly `Store.addListener(llvm::make_unique<ASTManager>(Out, Store));`?


================
Comment at: clangd/DocumentStore.h:39
+    for (const auto &Listener : Listeners)
+      Listener->onDocumentAdd(Uri, *this);
+  }
----------------
Is only the main thread supposed to addDocument-s? Otherwise the listener notifications might need to be guarded.


https://reviews.llvm.org/D29886