[PATCH] D72041: [clangd] Handle go-to-definition in macro invocations where the target appears in the expansion multiple times

This revision was automatically updated to reflect the committed changes. nridge marked 4 inline comments as done. Closed by commit rGe70a9f385025: [clangd] Handle go-to-definition in macro invocations where the target appears… (authored by nridge). Changed prior to commit:

[PATCH] D72041: [clangd] Handle go-to-definition in macro invocations where the target appears in the expansion multiple times

sammccall added inline comments. Comment at: clang-tools-extra/clangd/Selection.cpp:287 if (SM.getFileID(ArgStart) == SelFile) { - SourceLocation ArgEnd = SM.getTopMacroCallerLoc(Batch.back().location()); - return testTokenRange(SM.getFileOffset(ArgStart), -

[PATCH] D72041: [clangd] Handle go-to-definition in macro invocations where the target appears in the expansion multiple times

nridge marked an inline comment as done. nridge added inline comments. Comment at: clang-tools-extra/clangd/Selection.cpp:287 if (SM.getFileID(ArgStart) == SelFile) { - SourceLocation ArgEnd = SM.getTopMacroCallerLoc(Batch.back().location()); - return

[PATCH] D72041: [clangd] Handle go-to-definition in macro invocations where the target appears in the expansion multiple times

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Awesome, thanks for fixing this! Comment at: clang-tools-extra/clangd/Selection.cpp:261 + // consider it selected. + if

[PATCH] D72041: [clangd] Handle go-to-definition in macro invocations where the target appears in the expansion multiple times

nridge marked 4 inline comments as done. nridge added inline comments. Comment at: clang-tools-extra/clangd/Selection.cpp:261 + // consider it selected. + if (!SeenMacroCalls.insert(ArgStart).second) { +return NoTokens; sammccall wrote: >

[PATCH] D72041: [clangd] Handle go-to-definition in macro invocations where the target appears in the expansion multiple times

nridge updated this revision to Diff 247106. nridge marked an inline comment as done. nridge added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72041/new/ https://reviews.llvm.org/D72041 Files:

[PATCH] D72041: [clangd] Handle go-to-definition in macro invocations where the target appears in the expansion multiple times

sammccall added a comment. Thanks for fixing this. I think it could maybe use a further tweak, LMK if I'm missing something as my intuition around macros is pretty bad. Comment at: clang-tools-extra/clangd/Selection.cpp:261 + // consider it selected. + if

[PATCH] D72041: [clangd] Handle go-to-definition in macro invocations where the target appears in the expansion multiple times

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 61848 tests passed, 0 failed and 781 were skipped. {icon question-circle color=gray} clang-tidy: unknown. {icon check-circle color=green} clang-format: pass. Build artifacts

[PATCH] D72041: [clangd] Handle go-to-definition in macro invocations where the target appears in the expansion multiple times

nridge updated this revision to Diff 238094. nridge added a comment. Revise the approach to treat non-first expansions as not-selected Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72041/new/ https://reviews.llvm.org/D72041 Files:

[PATCH] D72041: [clangd] Handle go-to-definition in macro invocations where the target appears in the expansion multiple times

nridge added a comment. In D72041#1800189 , @sammccall wrote: > - the allSelectedNodes API on SelectionTree feels non-orthogonal: it's only > meaningful here because the input happened to be a point and thus touches a > single token and therefore node.

[PATCH] D72041: [clangd] Handle go-to-definition in macro invocations where the target appears in the expansion multiple times

sammccall added a comment. I like the goal here, a few high-level comments: - it seems unfortunate to solve this mostly in xrefs rather than mostly in SelectionTree. All the other clients potentially want this behavior. - the allSelectedNodes API on SelectionTree feels non-orthogonal: it's only

[PATCH] D72041: [clangd] Handle go-to-definition in macro invocations where the target appears in the expansion multiple times

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 61162 tests passed, 0 failed and 728 were skipped. {icon check-circle color=green} clang-tidy: pass. {icon check-circle color=green} clang-format: pass. Build artifacts

[PATCH] D72041: [clangd] Handle go-to-definition in macro invocations where the target appears in the expansion multiple times

nridge created this revision. nridge added a reviewer: hokein. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D72041 Files: