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

[Sam McCall via Phabricator via cfe-commits]

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),
-                            SM.getFileOffset(ArgEnd));
+      if (isFirstExpansion(FID, ArgStart, SM)) {
+        SourceLocation ArgEnd =
----------------
nridge wrote:
> sammccall wrote:
> > when false is the fallthrough to handling as if part of the macro body deliberate?
> > 
> > Thinking about it I suppose either that or returning NoTokens works, but please document it, e.g. `} else { /* fall through and treat as part of the macro body */}`
> It is deliberate. In fact, it is intended to address [this previous comment](https://reviews.llvm.org/D72041#inline-663000). Please let me know if I've misunderstood and the solution doesn't match the request.
> 
> I'll add a comment as you suggested.
Yeah I think the behavior is great, as is the implementation :-) just a little surprising.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72041/new/

https://reviews.llvm.org/D72041