[PATCH] D159497: [RFC][clangd] Check if SelectionTree is complete

2023-10-02 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv added a comment. ping? Any thoughts. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159497/new/ https://reviews.llvm.org/D159497 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D159497: [RFC][clangd] Check if SelectionTree is complete

2023-09-11 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv created this revision. Herald added subscribers: kadircet, arphaman. Herald added a project: All. kuganv retitled this revision from "[clangd] Check if SelectionTree is complete" to "[RFC][clangd] Check if SelectionTree is complete". kuganv edited the summary of this revision. kuganv added

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

2023-06-11 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv updated this revision to Diff 530328. kuganv marked 6 inline comments as done. kuganv added a comment. Updated based on the review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148088/new/ https://reviews.llvm.org/D148088 Files:

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

2023-05-23 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv added inline comments. Comment at: clang-tools-extra/clangd/Preamble.cpp:106-113 +CI.setSema(nullptr); +CI.setASTConsumer(nullptr); +if (CI.getASTReader()) { + CI.getASTReader()->setDeserializationListener(nullptr); + /* This just sets consumer to nul

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

2023-05-23 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv updated this revision to Diff 524836. kuganv marked an inline comment as done. kuganv added a comment. Set just PrecompilePreambleConsumer/PCHGenerator and ASTMutationListener to null. Set the FileManager VFS to consuming FS so that it is thread safe. Repository: rG LLVM Github Monorep

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

2023-05-19 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv updated this revision to Diff 523784. kuganv added a comment. Revised Except for: 1. Explicitly destructing in AfterExecute 2. setStatCache to take refence Removing Both of which is causing crash. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

2023-05-17 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv marked 2 inline comments as done. kuganv added inline comments. Comment at: clang-tools-extra/clangd/Preamble.cpp:698-700 + // While extending the life of FileMgr and VFS, StatCache should also be + // extended. + Ctx.setStatCache(Result->StatCache); ---

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

2023-05-16 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv updated this revision to Diff 522523. kuganv added a comment. Fixed missing merge that caused build error. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148088/new/ https://reviews.llvm.org/D148088 Files: clang-tools-extra/clangd/ClangdSe

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

2023-05-16 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv added inline comments. Comment at: clang-tools-extra/clangd/Preamble.cpp:694 Result->MainIsIncludeGuarded = CapturedInfo.isMainFileIncludeGuarded(); -return Result; +CapturedCtx.emplace(CapturedInfo.takeLife()); +return std::make_pair(Result, CapturedCtx);

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

2023-05-16 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv updated this revision to Diff 522471. kuganv added a comment. As per review, moved Preamble indexing into ClangdServer's IndexTasks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148088/new/ https://reviews.llvm.org/D148088 Files: clang-t

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

2023-05-15 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv added inline comments. Comment at: clang-tools-extra/clangd/Preamble.cpp:106-113 +CI.setSema(nullptr); +CI.setASTConsumer(nullptr); +if (CI.getASTReader()) { + CI.getASTReader()->setDeserializationListener(nullptr); + /* This just sets consumer to nul

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

2023-05-15 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv updated this revision to Diff 522083. kuganv added a comment. Fixed tests clangd/unittests/TUSchedulerTests.cpp require re-warite due to delayed onPreambleAST callback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148088/new/ https://revie

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

2023-05-13 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv updated this revision to Diff 521936. kuganv edited the summary of this revision. kuganv added a comment. Herald added a project: clang. Re-implemented based on review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148088/new/ https://reviews

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

2023-05-03 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv added a comment. In D148088#4316352 , @kadircet wrote: > Sorry I was trying to give some brief idea about what it might look like in > `Implementation Concerns` section above, but let me give some more details; > > I think we can just change the s

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

2023-05-03 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv added a comment. Thanks @kadircet , @sammccall and @ilya-biryukov for the super detailed explanation. As mentioned, we will test clangd with an ASAN build to test. I also would like to get your feedback on the implementation. After we claim the AST as per @sammccall's patch into LiveAST

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

2023-05-02 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv marked an inline comment as done. kuganv added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:88 +if (PreambleIndexTask) + PreambleIndexTask->runAsync("task:" + Path + Version, + std::move(Task)); --

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

2023-04-27 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv added a comment. In D148088#4302092 , @ilya-biryukov wrote: >> We would like to move the preamble index out of the critical path > > Could you provide motivation why you need to do it? What is the underlying > problem that this change is trying t

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

2023-04-26 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv created this revision. Herald added subscribers: kadircet, arphaman, javed.absar. Herald added a project: All. kuganv updated this revision to Diff 517147. kuganv added a comment. kuganv edited the summary of this revision. kuganv added reviewers: kadircet, sam, ilya-biryukov, nridge. kuganv

[PATCH] D144599: [clangd/index/remote]NFC: Adapt code to newer grpc/protobuf versions

2023-02-23 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv added a comment. > if you don't mind me asking, are you deliberately building remote-index > components? i.e. do you have an internal remote-index server/deployment ? > Asking as it'd be great to know that we've adoption here, outside of > ourselves. > > OTOH, if you're not actually us

[PATCH] D141950: [NFC] Use find_last_of when seraching for code in getRawCommentForDeclNoCacheImpl

2023-02-21 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv added a comment. In D141950#4141455 , @aaron.ballman wrote: > LGTM! You should probably put "NFC" into the patch title when landing the > changes though, so it's more clear as to why there's no test coverage. > > Do you think this warrants adding

[PATCH] D141950: Use find_last_of when seraching for code in getRawCommentForDeclNoCacheImpl

2023-02-13 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv updated this revision to Diff 496936. kuganv added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141950/new/ https://reviews.llvm.org/D141950 Files: clang/lib/AST/ASTContext.cpp Index: clang/lib/AST/ASTContext.cpp

[PATCH] D141950: Use find_last_of when seraching for code in getRawCommentForDeclNoCacheImpl

2023-02-04 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv updated this revision to Diff 494870. kuganv added a comment. Rebasing branch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141950/new/ https://reviews.llvm.org/D141950 Files: clang/lib/AST/ASTContext.cpp Index: clang/lib/AST/ASTContex

[PATCH] D141950: Use find_last_of when seraching for code in getRawCommentForDeclNoCacheImpl

2023-02-02 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv created this revision. Herald added a subscriber: kadircet. Herald added a project: All. kuganv updated this revision to Diff 489882. kuganv edited the summary of this revision. kuganv added a comment. kuganv added subscribers: DmitryPolukhin, 0x1eaf, ivanmurashko. kuganv updated this revisi

[PATCH] D124909: Fix issues with using clangd with C++ modules

2022-05-07 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv added a comment. @sammccall, What are the first steps that is needed for supporting modules with clangd. From what I see, options related to handling comments that are specifically added to the compile options by clangd needs handling. This RFC diff is on that note. AFIK, since, clang

[PATCH] D124909: Fix issues with using clangd with C++ modules

2022-05-04 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv added a comment. Thanks a lot for the detailed. comment. Yes, you are right in that we are using build system (buck) to build the modules and provide as part of the CDB. This is fairly cheap when we hit the buck cache that is also shared by others. You are right in that the PCM format is

[PATCH] D124909: Fixes following incompatibility issues with c++ modules

2022-05-04 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv created this revision. Herald added subscribers: usaxena95, kadircet, arphaman. Herald added a project: All. kuganv requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added projects: clang, clang-tools-extra. 1. Preamble generated by clangd is g

[PATCH] D124432: Fix issues with using clangd with C++ modules

2022-05-03 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv updated this revision to Diff 426724. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124432/new/ https://reviews.llvm.org/D124432 Files: clang-tools-extra/clangd/test/modules-options-compatablity-test/A.h clang-tools-extra/clangd/test/modules-options-compatablity-test/B.h cl

[PATCH] D124432: Fix issues with using clangd with C++ modules

2022-05-03 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv updated this revision to Diff 426690. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124432/new/ https://reviews.llvm.org/D124432 Files: clang-tools-extra/clangd/test/modules-options-compatablity-test/A.h clang-tools-extra/clangd/test/modules-options-compatablity-test/B.h cl

[PATCH] D124432: Fix issues with using clangd with C++ modules

2022-04-25 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv created this revision. Herald added subscribers: dexonsmith, usaxena95, kadircet, arphaman. Herald added a project: All. kuganv requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added projects: clang, clang-tools-extra. Fixes following