hokein 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);
----------------
ilya-biryukov wrote:
> 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.
> We're using getSpellingLoc here and getFileLoc later. Why not use getFileLoc
> everywhere?
There are two things in SymbolCollector:
- symbols & ranking signals, we use spelling location for them, the code is
part of this, `ReferencedDecls` is used to calculate the ranking
- references
this patch only targets the reference part (changing the loc here would break
many assumptions I think, and there was a failure test).
================
Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:655
+ void test() {
+ TYPE([[Foo]]) foo;
+ [[FOO]] foo2;
----------------
ilya-biryukov wrote:
> 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`
Added tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70480/new/
https://reviews.llvm.org/D70480
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
