[PATCH] D70489: [clangd] Add xref for macro to static index.

This revision was automatically updated to reflect the committed changes. Closed by commit rG9347655a2754: [clangd] Add xref for macro to static index. (authored by usaxena95). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70489/new/ https://reviews

[PATCH] D70489: [clangd] Add xref for macro to static index.

usaxena95 added a comment. Updated benchmarks: **Before this patch** Loaded Dex from static-index.idx with estimated memory usage 354410880 bytes - number of symbols: 462463 - number of refs: 6361763 - numnber of relations: 20322 **After this patch** Num refs increases by **5.7%**

[PATCH] D70489: [clangd] Add xref for macro to static index.

merge_guards_bot added a comment. Build result: fail - 60403 tests passed, 1 failed and 726 were skipped. failed: LLVM.Bindings/Go/go.test Log files: console-log.txt , CMakeCache.txt

[PATCH] D70489: [clangd] Add xref for macro to static index.

usaxena95 updated this revision to Diff 232250. usaxena95 marked 5 inline comments as done. usaxena95 added a comment. Addressed comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70489/new/ https://reviews.llvm.org/D70489 Files: clang-tool

[PATCH] D70489: [clangd] Add xref for macro to static index.

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. thanks, looks good with a few nits. I think the benchmark data doesn't correct any more with the latest patch, we don't increase number of symbols. Comment at: clang-tools-e

[PATCH] D70489: [clangd] Add xref for macro to static index.

merge_guards_bot added a comment. Build result: fail - 60403 tests passed, 1 failed and 726 were skipped. failed: LLVM.Bindings/Go/go.test Log files: console-log.txt , CMakeCache.txt

[PATCH] D70489: [clangd] Add xref for macro to static index.

merge_guards_bot added a comment. Build result: fail - 60403 tests passed, 1 failed and 726 were skipped. failed: LLVM.Bindings/Go/go.test Log files: console-log.txt , CMakeCache.txt

[PATCH] D70489: [clangd] Add xref for macro to static index.

usaxena95 updated this revision to Diff 232126. usaxena95 added a comment. Removed unintended space. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70489/new/ https://reviews.llvm.org/D70489 Files: clang-tools-extra/clangd/index/SymbolCollector.c

[PATCH] D70489: [clangd] Add xref for macro to static index.

usaxena95 added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:472 } - if (Opts.CollectMacro) { -assert(PP); hokein wrote: > hokein wrote: > > this code should not be modified, since we don't use the `CollectMacro` > > op

[PATCH] D70489: [clangd] Add xref for macro to static index.

usaxena95 updated this revision to Diff 232125. usaxena95 marked 3 inline comments as done. usaxena95 added a comment. Addressed comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70489/new/ https://reviews.llvm.org/D70489 Files: clang-tool

[PATCH] D70489: [clangd] Add xref for macro to static index.

hokein added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:472 } - if (Opts.CollectMacro) { -assert(PP); hokein wrote: > this code should not be modified, since we don't use the `CollectMacro` > option. this comment seem

[PATCH] D70489: [clangd] Add xref for macro to static index.

merge_guards_bot added a comment. Build result: fail - 60403 tests passed, 1 failed and 726 were skipped. failed: LLVM.Bindings/Go/go.test Log files: console-log.txt , CMakeCache.txt

[PATCH] D70489: [clangd] Add xref for macro to static index.

merge_guards_bot added a comment. Build result: fail - 60403 tests passed, 1 failed and 726 were skipped. failed: LLVM.Bindings/Go/go.test Log files: console-log.txt , CMakeCache.txt

[PATCH] D70489: [clangd] Add xref for macro to static index.

usaxena95 updated this revision to Diff 232078. usaxena95 marked 4 inline comments as done. usaxena95 added a comment. Addressed comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70489/new/ https://reviews.llvm.org/D70489 Files: clang-tool

[PATCH] D70489: [clangd] Add xref for macro to static index.

usaxena95 updated this revision to Diff 232079. usaxena95 added a comment. Explicitly turn off CollectMacro in test. #Updatin D70489 : [clangd] Add xref for macro to static index. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D70489: [clangd] Add xref for macro to static index.

hokein added a comment. thanks, mostly good, my main concern is that the patch still relies on the `CollectMacro` and `CollectMainFileSymbols ` option, see my comments below. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:349

[PATCH] D70489: [clangd] Add xref for macro to static index.

usaxena95 added a comment. **Before this patch** `CollectMacro` is disabled by default Loaded Dex from static-index.idx with estimated memory usage 354410880 bytes - number of symbols: 462463 - number of refs: 6361763 - numnber of relations: 20322 **Before this patch** `CollectMacr

[PATCH] D70489: [clangd] Add xref for macro to static index.

merge_guards_bot added a comment. Build result: pass - 60166 tests passed, 0 failed and 730 were skipped. Log files: console-log.txt , CMakeCache.txt

[PATCH] D70489: [clangd] Add xref for macro to static index.

usaxena95 updated this revision to Diff 231834. usaxena95 marked 5 inline comments as done. usaxena95 added a comment. Addressed comments. Will update the increase in size of index shortly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70489/new/

[PATCH] D70489: [clangd] Add xref for macro to static index.

usaxena95 added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:532 + const IdentifierInfo *II = MacroRef.first; + if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo()) +if (auto ID = getSymbolID(II->getName(), MI, PP->g

[PATCH] D70489: [clangd] Add xref for macro to static index.

hokein added a comment. btw, could you measure the increasing size of the index with this patch? You could run ./bin/clangd-indexer -format=binary -executor=all-TUs . > static-index.idx ./bin/dexp static-index.idx # you will see the memory usage. Comment at: clang-tools

[PATCH] D70489: [clangd] Add xref for macro to static index.

merge_guards_bot added a comment. Build result: pass - 60166 tests passed, 0 failed and 730 were skipped. Log files: console-log.txt , CMakeCache.txt

[PATCH] D70489: [clangd] Add xref for macro to static index.

usaxena95 updated this revision to Diff 231660. usaxena95 added a comment. Added correct documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70489/new/ https://reviews.llvm.org/D70489 Files: clang-tools-extra/clangd/index/SymbolCollecto

[PATCH] D70489: [clangd] Add xref for macro to static index.

usaxena95 marked 2 inline comments as done. usaxena95 added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:374 +Roles & static_cast(index::SymbolRole::Definition) || +Roles & static_cast(index::SymbolRole::Reference))) return

[PATCH] D70489: [clangd] Add xref for macro to static index.

merge_guards_bot added a comment. Build result: pass - 60166 tests passed, 0 failed and 730 were skipped. Log files: console-log.txt , CMakeCache.txt

[PATCH] D70489: [clangd] Add xref for macro to static index.

usaxena95 updated this revision to Diff 231648. usaxena95 marked 5 inline comments as done. usaxena95 added a comment. Addressed comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70489/new/ https://reviews.llvm.org/D70489 Files: clang-tool

[PATCH] D70489: [clangd] Add xref for macro to static index.

usaxena95 marked an inline comment as done. usaxena95 added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:374 +Roles & static_cast(index::SymbolRole::Definition) || +Roles & static_cast(index::SymbolRole::Reference))) return

[PATCH] D70489: [clangd] Add xref for macro to static index.

hokein added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:374 +Roles & static_cast(index::SymbolRole::Definition) || +Roles & static_cast(index::SymbolRole::Reference))) return true; I think we should avoid

[PATCH] D70489: [clangd] Add xref for macro to static index.

usaxena95 created this revision. usaxena95 added a reviewer: hokein. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. This adds the references for macros to the SymbolCollector (used for static index). Enabled if `CollectM

[PATCH] D70489: [clangd] Add xref for macro to static index.

merge_guards_bot added a comment. Build result: pass - 60166 tests passed, 0 failed and 730 were skipped. Log files: console-log.txt , CMakeCache.txt