[PATCH] D88472: [clangd] Don't set the Underlying bit on targets of UsingDecls.
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 6 00:17:34 PDT 2020
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
LG, thanks for all the iterations here.
I do want to solve the regression but we can land without that.
================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:345
} else if (const UsingDecl *UD = dyn_cast<UsingDecl>(D)) {
for (const UsingShadowDecl *S : UD->shadows())
+ add(S->getUnderlyingDecl(), Flags);
----------------
maybe a comment here "// no Underlying as this is a non-renaming alias"
================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:357
ValueFilter)) {
- add(Target, Flags | Rel::Underlying);
+ add(Target, Flags);
}
----------------
and here
================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1121
namespace ns { class [[Foo]] {}; }
- using ns::F^oo;
+ using ns::[[F^oo]];
)cpp",
----------------
I'd like to make a plan to solve this regression (we can do it here or in a followup patch, happy to hack on that).
What do you think about the following heuristic:
- if go-to-definition yields multiple results
- and a particular result touches the cursor*
- (optional) and that result is marked with Alias
- then drop it from the set
It means we still keep some special-case code, but it's not complex and fairly intuitive I think.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88472/new/
https://reviews.llvm.org/D88472
More information about the cfe-commits
mailing list