[PATCH] D88463: [clangd] Try harder to get accurate ranges for documentSymbols in macros
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 12 02:57:29 PDT 2020
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
More information about the cfe-commits
mailing list