alekseyshl added a comment.
http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/2437 is
broken by this revision.
Repository:
rL LLVM
https://reviews.llvm.org/D29886
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lis
This revision was automatically updated to reflect the committed changes.
Closed by commit rL295180: [clangd] Wire up ASTUnit and publish diagnostics
with it. (authored by d0k).
Changed prior to commit:
https://reviews.llvm.org/D29886?vs=88523&id=88532#toc
Repository:
rL LLVM
https://review
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
Comment at: clangd/ASTManager.cpp:138-139
+ // Currently we discard all pending requests and just enqueue the latest one.
+ while (!RequestQueue.empty())
+RequestQue
bkramer updated this revision to Diff 88523.
bkramer added a comment.
- Inline the request struct again.
https://reviews.llvm.org/D29886
Files:
clangd/ASTManager.cpp
clangd/ASTManager.h
clangd/CMakeLists.txt
clangd/ClangDMain.cpp
clangd/DocumentStore.h
test/clangd/diagnostics.test
klimek added inline comments.
Comment at: clangd/ASTManager.h:67
+ /// Setting Done to true will make the worker thread terminate.
+ std::atomic Done;
+};
bkramer wrote:
> klimek wrote:
> > arphaman wrote:
> > > It looks like `Done` is always accessed in a scop
bkramer updated this revision to Diff 88522.
bkramer added a comment.
- Do not lock while running DocumentStore callbacks
- Replace fake queue with a real queue (but it still has at most one element.
https://reviews.llvm.org/D29886
Files:
clangd/ASTManager.cpp
clangd/ASTManager.h
clangd/C
cierpuchaw added inline comments.
Comment at: clangd/DocumentStore.h:42
/// Delete a document from the store.
- void removeDocument(StringRef Uri) { Docs.erase(Uri); }
+ void removeDocument(StringRef Uri) { Docs.erase(Uri);
+for (const auto &Listener : Listeners)
---
bkramer added inline comments.
Comment at: clangd/ASTManager.cpp:21
+using namespace clang;
+using namespace clangd;
+
ioeric wrote:
> Any reason not to wrap code in namespaces instead?
I don't really have an opinion one way or the other, but this seems to be the
bkramer updated this revision to Diff 88515.
bkramer marked 16 inline comments as done.
bkramer added a comment.
- Address review comments.
https://reviews.llvm.org/D29886
Files:
clangd/ASTManager.cpp
clangd/ASTManager.h
clangd/CMakeLists.txt
clangd/ClangDMain.cpp
clangd/DocumentStore
klimek added inline comments.
Comment at: clangd/ASTManager.cpp:69
+ // our one-element queue is empty.
+ if (!RequestIsPending && !Done)
+ClangRequestCV.wait(Lock);
arphaman wrote:
> This is just a suggestion based on personal opinion: `Request
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?
Co
arphaman added inline comments.
Comment at: clangd/ASTManager.cpp:69
+ // our one-element queue is empty.
+ if (!RequestIsPending && !Done)
+ClangRequestCV.wait(Lock);
This is just a suggestion based on personal opinion: `RequestIsPending` and
krasimir added inline comments.
Comment at: clangd/DocumentStore.h:42
/// Delete a document from the store.
- void removeDocument(StringRef Uri) { Docs.erase(Uri); }
+ void removeDocument(StringRef Uri) { Docs.erase(Uri);
+for (const auto &Listener : Listeners)
-
ioeric added inline comments.
Comment at: clangd/ASTManager.cpp:21
+using namespace clang;
+using namespace clangd;
+
Any reason not to wrap code in namespaces instead?
https://reviews.llvm.org/D29886
___
cfe-commi
cierpuchaw added inline comments.
Comment at: clangd/DocumentStore.h:42
/// Delete a document from the store.
- void removeDocument(StringRef Uri) { Docs.erase(Uri); }
+ void removeDocument(StringRef Uri) { Docs.erase(Uri);
+for (const auto &Listener : Listeners)
---
cierpuchaw added inline comments.
Comment at: clangd/DocumentStore.h:42
/// Delete a document from the store.
- void removeDocument(StringRef Uri) { Docs.erase(Uri); }
+ void removeDocument(StringRef Uri) { Docs.erase(Uri);
+for (const auto &Listener : Listeners)
---
bkramer created this revision.
Herald added a subscriber: mgorny.
This requires an accessible compilation database. The parsing is done
asynchronously on a separate thread.
https://reviews.llvm.org/D29886
Files:
clangd/ASTManager.cpp
clangd/ASTManager.h
clangd/CMakeLists.txt
clangd/Clan
17 matches
Mail list logo
