[PATCH] D51747: [clangd] Show deprecation diagnostics

2018-09-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In https://reviews.llvm.org/D51747#1233530, @ilya-biryukov wrote: > In https://reviews.llvm.org/D51747#1233499, @kadircet wrote: > > > Np, not planning to land it before we figure out the issue with vim can't > > handling lots of diagnostics anyway. > > > Just

[PATCH] D51747: [clangd] Show deprecation diagnostics

2018-09-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 165462. kadircet marked 2 inline comments as done. kadircet added a comment. - Add matchers to test. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51747 Files: clangd/ClangdServer.cpp unittests/clangd/ClangdTests.cpp Index:

[PATCH] D51747: [clangd] Show deprecation diagnostics

2018-09-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. Awesome, thank you. Just a couple of last nits. Comment at: unittests/clangd/ClangdTests.cpp:984 +std::vector Diagnostics) override { + std::lock_guard Lock(Mutex); + for(const

[PATCH] D51747: [clangd] Show deprecation diagnostics

2018-09-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. In https://reviews.llvm.org/D51747#1233499, @kadircet wrote: > Np, not planning to land it before we figure out the issue with vim can't > handling lots of diagnostics anyway. Just wondering: what's the problem with Vim? Slowness? Bad UI for diagnostics when

[PATCH] D51747: [clangd] Show deprecation diagnostics

2018-09-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In https://reviews.llvm.org/D51747#1233401, @sammccall wrote: > In https://reviews.llvm.org/D51747#1233377, @kadircet wrote: > > > In https://reviews.llvm.org/D51747#1233371, @sammccall wrote: > > > > > Sorry for all the back and forth on this patch, but I have to

[PATCH] D51747: [clangd] Show deprecation diagnostics

2018-09-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 165302. kadircet added a comment. This revision is now accepted and ready to land. - Turn back to unit tests. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51747 Files: clangd/ClangdServer.cpp unittests/clangd/ClangdTests.cpp

[PATCH] D51747: [clangd] Show deprecation diagnostics

2018-09-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In https://reviews.llvm.org/D51747#1233377, @kadircet wrote: > In https://reviews.llvm.org/D51747#1233371, @sammccall wrote: > > > Sorry for all the back and forth on this patch, but I have to ask... what's > > up with switching to lit tests? > > > > We've mostly

[PATCH] D51747: [clangd] Show deprecation diagnostics

2018-09-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In https://reviews.llvm.org/D51747#1233371, @sammccall wrote: > Sorry for all the back and forth on this patch, but I have to ask... what's > up with switching to lit tests? > > We've mostly started avoiding these as they're hard to maintain and debug > (not to

[PATCH] D51747: [clangd] Show deprecation diagnostics

2018-09-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Sorry for all the back and forth on this patch, but I have to ask... what's up with switching to lit tests? We've mostly started avoiding these as they're hard to maintain and debug (not to mention write... crazy sed tricks!) They're mostly used in places where we

[PATCH] D51747: [clangd] Show deprecation diagnostics

2018-09-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet planned changes to this revision. kadircet added a comment. When doing some manual testing saw that it causes a lot of slow down on vim, even when I have just about 200 diagnostics, will wait till a few others check that out. We might wanna limit the number of diagnostics and keep only

[PATCH] D51747: [clangd] Show deprecation diagnostics

2018-09-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 165275. kadircet marked 2 inline comments as done. kadircet added a comment. - Convert unit tests to lit tests and address comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51747 Files: clangd/ClangdServer.cpp

[PATCH] D51747: [clangd] Show deprecation diagnostics

2018-09-13 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/ClangdServer.cpp:534 - // Inject the resource dir. + // Inject the resource dir. These flags are working for both gcc and clang-cl + //