[clangd] Report declaration references in findExplicitReferences.

[merge_guards_bot]

Bulid results are available at http://results.llvm-merge-guard.org/Phabricator-36

See http://jenkins.llvm-merge-guard.org/job/Phabricator/36/ for more details.

[ilya-biryukov]

Mostly LG, but please take a look at the NITs and the implementation-related comments.

clang-tools-extra/clangd/FindTarget.cpp

415

This should return SmallVector<ReferenceLoc, 2> now, some declarations can have both decl and non-decl references.

418

We don't need this flag, in every case we now whether a reference points to a decl or not.

455

Similar to DeclaratorDecl, let's handle this in VisitNamedDecl

460

This should be handled in VisitNamedDecl, we should use a helper similar to getQualifier from AST.h to get the qualifier loc instead.

465

NIT: don't use negative flags, they are hard to read. Prefer IsDeclRef

469

Why do we need this in the first place?
Expressions never declare anything, just pass false here

497

Same here: types don't declare anything normally, remove this field and use false directly instead.

675

NIT: please add /*IsDecl=*/ comment here and in other places to make reading the code simpler.

762

NIT: maybe use declaration instead?
ref is definitely redundant, we know we're printing a reference.

clang-tools-extra/clangd/FindTarget.h

90

Initialize to make sure we can't ever have UB when using this struct.

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

528

Why do we need this? Can't we test everything we do by putting into foo?

The original idea was to avoid too much output in the tests by letting the setup code live outside the function that's being annotated.
Any reason why we have to do the full file here?

I know it's a weird limitation, but that's one that was already made and I'd keep it this way if it's possible.

594

We should keep these non-declaration references.
The actual using namespace declaration does not have a name, so we don't report a declaration there

[hokein]

address comments:

• remove the confusing IsDeclRef
• use getQualifier from AST.h

[hokein]

clang-tools-extra/clangd/FindTarget.cpp

415

Can you give some examples? It seems that non-decl references are handled by other refIn* functions.

int Foo = OtherBar; // OtherBar is captured at the momment.

460

good point, didn't aware of this utility method, exposed it in AST.h.

762

changed to decl.

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

528

I tried it but failed.

We can't test the qualified definitions (see example below) if we put everything into function foo (as defining a namespace is not allowed inside a function)

namespace $0^ns {
class $1^Foo {
 void $2^method();
};
void $3^func();
}
void $4^ns::$5^Foo::$6^method() {}
void $7^ns::$8^func() {}

[ilya-biryukov]

clang-tools-extra/clangd/FindTarget.cpp

415

namespace $1[[a]] = $2[[std]];

$1 is a declaration of a namespace alias.
$2 is a reference to namespace std.

[ilya-biryukov]

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

528

Fair point, but this patch does not add support for references in qualifiers of declarations.
I'd keep this change away.

We can totally test all things added by this patch inside the function body.

Could you split the part that adds tests for qualifiers into a separate change?
That would both make the current change more focused and allow to discuss alternatives for the testing story independently of the functionality we're adding here.

[kadircet]

clang-tools-extra/clangd/AST.cpp

114

could you move this one back above getQualifierLoc ?

clang-tools-extra/clangd/FindTarget.cpp

415

I was just about to make the same point. After this patch lands I would like to detect using namespaces inside function body, and currently there is no way to distinguish between a usingdirective and a namespacealias. Since both is only referencing the target namespace.

So even if this starts reporting $2 in the comment above, there should be a difference between that and $3 in using namespace $3[[std]];

[ilya-biryukov]

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

849

Could we avoid adding this test in this revision?
We could add it in a follow-up patch along with the fix.

853

Could we avoid reporting destructor references for now? I feel they would create more confusion than bring value.
Mostly worried that they overlap with a reference to the type name and now all client code will probably want to filter out destructor references.

I believe they're not critical to any of the use-cases we have so far, having a `// FIXME: decide how to surface destructors when we need them should be good enough for now

[ilya-biryukov]

clang-tools-extra/clangd/FindTarget.cpp

437

This one is a not a declaration reference.
We could call VisitNamedDecl here to report the actual declaration.

Note that this would require returning two results from this function.

[ilya-biryukov]

clang-tools-extra/clangd/FindTarget.cpp

415

That's intentional, though. When we report references, we deliberately leave out information on where it's coming from - otherwise it's hard to encode it.

However, if we really need that, we could also pass a NamedDecl* when reporting declarations (that would mean a slightly different interface, though).

We still have some complexity budget in findExplicitReferences and we should probably spend it here.
OTOH, adding more stuff will probably bloat it too much, I hope we'll stop there... There will still be cases when one would have to write a separate AST traversal to do more complicated stuff. It's unfortunate that this is twice as expensive, but it shouldn't matter in most cases (e.g. when running inside Tweak::apply)

I'd suggest we keep this patch with IsDecl flag, but happy to look into your use-case with using namespaces more closely...

[hokein]

address comments.

[hokein]

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

853

Agree. it is not needed in rename feature as well, we have a typeLoc occurrence in ~[[Foo]].

[ilya-biryukov]

Mostly NITs from my side, the change LG, thanks!

clang-tools-extra/clangd/FindTarget.cpp

422

Same comment as below: could you say ReferenceLoc explicitly here and in other places?
Plain initializer lists are somewhat hard to parse

438

NIT: maybe put this comment on a previous line?
Should read more naturally.

672

I'd probably suggest to return SmallVector<ReferenceLoc, 2> from all functions, so that they compose better in this situation.

Although Optional<ReferenceLoc> captures the semantics of some of those better, I don't think it's particularly useful at this point.
And having to deconstruct all optionals here leads to a lot of clunky code, it'd be much better if we could keep the structure the same as it's used to be.

673

NIT: add braces around an inner multi-line if?

675

Could we have {ReferenceLoc{ instead of {{?
Plain initializer lists are hard to comprehend, especially when they're nested.

681

Remove this comment? Seems to be a leftover from experiments

clang-tools-extra/clangd/unittests/XRefsTests.cpp

2280–2281

This looks unrelated. Maybe revert and land separately?

[kadircet]

clang-tools-extra/clangd/FindTarget.cpp

415

as discussed offline, it is a special enough use-case to handle in define-inline.

[hokein]

address remaining nits.

clang-tools-extra/clangd/unittests/XRefsTests.cpp

2280–2281

This patch also modifies the getNonLocalDeclRefs which is related to the test here. I'd keep the change in this patch as it is minor and NFC.

[ilya-biryukov]

clang-tools-extra/clangd/FindTarget.cpp

496

Could we keep an llvm::Optional<> here in the visitor?
The logic in VisitElaboratedTypeLoc looks cheesy if there's more than 1 reference stored...

The function can keep returning SmallVector, that's totally fine.

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

603

Similar to using namespace, this should be a non-decl reference.

[hokein]

address comments.

[ilya-biryukov]

LGTM. Many thanks!

[merge_guards_bot]

Build result: fail - 33449 tests passed, 2 failed and 463 were skipped.

failed: LLVM.tools/llvm-ar/mri-utf8.test
failed: LLVM.tools/llvm-objdump/X86/disassemble-functions.test

Log files: cmake-log.txt, ninja_check_all-log.txt, CMakeCache.txt

clang-tools-extra/clangd/AST.h

 ///   template <> struct vector<bool> {}; // <-- explicit specialization
 ///
 /// Note that explicit instantiations are NOT explicit specializations, albeit
 /// they look similar.
 ///   template struct vector<bool>; // <-- explicit instantiation, NOT an
 ///   explicit specialization.
 bool isExplicitTemplateSpecialization(const NamedDecl *D);
 
+/// Returns a nested name specifier loc of \p ND if it was present in the
+/// source, e.g.
+///     void ns::something::foo() -> returns 'ns::something'
+///     void foo() -> returns null
+NestedNameSpecifierLoc getQualifierLoc(const NamedDecl &ND);
+
 } // namespace clangd
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_

clang-tools-extra/clangd/AST.cpp

std::string printQualifiedName(const NamedDecl &ND) {
   assert(!StringRef(QName).startswith("::"));
   return QName;
 }
 
 static bool isAnonymous(const DeclarationName &N) {
   return N.isIdentifier() && !N.getAsIdentifierInfo();
 }
 
-/// Returns a nested name specifier of \p ND if it was present in the source,
-/// e.g.
-///     void ns::something::foo() -> returns 'ns::something'
-///     void foo() -> returns null
-static NestedNameSpecifier *getQualifier(const NamedDecl &ND) {
+NestedNameSpecifierLoc getQualifierLoc(const NamedDecl &ND) {
   if (auto *V = llvm::dyn_cast<DeclaratorDecl>(&ND))
-    return V->getQualifier();
+    return V->getQualifierLoc();
   if (auto *T = llvm::dyn_cast<TagDecl>(&ND))
-    return T->getQualifier();
-  return nullptr;
+    return T->getQualifierLoc();
+  return NestedNameSpecifierLoc();
 }
 
 std::string printName(const ASTContext &Ctx, const NamedDecl &ND) {
   std::string Name;
   llvm::raw_string_ostream Out(Name);
   PrintingPolicy PP(Ctx.getLangOpts());
 
   // Handle 'using namespace'. They all have the same name - <using-directive>.
   if (auto *UD = llvm::dyn_cast<UsingDirectiveDecl>(&ND)) {
     Out << "using namespace ";
     if (auto *Qual = UD->getQualifier())
if (isAnonymous(ND.getDeclName())) {
     if (auto *Cls = llvm::dyn_cast<RecordDecl>(&ND))
       return ("(anonymous " + Cls->getKindName() + ")").str();
     if (isa<EnumDecl>(ND))
       return "(anonymous enum)";
     return "(anonymous)";
   }
 
   // Print nested name qualifier if it was written in the source code.
-  if (auto *Qualifier = getQualifier(ND))
+  if (auto *Qualifier = getQualifierLoc(ND).getNestedNameSpecifier())
     Qualifier->print(Out, PP);
   // Print the name itself.
   ND.getDeclName().print(Out, PP);
   // Print template arguments.
   Out << printTemplateSpecializationArgs(ND);
 
   return Out.str();
 }

clang-tools-extra/clangd/FindTarget.h

 /// Information about a reference written in the source code, independent of the
 /// actual AST node that this reference lives in.
 /// Useful for tools that are source-aware, e.g. refactorings.
 struct ReferenceLoc {
   /// Contains qualifier written in the code, if any, e.g. 'ns::' for 'ns::foo'.
   NestedNameSpecifierLoc Qualifier;
   /// Start location of the last name part, i.e. 'foo' in 'ns::foo<int>'.
   SourceLocation NameLoc;
+  /// True if the reference is a declaration or definition;
+  bool IsDecl = false;
   // FIXME: add info about template arguments.
   /// A list of targets referenced by this name. Normally this has a single
   /// element, but multiple is also possible, e.g. in case of using declarations
   /// or unresolved overloaded functions.
   /// For dependent and unresolved references, Targets can also be empty.
   llvm::SmallVector<const NamedDecl *, 1> Targets;
 };
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, ReferenceLoc R);

clang-tools-extra/clangd/FindTarget.cpp

for (auto &D : Decls) {
     Targets.push_back(llvm::cast<NamedDecl>(D.first));
   }
   if (!SeenTemplateInstantiations)
     Targets.insert(Targets.end(), TemplatePatterns.begin(),
                    TemplatePatterns.end());
   return Targets;
 }
 
-Optional<ReferenceLoc> refInDecl(const Decl *D) {
+llvm::SmallVector<ReferenceLoc, 2> refInDecl(const Decl *D) {
   struct Visitor : ConstDeclVisitor<Visitor> {
-    llvm::Optional<ReferenceLoc> Ref;
+    llvm::SmallVector<ReferenceLoc, 2> Refs;
 
     void VisitUsingDirectiveDecl(const UsingDirectiveDecl *D) {
-      Ref = ReferenceLoc{D->getQualifierLoc(),
+      // We want to keep it as non-declaration references, as the
+      // "using namespace" declaration doesn't have a name.
+      Refs.push_back(ReferenceLoc{D->getQualifierLoc(),
                                   D->getIdentLocation(),
-                         {D->getNominatedNamespaceAsWritten()}};
+                                  /*IsDecl=*/false,
+                                  {D->getNominatedNamespaceAsWritten()}});
     }
 
     void VisitUsingDecl(const UsingDecl *D) {
-      Ref = ReferenceLoc{D->getQualifierLoc(), D->getLocation(),
-                         explicitReferenceTargets(DynTypedNode::create(*D),
-                                                  DeclRelation::Underlying)};
+      // "using ns::identifer;" is a non-declaration reference.
+      Refs.push_back(
+          ReferenceLoc{D->getQualifierLoc(), D->getLocation(), /*IsDecl=*/false,
+                                                DeclRelation::Underlying)});
     }
 
     void VisitNamespaceAliasDecl(const NamespaceAliasDecl *D) {
-      Ref = ReferenceLoc{D->getQualifierLoc(),
+      // For namespace alias, "namespace Foo = Target;", we add two references.
+      // Add a declaration reference for Foo.
+      VisitNamedDecl(D);
+      // Add a non-declaration reference for Target.
+      Refs.push_back(ReferenceLoc{D->getQualifierLoc(),
                                   D->getTargetNameLoc(),
-                         {D->getAliasedNamespace()}};
+                                  /*IsDecl=*/false,
+                                  {D->getAliasedNamespace()}});
+    }
+
+    void VisitNamedDecl(const NamedDecl *ND) {
+      // FIXME: decide on how to surface destructors when we need them.
+      if (llvm::isa<CXXDestructorDecl>(ND))
+        return;
+      Refs.push_back(ReferenceLoc{
+          getQualifierLoc(*ND), ND->getLocation(), /*IsDecl=*/true, {ND}});
     }
   };
 
   Visitor V;
   V.Visit(D);
-  return V.Ref;
+  return V.Refs;
 }
 
-Optional<ReferenceLoc> refInExpr(const Expr *E) {
+llvm::SmallVector<ReferenceLoc, 2> refInExpr(const Expr *E) {
   struct Visitor : ConstStmtVisitor<Visitor> {
     // FIXME: handle more complicated cases, e.g. ObjC, designated initializers.
-    llvm::Optional<ReferenceLoc> Ref;
+    llvm::SmallVector<ReferenceLoc, 2> Refs;
 
     void VisitDeclRefExpr(const DeclRefExpr *E) {
-      Ref = ReferenceLoc{
-          E->getQualifierLoc(), E->getNameInfo().getLoc(), {E->getFoundDecl()}};
+      Refs.push_back(ReferenceLoc{E->getQualifierLoc(),
+                                  E->getNameInfo().getLoc(),
+                                  /*IsDecl=*/false,
+                                  {E->getFoundDecl()}});
     }
 
     void VisitMemberExpr(const MemberExpr *E) {
-      Ref = ReferenceLoc{E->getQualifierLoc(),
+      Refs.push_back(ReferenceLoc{E->getQualifierLoc(),
                                   E->getMemberNameInfo().getLoc(),
-                         {E->getFoundDecl()}};
+                                  /*IsDecl=*/false,
+                                  {E->getFoundDecl()}});
     }
 
     void VisitOverloadExpr(const OverloadExpr *E) {
-      Ref = ReferenceLoc{E->getQualifierLoc(), E->getNameInfo().getLoc(),
+      Refs.push_back(ReferenceLoc{E->getQualifierLoc(),
+                                  E->getNameInfo().getLoc(),
+                                  /*IsDecl=*/false,
                                   llvm::SmallVector<const NamedDecl *, 1>(
-                             E->decls().begin(), E->decls().end())};
+                                      E->decls().begin(), E->decls().end())});
     }
   };
 
   Visitor V;
   V.Visit(E);
-  return V.Ref;
+  return V.Refs;
 }
 
-Optional<ReferenceLoc> refInTypeLoc(TypeLoc L) {
+llvm::SmallVector<ReferenceLoc, 2> refInTypeLoc(TypeLoc L) {
   struct Visitor : TypeLocVisitor<Visitor> {
     llvm::Optional<ReferenceLoc> Ref;
 
     void VisitElaboratedTypeLoc(ElaboratedTypeLoc L) {
       // We only know about qualifier, rest if filled by inner locations.
       Visit(L.getNamedTypeLoc().getUnqualifiedLoc());
       // Fill in the qualifier.
       if (!Ref)
         return;
       assert(!Ref->Qualifier.hasQualifier() && "qualifier already set");
       Ref->Qualifier = L.getQualifierLoc();
     }
 
     void VisitTagTypeLoc(TagTypeLoc L) {
-      Ref =
-          ReferenceLoc{NestedNameSpecifierLoc(), L.getNameLoc(), {L.getDecl()}};
+      Ref = ReferenceLoc{NestedNameSpecifierLoc(),
+                         L.getNameLoc(),
+                         /*IsDecl=*/false,
+                         {L.getDecl()}};
     }
 
     void VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc L) {
-      Ref =
-          ReferenceLoc{NestedNameSpecifierLoc(), L.getNameLoc(), {L.getDecl()}};
+      Ref = ReferenceLoc{NestedNameSpecifierLoc(),
+                         L.getNameLoc(),
+                         /*IsDecl=*/false,
+                         {L.getDecl()}};
     }
 
     void VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc L) {
       // We must ensure template type aliases are included in results if they
       // were written in the source code, e.g. in
       //    template <class T> using valias = vector<T>;
       //    ^valias<int> x;
       // 'explicitReferenceTargets' will return:
       //    1. valias with mask 'Alias'.
       //    2. 'vector<int>' with mask 'Underlying'.
       //  we want to return only #1 in this case.
       Ref = ReferenceLoc{
-          NestedNameSpecifierLoc(), L.getTemplateNameLoc(),
+          NestedNameSpecifierLoc(), L.getTemplateNameLoc(), /*IsDecl=*/false,
           explicitReferenceTargets(DynTypedNode::create(L.getType()),
                                    DeclRelation::Alias)};
     }
     void VisitDeducedTemplateSpecializationTypeLoc(
         DeducedTemplateSpecializationTypeLoc L) {
       Ref = ReferenceLoc{
-          NestedNameSpecifierLoc(), L.getNameLoc(),
+          NestedNameSpecifierLoc(), L.getNameLoc(), /*IsDecl=*/false,
           explicitReferenceTargets(DynTypedNode::create(L.getType()),
                                    DeclRelation::Alias)};
     }
 
     void VisitDependentTemplateSpecializationTypeLoc(
         DependentTemplateSpecializationTypeLoc L) {
       Ref = ReferenceLoc{
-          L.getQualifierLoc(), L.getTemplateNameLoc(),
+          L.getQualifierLoc(), L.getTemplateNameLoc(), /*IsDecl=*/false,
           explicitReferenceTargets(DynTypedNode::create(L.getType()))};
     }
 
     void VisitDependentNameTypeLoc(DependentNameTypeLoc L) {
       Ref = ReferenceLoc{
-          L.getQualifierLoc(), L.getNameLoc(),
+          L.getQualifierLoc(), L.getNameLoc(), /*IsDecl=*/false,
           explicitReferenceTargets(DynTypedNode::create(L.getType()))};
     }
 
     void VisitTypedefTypeLoc(TypedefTypeLoc L) {
-      Ref = ReferenceLoc{
-          NestedNameSpecifierLoc(), L.getNameLoc(), {L.getTypedefNameDecl()}};
+      Ref = ReferenceLoc{NestedNameSpecifierLoc(),
+                         L.getNameLoc(),
+                         /*IsDecl=*/false,
+                         {L.getTypedefNameDecl()}};
     }
   };
 
   Visitor V;
   V.Visit(L.getUnqualifiedLoc());
-  return V.Ref;
+  if (!V.Ref)
+    return {};
+  return {*V.Ref};
 }
 
 class ExplicitReferenceColletor
     : public RecursiveASTVisitor<ExplicitReferenceColletor> {
 public:
   ExplicitReferenceColletor(llvm::function_ref<void(ReferenceLoc)> Out)
       : Out(Out) {
     assert(Out);
public:
   // TemplateArgumentLoc is the only way to get locations for references to
   // template template parameters.
   bool TraverseTemplateArgumentLoc(TemplateArgumentLoc A) {
     switch (A.getArgument().getKind()) {
     case TemplateArgument::Template:
     case TemplateArgument::TemplateExpansion:
       reportReference(ReferenceLoc{A.getTemplateQualifierLoc(),
                                    A.getTemplateNameLoc(),
+                                   /*IsDecl=*/false,
                                    {A.getArgument()
                                         .getAsTemplateOrTemplatePattern()
                                         .getAsTemplateDecl()}},
                       DynTypedNode::create(A.getArgument()));
       break;
     case TemplateArgument::Declaration:
       break; // FIXME: can this actually happen in TemplateArgumentLoc?
     case TemplateArgument::Integral:
private:
   ///   - for implicitly generated nodes (e.g. MemberExpr from range-based-for),
   ///     source location information may be missing,
   ///   - for dependent code, targets may be empty.
   ///
   /// (!) For the purposes of this function declarations are not considered to
   ///     be references. However, declarations can have references inside them,
   ///     e.g. 'namespace foo = std' references namespace 'std' and this
   ///     function will return the corresponding reference.
-  llvm::Optional<ReferenceLoc> explicitReference(DynTypedNode N) {
+  llvm::SmallVector<ReferenceLoc, 2> explicitReference(DynTypedNode N) {
     if (auto *D = N.get<Decl>())
       return refInDecl(D);
     if (auto *E = N.get<Expr>())
       return refInExpr(E);
     if (auto *NNSL = N.get<NestedNameSpecifierLoc>())
-      return ReferenceLoc{NNSL->getPrefix(), NNSL->getLocalBeginLoc(),
+      return {ReferenceLoc{NNSL->getPrefix(), NNSL->getLocalBeginLoc(), false,
                            explicitReferenceTargets(DynTypedNode::create(
-                              *NNSL->getNestedNameSpecifier()))};
+                               *NNSL->getNestedNameSpecifier()))}};
     if (const TypeLoc *TL = N.get<TypeLoc>())
       return refInTypeLoc(*TL);
     if (const CXXCtorInitializer *CCI = N.get<CXXCtorInitializer>()) {
       if (CCI->isBaseInitializer())
         return refInTypeLoc(CCI->getBaseClassLoc());
       assert(CCI->isAnyMemberInitializer());
-      return ReferenceLoc{NestedNameSpecifierLoc(),
+      return {ReferenceLoc{NestedNameSpecifierLoc(),
                            CCI->getMemberLocation(),
-                          {CCI->getAnyMember()}};
+                           /*IsDecl=*/false,
+                           {CCI->getAnyMember()}}};
     }
     // We do not have location information for other nodes (QualType, etc)
-    return llvm::None;
+    return {};
   }
 
   void visitNode(DynTypedNode N) {
-    auto Ref = explicitReference(N);
-    if (!Ref)
-      return;
-    reportReference(*Ref, N);
+    for (const auto &R : explicitReference(N))
+      reportReference(R, N);
   }
 
   void reportReference(const ReferenceLoc &Ref, DynTypedNode N) {
     // Our promise is to return only references from the source code. If we lack
     // location information, skip these nodes.
     // Normally this should not happen in practice, unless there are bugs in the
     // traversals or users started the traversal at an implicit node.
     if (Ref.NameLoc.isInvalid()) {
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, ReferenceLoc R) {
   }
   OS << "}";
   if (R.Qualifier) {
     OS << ", qualifier = '";
     R.Qualifier.getNestedNameSpecifier()->print(OS,
                                                 PrintingPolicy(LangOptions()));
     OS << "'";
   }
+  if (R.IsDecl)
+    OS << ", decl";
   return OS;
 }
 
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/XRefs.cpp

 
 llvm::DenseSet<const Decl *> getNonLocalDeclRefs(ParsedAST &AST,
                                                  const FunctionDecl *FD) {
   if (!FD->hasBody())
     return {};
   llvm::DenseSet<const Decl *> DeclRefs;
   findExplicitReferences(FD, [&](ReferenceLoc Ref) {
     for (const Decl *D : Ref.Targets) {
-      if (!index::isFunctionLocalSymbol(D) && !D->isTemplateParameter())
+      if (!index::isFunctionLocalSymbol(D) && !D->isTemplateParameter() &&
+          !Ref.IsDecl)
         DeclRefs.insert(D);
     }
   });
   return DeclRefs;
 }
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

AllRefs annotateReferencesInFoo(llvm::StringRef Code) {
     findExplicitReferences(Func.getBody(), [&Refs](ReferenceLoc R) {
       Refs.push_back(std::move(R));
     });
 
     auto &SM = AST.getSourceManager();
     llvm::sort(Refs, [&](const ReferenceLoc &L, const ReferenceLoc &R) {
       return SM.isBeforeInTranslationUnit(L.NameLoc, R.NameLoc);
     });
 
     std::string AnnotatedCode;
     unsigned NextCodeChar = 0;
     for (unsigned I = 0; I < Refs.size(); ++I) {
       auto &R = Refs[I];
 
       SourceLocation Pos = R.NameLoc;
       assert(Pos.isValid());
       if (Pos.isMacroID()) // FIXME: figure out how to show macro locations.
std::pair</*Code*/ llvm::StringRef, /*References*/ llvm::StringRef> Cases[] =
           namespace ns {}
           namespace alias = ns;
           void foo() {
             using namespace $0^ns;
             using namespace $1^alias;
           }
         )cpp",
            "0: targets = {ns}\n"
            "1: targets = {alias}\n"},
           // Using declarations.
           {R"cpp(
           namespace ns { int global; }
           void foo() {
             using $0^ns::$1^global;
           }
         )cpp",
            "0: targets = {ns}\n"
            "1: targets = {ns::global}, qualifier = 'ns::'\n"},
           // Simple types.
           {R"cpp(
          struct Struct { int a; };
          using Typedef = int;
          void foo() {
-           $0^Struct x;
-           $1^Typedef y;
-           static_cast<$2^Struct*>(0);
+           $0^Struct $1^x;
+           $2^Typedef $3^y;
+           static_cast<$4^Struct*>(0);
          }
        )cpp",
            "0: targets = {Struct}\n"
-           "1: targets = {Typedef}\n"
-           "2: targets = {Struct}\n"},
+           "1: targets = {x}, decl\n"
+           "2: targets = {Typedef}\n"
+           "3: targets = {y}, decl\n"
+           "4: targets = {Struct}\n"},
           // Name qualifiers.
           {R"cpp(
          namespace a { namespace b { struct S { typedef int type; }; } }
          void foo() {
-           $0^a::$1^b::$2^S x;
-           using namespace $3^a::$4^b;
-           $5^S::$6^type y;
+           $0^a::$1^b::$2^S $3^x;
+           using namespace $4^a::$5^b;
+           $6^S::$7^type $8^y;
          }
         )cpp",
            "0: targets = {a}\n"
            "1: targets = {a::b}, qualifier = 'a::'\n"
            "2: targets = {a::b::S}, qualifier = 'a::b::'\n"
-           "3: targets = {a}\n"
-           "4: targets = {a::b}, qualifier = 'a::'\n"
-           "5: targets = {a::b::S}\n"
-           "6: targets = {a::b::S::type}, qualifier = 'struct S::'\n"},
+           "3: targets = {x}, decl\n"
+           "4: targets = {a}\n"
+           "5: targets = {a::b}, qualifier = 'a::'\n"
+           "6: targets = {a::b::S}\n"
+           "7: targets = {a::b::S::type}, qualifier = 'struct S::'\n"
+           "8: targets = {y}, decl\n"},
           // Simple templates.
           {R"cpp(
           template <class T> struct vector { using value_type = T; };
           template <> struct vector<bool> { using value_type = bool; };
           void foo() {
-            $0^vector<int> vi;
-            $1^vector<bool> vb;
+            $0^vector<int> $1^vi;
+            $2^vector<bool> $3^vb;
           }
         )cpp",
            "0: targets = {vector<int>}\n"
-           "1: targets = {vector<bool>}\n"},
+           "1: targets = {vi}, decl\n"
+           "2: targets = {vector<bool>}\n"
+           "3: targets = {vb}, decl\n"},
           // Template type aliases.
           {R"cpp(
             template <class T> struct vector { using value_type = T; };
             template <> struct vector<bool> { using value_type = bool; };
             template <class T> using valias = vector<T>;
             void foo() {
-              $0^valias<int> vi;
-              $1^valias<bool> vb;
+              $0^valias<int> $1^vi;
+              $2^valias<bool> $3^vb;
             }
           )cpp",
            "0: targets = {valias}\n"
-           "1: targets = {valias}\n"},
+           "1: targets = {vi}, decl\n"
+           "2: targets = {valias}\n"
+           "3: targets = {vb}, decl\n"},
           // MemberExpr should know their using declaration.
           {R"cpp(
             struct X { void func(int); }
             struct Y : X {
               using X::func;
             };
             void foo(Y y) {
               $0^y.$1^func(1);
std::pair</*Code*/ llvm::StringRef, /*References*/ llvm::StringRef> Cases[] =
           // No references from implicit nodes.
           {R"cpp(
             struct vector {
               int *begin();
               int *end();
             };
 
             void foo() {
-              for (int x : $0^vector()) {
-                $1^x = 10;
+              for (int $0^x : $1^vector()) {
+                $2^x = 10;
               }
             }
         )cpp",
-           "0: targets = {vector}\n"
-           "1: targets = {x}\n"},
+           "0: targets = {x}, decl\n"
+           "1: targets = {vector}\n"
+           "2: targets = {x}\n"},
           // Handle UnresolvedLookupExpr.
           {R"cpp(
             namespace ns1 { void func(char*); }
             namespace ns2 { void func(int*); }
             using namespace ns1;
             using namespace ns2;
 
             template <class T>
std::pair</*Code*/ llvm::StringRef, /*References*/ llvm::StringRef> Cases[] =
            "1: targets = {X::func, X::func}\n"
            "2: targets = {t}\n"},
           // Type template parameters.
           {R"cpp(
             template <class T>
             void foo() {
               static_cast<$0^T>(0);
               $1^T();
-              $2^T t;
+              $2^T $3^t;
             }
         )cpp",
            "0: targets = {T}\n"
            "1: targets = {T}\n"
-           "2: targets = {T}\n"},
+           "2: targets = {T}\n"
+           "3: targets = {t}, decl\n"},
           // Non-type template parameters.
           {R"cpp(
             template <int I>
             void foo() {
-              int x = $0^I;
+              int $0^x = $1^I;
             }
         )cpp",
-           "0: targets = {I}\n"},
+           "0: targets = {x}, decl\n"
+           "1: targets = {I}\n"},
           // Template template parameters.
           {R"cpp(
             template <class T> struct vector {};
 
             template <template<class> class TT, template<class> class ...TP>
             void foo() {
-              $0^TT<int> x;
-              $1^foo<$2^TT>();
-              $3^foo<$4^vector>()
-              $5^foo<$6^TP...>();
+              $0^TT<int> $1^x;
+              $2^foo<$3^TT>();
+              $4^foo<$5^vector>()
+              $6^foo<$7^TP...>();
             }
         )cpp",
            "0: targets = {TT}\n"
-           "1: targets = {foo}\n"
-           "2: targets = {TT}\n"
-           "3: targets = {foo}\n"
-           "4: targets = {vector}\n"
-           "5: targets = {foo}\n"
-           "6: targets = {TP}\n"},
+           "1: targets = {x}, decl\n"
+           "2: targets = {foo}\n"
+           "3: targets = {TT}\n"
+           "4: targets = {foo}\n"
+           "5: targets = {vector}\n"
+           "6: targets = {foo}\n"
+           "7: targets = {TP}\n"},
           // Non-type template parameters with declarations.
           {R"cpp(
             int func();
             template <int(*)()> struct wrapper {};
 
             template <int(*FuncParam)()>
             void foo() {
-              $0^wrapper<$1^func> w;
-              $2^FuncParam();
+              $0^wrapper<$1^func> $2^w;
+              $3^FuncParam();
             }
         )cpp",
            "0: targets = {wrapper<&func>}\n"
            "1: targets = {func}\n"
-           "2: targets = {FuncParam}\n"},
+           "2: targets = {w}, decl\n"
+           "3: targets = {FuncParam}\n"},
+          {R"cpp(
+             namespace ns {}
+             class S {};
+             void foo() {
+               class $0^Foo { $1^Foo(); ~$2^Foo(); int $3^field; };
+               int $4^Var;
+               enum $5^E { $6^ABC };
+               typedef int $7^INT;
+               using $8^INT2 = int;
+               namespace $9^NS = $10^ns;
+             }
+           )cpp",
+           "0: targets = {Foo}, decl\n"
+           "1: targets = {foo()::Foo::Foo}, decl\n"
+           "2: targets = {Foo}\n"
+           "3: targets = {foo()::Foo::field}, decl\n"
+           "4: targets = {Var}, decl\n"
+           "5: targets = {E}, decl\n"
+           "6: targets = {foo()::ABC}, decl\n"
+           "7: targets = {INT}, decl\n"
+           "8: targets = {INT2}, decl\n"
+           "9: targets = {NS}, decl\n"
+           "10: targets = {ns}\n"},
       };
 
   for (const auto &C : Cases) {
     llvm::StringRef ExpectedCode = C.first;
     llvm::StringRef ExpectedRefs = C.second;
 
     auto Actual =
         annotateReferencesInFoo(llvm::Annotations(ExpectedCode).code());
     EXPECT_EQ(ExpectedCode, Actual.AnnotatedCode);
     EXPECT_EQ(ExpectedRefs, Actual.DumpedReferences) << ExpectedCode;
   }
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/unittests/XRefsTests.cpp

for (const Case &C : Cases) {
     ASSERT_NE(FD, nullptr);
 
     auto NonLocalDeclRefs = getNonLocalDeclRefs(AST, FD);
     std::vector<std::string> Names;
     for (const Decl *D : NonLocalDeclRefs) {
       if (const auto *ND = llvm::dyn_cast<NamedDecl>(D))
         Names.push_back(ND->getQualifiedNameAsString());
     }
-    EXPECT_THAT(Names, UnorderedElementsAreArray(C.ExpectedDecls));
+    EXPECT_THAT(Names, UnorderedElementsAreArray(C.ExpectedDecls))
+        << File.code();
   }
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang