From f24649b77d856157c64841457dcc4f70530d607c Mon Sep 17 00:00:00 2001 From: Haojian Wu Date: Wed, 7 Oct 2020 10:01:04 +0200 Subject: [PATCH] [clangd] Don't set the Underlying bit on targets of UsingDecls. With this patch, we don't treat `using ns::X` as a first-class declaration like `using Z = ns::Y`, reference to X that goes through this using-decl is considered a direct reference (without the Underlying bit). Fix the workaround in https://reviews.llvm.org/D87225 and https://reviews.llvm.org/D74054. Reviewed By: sammccall Differential Revision: https://reviews.llvm.org/D88472 --- clang-tools-extra/clangd/FindTarget.cpp | 6 ++--- clang-tools-extra/clangd/FindTarget.h | 15 ++++++++---- clang-tools-extra/clangd/XRefs.cpp | 23 ------------------- .../clangd/unittests/FindTargetTests.cpp | 13 ++++------- .../clangd/unittests/XRefsTests.cpp | 9 ++++---- 5 files changed, 24 insertions(+), 42 deletions(-) diff --git a/clang-tools-extra/clangd/FindTarget.cpp b/clang-tools-extra/clangd/FindTarget.cpp index 9db814368a024d..4cf62d3d1539c8 100644 --- a/clang-tools-extra/clangd/FindTarget.cpp +++ b/clang-tools-extra/clangd/FindTarget.cpp @@ -342,8 +342,9 @@ struct TargetFinder { add(TND->getUnderlyingType(), Flags | Rel::Underlying); Flags |= Rel::Alias; // continue with the alias. } else if (const UsingDecl *UD = dyn_cast(D)) { + // no Underlying as this is a non-renaming alias. for (const UsingShadowDecl *S : UD->shadows()) - add(S->getUnderlyingDecl(), Flags | Rel::Underlying); + add(S->getUnderlyingDecl(), Flags); Flags |= Rel::Alias; // continue with the alias. } else if (const auto *NAD = dyn_cast(D)) { add(NAD->getUnderlyingDecl(), Flags | Rel::Underlying); @@ -354,7 +355,7 @@ struct TargetFinder { UUVD->getQualifier()->getAsType(), [UUVD](ASTContext &) { return UUVD->getNameInfo().getName(); }, ValueFilter)) { - add(Target, Flags | Rel::Underlying); + add(Target, Flags); // no Underlying as this is a non-renaming alias } Flags |= Rel::Alias; // continue with the alias } else if (const UsingShadowDecl *USD = dyn_cast(D)) { @@ -364,7 +365,6 @@ struct TargetFinder { // Shadow decls are synthetic and not themselves interesting. // Record the underlying decl instead, if allowed. D = USD->getTargetDecl(); - Flags |= Rel::Underlying; // continue with the underlying decl. } else if (const auto *DG = dyn_cast(D)) { D = DG->getDeducedTemplate(); } else if (const ObjCImplementationDecl *IID = diff --git a/clang-tools-extra/clangd/FindTarget.h b/clang-tools-extra/clangd/FindTarget.h index 48ad9e6513bbb3..f328ae358c0655 100644 --- a/clang-tools-extra/clangd/FindTarget.h +++ b/clang-tools-extra/clangd/FindTarget.h @@ -102,13 +102,20 @@ enum class DeclRelation : unsigned { TemplatePattern, // Alias options apply when the declaration is an alias. - // e.g. namespace clang { [[StringRef]] S; } + // e.g. namespace client { [[X]] x; } /// This declaration is an alias that was referred to. - /// e.g. using llvm::StringRef (the UsingDecl directly referenced). + /// e.g. using ns::X (the UsingDecl directly referenced), + /// using Z = ns::Y (the TypeAliasDecl directly referenced) Alias, - /// This is the underlying declaration for an alias, decltype etc. - /// e.g. class llvm::StringRef (the underlying declaration referenced). + /// This is the underlying declaration for a renaming-alias, decltype etc. + /// e.g. class ns::Y (the underlying declaration referenced). + /// + /// Note that we don't treat `using ns::X` as a first-class declaration like + /// `using Z = ns::Y`. Therefore reference to X that goes through this + /// using-decl is considered a direct reference (without the Underlying bit). + /// Nevertheless, we report `using ns::X` as an Alias, so that some features + /// like go-to-definition can still target it. Underlying, }; llvm::raw_ostream &operator<<(llvm::raw_ostream &, DeclRelation); diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 9532e1418cca73..9469ab46c9fc31 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -343,18 +343,6 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier, } } - // Give the underlying decl if navigation is triggered on a non-renaming - // alias. - if (llvm::isa(D) || llvm::isa(D)) { - // FIXME: address more complicated cases. TargetDecl(... Underlying) gives - // all overload candidates, we only want the targeted one if the cursor is - // on an using-alias usage, workround it with getDeclAtPosition. - llvm::for_each( - getDeclAtPosition(AST, CurLoc, DeclRelation::Underlying, NodeKind), - [&](const NamedDecl *UD) { AddResultDecl(UD); }); - continue; - } - // Special case: if the class name is selected, also map Objective-C // categories and category implementations back to their class interface. // @@ -1159,17 +1147,6 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit, DeclRelation::TemplatePattern | DeclRelation::Alias; std::vector Decls = getDeclAtPosition(AST, *CurLoc, Relations); - std::vector NonrenamingAliasUnderlyingDecls; - // If the results include a *non-renaming* alias, get its - // underlying decls as well. (See similar logic in locateASTReferent()). - for (const NamedDecl *D : Decls) { - if (llvm::isa(D) || llvm::isa(D)) { - for (const NamedDecl *AD : - getDeclAtPosition(AST, *CurLoc, DeclRelation::Underlying)) - NonrenamingAliasUnderlyingDecls.push_back(AD); - } - } - llvm::copy(NonrenamingAliasUnderlyingDecls, std::back_inserter(Decls)); // We traverse the AST to find references in the main file. auto MainFileRefs = findRefs(Decls, AST); diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp index 5bfdaaf6c3434c..e4f584bea01f82 100644 --- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp @@ -181,8 +181,7 @@ TEST_F(TargetDeclTest, UsingDecl) { int x = [[f]](42); )cpp"; // f(char) is not referenced! - EXPECT_DECLS("DeclRefExpr", {"using foo::f", Rel::Alias}, - {"int f(int)", Rel::Underlying}); + EXPECT_DECLS("DeclRefExpr", {"using foo::f", Rel::Alias}, {"int f(int)"}); Code = R"cpp( namespace foo { @@ -192,9 +191,8 @@ TEST_F(TargetDeclTest, UsingDecl) { [[using foo::f]]; )cpp"; // All overloads are referenced. - EXPECT_DECLS("UsingDecl", {"using foo::f", Rel::Alias}, - {"int f(int)", Rel::Underlying}, - {"int f(char)", Rel::Underlying}); + EXPECT_DECLS("UsingDecl", {"using foo::f", Rel::Alias}, {"int f(int)"}, + {"int f(char)"}); Code = R"cpp( struct X { @@ -205,8 +203,7 @@ TEST_F(TargetDeclTest, UsingDecl) { }; int x = Y().[[foo]](); )cpp"; - EXPECT_DECLS("MemberExpr", {"using X::foo", Rel::Alias}, - {"int foo()", Rel::Underlying}); + EXPECT_DECLS("MemberExpr", {"using X::foo", Rel::Alias}, {"int foo()"}); Code = R"cpp( template @@ -219,7 +216,7 @@ TEST_F(TargetDeclTest, UsingDecl) { }; )cpp"; EXPECT_DECLS("UnresolvedUsingValueDecl", {"using Base::waldo", Rel::Alias}, - {"void waldo()", Rel::Underlying}); + {"void waldo()"}); } TEST_F(TargetDeclTest, ConstructorInitList) { diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp index 40637b5fa7582f..9f77db39a3e490 100644 --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -1118,17 +1118,18 @@ TEST(LocateSymbol, Alias) { // decls. R"cpp( namespace ns { class [[Foo]] {}; } - using ns::F^oo; + // FIXME: don't return the using decl if it touches the cursor position. + using ns::[[F^oo]]; )cpp", R"cpp( namespace ns { int [[x]](char); int [[x]](double); } - using ns::^x; + using ns::[[^x]]; )cpp", R"cpp( namespace ns { int [[x]](char); int x(double); } - using ns::x; + using ns::[[x]]; int y = ^x('a'); )cpp", @@ -1156,7 +1157,7 @@ TEST(LocateSymbol, Alias) { }; template struct Derived : Base { - using Base::w^aldo; + using Base::[[w^aldo]]; }; )cpp", };