This revision was automatically updated to reflect the committed changes.
Closed by commit rL347474: [clangd] Cleanup: make the diags callback global in
TUScheduler (authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Everything looks so much better...
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54760
___
cfe-commits mailing list
cfe-c
ilya-biryukov updated this revision to Diff 175056.
ilya-biryukov added a comment.
- Rebase
- Remove makeUpdateCallbacks, expose the callback class instead
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54760
Files:
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/TUS
ilya-biryukov planned changes to this revision.
ilya-biryukov added a comment.
Would like to land https://reviews.llvm.org/D54829 first and rebase on top of
it.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54760
___
cfe-commits m
ilya-biryukov updated this revision to Diff 175028.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
- Remove accidentally added redundant 'private:' section
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54760
Files:
clangd/ClangdServer.cpp
cl
ilya-biryukov added inline comments.
Comment at: clangd/ClangdServer.cpp:77
+FileIndex *FIndex,
+llvm::unique_function)> OnDiags) {
+ using DiagsCallback = decltype(OnDiags);
sammccall wrote:
> hmm, the double-indirection for diags seems a bit funny, esp
sammccall added a comment.
Just the context thing. Rest is some optional simplifications.
Comment at: clangd/ClangdServer.cpp:77
+FileIndex *FIndex,
+llvm::unique_function)> OnDiags) {
+ using DiagsCallback = decltype(OnDiags);
hmm, the double-indirect
ilya-biryukov added inline comments.
Comment at: clangd/ClangdServer.h:232
+ /// addDocument. Used to avoid races when sending diagnostics to the clients.
+ static Key DocVersionKey;
sammccall wrote:
> I'm not sure using context here buys much: there aren't m
ilya-biryukov updated this revision to Diff 174914.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
- Merge the diagnostics callback into ParsingCallbacks
- updateWithDiags(File, getInputs(File...)) -> updateWithDiags(File, ...)
Repository:
rCTE Clang Tools Extra
sammccall added a comment.
Nice!
The biggest suggestion I have is merging the callback into ASTCallbacks. That's
awkward in initialization (`makeUpdateCallbacks` probably needs to be called
unconditionally with a maybe-null pointer) but it seems like a natural grouping
in all the places it gets
ilya-biryukov added a comment.
The tests still model the old callbacks using callbacks, unfortunately I see no
good way to test same things in any other way.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54760
___
cfe-commits mail
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric,
javed.absar.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54760
Files:
clangd/ClangdServer.cpp
clangd/ClangdServer.h
cl
12 matches
Mail list logo
