[PATCH] D58341: [clangd] Index UsingDecls

2019-02-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE354879: [clangd] Index UsingDecls (authored by kadircet, committed by ). Changed prior to commit: https://reviews.llvm.org/D58341?vs=188170&id=188363#toc Repository: rCTE Clang Tools Extra CHANGES

[PATCH] D58341: [clangd] Index UsingDecls

2019-02-25 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 188170. kadircet marked 2 inline comments as done. kadircet added a comment. - Address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58341/new/ https://reviews.llvm.org/D58341 Files: unittests/clang

[PATCH] D58341: [clangd] Index UsingDecls

2019-02-25 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. Thanks for the explanation. In D58341#1401365 , @ilya-biryukov wrote: > In D58341#1401295 , @hokein wrote: > > > std::strcmp is a fair case here. Sema seems not returning using-decls as >

[PATCH] D58341: [clangd] Index UsingDecls

2019-02-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 187376. kadircet marked an inline comment as done. kadircet added a comment. - Revert last change Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58341/new/ https://reviews.llvm.org/D58341 Files: unittests/cla

[PATCH] D58341: [clangd] Index UsingDecls

2019-02-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM from my side, with a few NITs. But let's wait for an LGTM from Haojian too, to make sure his concerns are addressed. Comment at: unittests/clangd/SymbolCo

[PATCH] D58341: [clangd] Index UsingDecls

2019-02-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 187367. kadircet added a comment. - Add tests for code completion Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58341/new/ https://reviews.llvm.org/D58341 Files: unittests/clangd/SymbolCollectorTests.cpp u

[PATCH] D58341: [clangd] Index UsingDecls

2019-02-18 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. In D58341#1401295 , @hokein wrote: > std::strcmp is a fair case here. Sema seems not returning using-decls as part > of code completion results, it this an intended behavior? Yeah, I think it is. There's an explicit code p

[PATCH] D58341: [clangd] Index UsingDecls

2019-02-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. In D58341#1401212 , @ilya-biryukov wrote: > For context: @hokein mentioned problems in the clangd's code completion if we > would index these symbols. > This patch in itself does not hurt much, users of the indexing API can > de

[PATCH] D58341: [clangd] Index UsingDecls

2019-02-18 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. For context: @hokein mentioned problems in the clangd's code completion if we would index these symbols. This patch in itself does not hurt much, users of the indexing API can decide how to deal with `UsingDecl` on their own, clangd is just one of the clients. >

[PATCH] D58341: [clangd] Index UsingDecls

2019-02-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. I wonder how does merge work with Sema results? See the case below, IIUC our indexer has one symbol for this using decl, but the code completion result from Sema contains two symbols. The symbol ids of these 3 symbols are different, so we will end up with 3 completion r

[PATCH] D58341: [clangd] Index UsingDecls

2019-02-18 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. Could we also add a test for code completion to make sure we return the new decls there? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58341/new/ https://reviews.llvm.org/D58341

[PATCH] D58341: [clangd] Index UsingDecls

2019-02-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 187209. kadircet added a comment. - Update USR to include "UD" Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58341/new/ https://reviews.llvm.org/D58341 Files: unittests/clangd/SymbolCollectorTests.cpp unit

[PATCH] D58341: [clangd] Index UsingDecls

2019-02-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: hokein. Herald added subscribers: cfe-commits, jdoerfert, arphaman, jkorous, MaskRay, ioeric, ilya-biryukov. Herald added a project: clang. D58340 enables indexing of USRs, this makes sure test in clangd