[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