sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Thanks! This seems totally complete now, though I bet there's another case
possible somehow!
================
Comment at: clang-tools-extra/clangd/FindSymbols.cpp:157
+ SourceLocation FallbackNameLoc;
+ if (isSpelledInSource(Loc, SM)) {
+ // Prefer the spelling loc, but save the expansion loc as a fallback.
----------------
Hmm... the common file-location case takes a pretty weird path here: both
NameLoc and FallbackNameLoc end up set to Loc, and hopefully we never hit the
!contains condition (but if we did, we'd just calculate it twice and then hit
the second fallback)
Maybe clearer, though a little longer, to handle explicitly like
```
SourceLocation NameLoc = ND.getLocation();
SourceLocation FallbackNameLoc;
if (NameLoc.isMacroLocation()) {
if (spelled in source) {
NameLoc = getSpelling
FallbackNameLoc = getExpanson
} else {
NameLoc = getExpansion
}
}
```
up to you
================
Comment at: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp:637
+ AllOf(WithName("Test"), SymNameRange(Main.range("expansion2"))),
+ AllOf(WithName("waldo"),
+ SymRangeContains(Main.range("bodyStatement")))));
----------------
nit: despite the "contains" relation being critical, I think it's worth
asserting the exact range
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88463/new/
https://reviews.llvm.org/D88463
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
