[PATCH] D70480: [clangd] Use expansion location when the ref is inside macros.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:280
       (Roles & static_cast<unsigned>(index::SymbolRole::Reference)) &&
-      SM.getFileID(SpellingLoc) == SM.getMainFileID())
+      SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
     ReferencedDecls.insert(ND);
----------------
We're using `getSpellingLoc` here and `getFileLoc` later. Why not use `getFileLoc` everywhere?

Having a variable (similar to the `SpellingLoc` we had before) and calling `getFileLoc` only once also seems preferable.


================
Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:655
+    void test() {
+      TYPE([[Foo]]) foo;
+      [[FOO]] foo2;
----------------
Could you also check:
1. Multi-level macro arguments `TYPE(TYPE(FOO))`
2. Concatenated tokens as identifiers, e.g. coming from `#define CAT(X, Y) X##Y`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70480