[clangd] targetDecl() returns only NamedDecls.

[sammccall]

Summary

While it's perfectly reasonable for non-named decls such as
static_assert to resolve to themselves:

• nothing else ever resolves to them
• features based on references (hover, highlight, find refs etc) tend to be uninteresting where only trivial references are possible
• returning NamedDecl is a more convenient API (we cast to it in many places)
• this aligns closer to findExplicitReferences/explicitReferenceTargets

This fixes a crash in explicitReferenceTargets: if the target is a
non-named decl then there's an invalid unchecked cast to NamedDecl.

In practice this means when hovering over e.g. a static_assert:

• before ac3f9e4842, we would show a (boring) hover card
• after ac3f9e4842, we would crash
• after this patch, we will show nothing

[merge_guards_bot]

Unit tests: pass. 61180 tests passed, 0 failed and 729 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[ilya-biryukov]

LGTM

clang-tools-extra/clangd/FindTarget.h

 ///                TypedefTypeLoc T
 ///                       |
 ///                 TypedefType T
 ///                    /     \
 ///           [underlying]  [alias]
 ///                  /         \
 ///          RecordDecl S    TypeAliasDecl T
 ///
+/// Note that this function only returns NamedDecls. Generally other decls
+/// don't have references in this sense, just the node itself.
+/// If callers want to support such decls, they should cast the node directly.
+///
 /// FIXME: some AST nodes cannot be DynTypedNodes, these cannot be specified.
-llvm::SmallVector<const Decl *, 1>
+llvm::SmallVector<const NamedDecl *, 1>
 targetDecl(const ast_type_traits::DynTypedNode &, DeclRelationSet Mask);
 
 /// Similar to targetDecl(), however instead of applying a filter, all possible
 /// decls are returned along with their DeclRelationSets.
 /// This is suitable for indexing, where everything is recorded and filtering
 /// is applied later.
-llvm::SmallVector<std::pair<const Decl *, DeclRelationSet>, 1>
+llvm::SmallVector<std::pair<const NamedDecl *, DeclRelationSet>, 1>
 allTargetDecls(const ast_type_traits::DynTypedNode &);
 
 enum class DeclRelation : unsigned {
   // Template options apply when the declaration is an instantiated template.
   // e.g. [[vector<int>]] vec;
 
   /// This is the template instantiation that was referred to.
   /// e.g. template<> class vector<int> (the implicit specialization)

clang-tools-extra/clangd/FindTarget.cpp

 // This affects:
 //  - DependentTemplateSpecializationType,
 //  - DependentNameType
 //  - UnresolvedUsingValueDecl
 //  - UnresolvedUsingTypenameDecl
 struct TargetFinder {
   using RelSet = DeclRelationSet;
   using Rel = DeclRelation;
-  llvm::SmallDenseMap<const Decl *, RelSet> Decls;
+  llvm::SmallDenseMap<const NamedDecl *, RelSet> Decls;
   RelSet Flags;
 
-  static const Decl *getTemplatePattern(const Decl *D) {
+  static const NamedDecl *getTemplatePattern(const NamedDecl *D) {
     if (const CXXRecordDecl *CRD = dyn_cast<CXXRecordDecl>(D)) {
       return CRD->getTemplateInstantiationPattern();
     } else if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
       return FD->getTemplateInstantiationPattern();
     } else if (auto *VD = dyn_cast<VarDecl>(D)) {
       // Hmm: getTIP returns its arg if it's not an instantiation?!
       VarDecl *T = VD->getTemplateInstantiationPattern();
       return (T == D) ? nullptr : T;
     } else if (const auto *ED = dyn_cast<EnumDecl>(D)) {
       return ED->getInstantiatedFromMemberEnum();
     } else if (isa<FieldDecl>(D) || isa<TypedefNameDecl>(D)) {
-      const auto *ND = cast<NamedDecl>(D);
-      if (const DeclContext *Parent = dyn_cast_or_null<DeclContext>(
-              getTemplatePattern(llvm::cast<Decl>(ND->getDeclContext()))))
-        for (const NamedDecl *BaseND : Parent->lookup(ND->getDeclName()))
-          if (!BaseND->isImplicit() && BaseND->getKind() == ND->getKind())
+      if (const auto *Parent = llvm::dyn_cast<NamedDecl>(D->getDeclContext()))
+        if (const DeclContext *ParentPat =
+                dyn_cast_or_null<DeclContext>(getTemplatePattern(Parent)))
+          for (const NamedDecl *BaseND : ParentPat->lookup(D->getDeclName()))
+            if (!BaseND->isImplicit() && BaseND->getKind() == D->getKind())
               return BaseND;
     } else if (const auto *ECD = dyn_cast<EnumConstantDecl>(D)) {
       if (const auto *ED = dyn_cast<EnumDecl>(ECD->getDeclContext())) {
         if (const EnumDecl *Pattern = ED->getInstantiatedFromMemberEnum()) {
           for (const NamedDecl *BaseECD : Pattern->lookup(ECD->getDeclName()))
             return BaseECD;
         }
       }
     }
     return nullptr;
   }
 
   template <typename T> void debug(T &Node, RelSet Flags) {
     dlog("visit [{0}] {1}", Flags,
          nodeToString(ast_type_traits::DynTypedNode::create(Node)));
   }
 
-  void report(const Decl *D, RelSet Flags) {
+  void report(const NamedDecl *D, RelSet Flags) {
     dlog("--> [{0}] {1}", Flags,
          nodeToString(ast_type_traits::DynTypedNode::create(*D)));
     Decls[D] |= Flags;
   }
 
 public:
-  void add(const Decl *D, RelSet Flags) {
+  void add(const Decl *Dcl, RelSet Flags) {
+    const NamedDecl *D = llvm::dyn_cast<NamedDecl>(Dcl);
     if (!D)
       return;
     debug(*D, Flags);
     if (const UsingDirectiveDecl *UDD = llvm::dyn_cast<UsingDirectiveDecl>(D))
       D = UDD->getNominatedNamespaceAsWritten();
 
     if (const TypedefNameDecl *TND = dyn_cast<TypedefNameDecl>(D)) {
       add(TND->getUnderlyingType(), Flags | Rel::Underlying);
void add(const CXXCtorInitializer *CCI, RelSet Flags) {
     if (CCI->isAnyMemberInitializer())
       add(CCI->getAnyMember(), Flags);
     // Constructor calls contain a TypeLoc node, so we don't handle them here.
   }
 };
 
 } // namespace
 
-llvm::SmallVector<std::pair<const Decl *, DeclRelationSet>, 1>
+llvm::SmallVector<std::pair<const NamedDecl *, DeclRelationSet>, 1>
 allTargetDecls(const ast_type_traits::DynTypedNode &N) {
   dlog("allTargetDecls({0})", nodeToString(N));
   TargetFinder Finder;
   DeclRelationSet Flags;
   if (const Decl *D = N.get<Decl>())
     Finder.add(D, Flags);
   else if (const Stmt *S = N.get<Stmt>())
     Finder.add(S, Flags);
   else if (const NestedNameSpecifierLoc *NNSL = N.get<NestedNameSpecifierLoc>())
     Finder.add(NNSL->getNestedNameSpecifier(), Flags);
   else if (const NestedNameSpecifier *NNS = N.get<NestedNameSpecifier>())
     Finder.add(NNS, Flags);
   else if (const TypeLoc *TL = N.get<TypeLoc>())
     Finder.add(TL->getType(), Flags);
   else if (const QualType *QT = N.get<QualType>())
     Finder.add(*QT, Flags);
   else if (const CXXCtorInitializer *CCI = N.get<CXXCtorInitializer>())
     Finder.add(CCI, Flags);
 
   return {Finder.Decls.begin(), Finder.Decls.end()};
 }
 
-llvm::SmallVector<const Decl *, 1>
+llvm::SmallVector<const NamedDecl *, 1>
 targetDecl(const ast_type_traits::DynTypedNode &N, DeclRelationSet Mask) {
-  llvm::SmallVector<const Decl *, 1> Result;
+  llvm::SmallVector<const NamedDecl *, 1> Result;
   for (const auto &Entry : allTargetDecls(N)) {
     if (!(Entry.second & ~Mask))
       Result.push_back(Entry.first);
   }
   return Result;
 }
 
 llvm::SmallVector<const NamedDecl *, 1>
explicitReferenceTargets(DynTypedNode N, DeclRelationSet Mask) {
 
   llvm::SmallVector<const NamedDecl *, 1> TemplatePatterns;
   llvm::SmallVector<const NamedDecl *, 1> Targets;
   bool SeenTemplateInstantiations = false;
   for (auto &D : Decls) {
     if (D.second & ~Mask)
       continue;
     if (D.second & DeclRelation::TemplatePattern) {
-      TemplatePatterns.push_back(llvm::cast<NamedDecl>(D.first));
+      TemplatePatterns.push_back(D.first);
       continue;
     }
     if (D.second & DeclRelation::TemplateInstantiation)
       SeenTemplateInstantiations = true;
-    Targets.push_back(llvm::cast<NamedDecl>(D.first));
+    Targets.push_back(D.first);
   }
   if (!SeenTemplateInstantiations)
     Targets.insert(Targets.end(), TemplatePatterns.begin(),
                    TemplatePatterns.end());
   return Targets;
 }
 
 namespace {

clang-tools-extra/clangd/XRefs.cpp

 namespace {
 
 // Returns the single definition of the entity declared by D, if visible.
 // In particular:
 // - for non-redeclarable kinds (e.g. local vars), return D
 // - for kinds that allow multiple definitions (e.g. namespaces), return nullptr
 // Kinds of nodes that always return nullptr here will not have definitions
 // reported by locateSymbolAt().
-const Decl *getDefinition(const Decl *D) {
+const NamedDecl *getDefinition(const NamedDecl *D) {
   assert(D);
   // Decl has one definition that we can find.
   if (const auto *TD = dyn_cast<TagDecl>(D))
     return TD->getDefinition();
   if (const auto *VD = dyn_cast<VarDecl>(D))
     return VD->getDefinition();
   if (const auto *FD = dyn_cast<FunctionDecl>(D))
     return FD->getDefinition();
SymbolLocation getPreferredLocation(const Location &ASTLoc,
   Symbol ASTSym, IdxSym;
   ASTSym.ID = IdxSym.ID = SymbolID("dummy_id");
   ASTSym.CanonicalDeclaration = toIndexLocation(ASTLoc, Scratch);
   IdxSym.CanonicalDeclaration = IdxLoc;
   auto Merged = mergeSymbol(ASTSym, IdxSym);
   return Merged.CanonicalDeclaration;
 }
 
-std::vector<const Decl *> getDeclAtPosition(ParsedAST &AST, SourceLocation Pos,
+std::vector<const NamedDecl *> getDeclAtPosition(ParsedAST &AST,
+                                                 SourceLocation Pos,
                                                  DeclRelationSet Relations) {
   FileID FID;
   unsigned Offset;
   std::tie(FID, Offset) = AST.getSourceManager().getDecomposedSpellingLoc(Pos);
   SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
-  std::vector<const Decl *> Result;
+  std::vector<const NamedDecl *> Result;
   if (const SelectionTree::Node *N = Selection.commonAncestor()) {
     auto Decls = targetDecl(N->ASTNode, Relations);
     Result.assign(Decls.begin(), Decls.end());
   }
   return Result;
 }
 
 llvm::Optional<Location> makeLocation(ASTContext &AST, SourceLocation TokLoc,
if (auto L = sourceLocationInMainFile(SM, Pos)) {
     elog("locateSymbolAt failed to convert position to source location: {0}",
          L.takeError());
     return Result;
   }
 
   // Emit all symbol locations (declaration or definition) from AST.
   DeclRelationSet Relations =
       DeclRelation::TemplatePattern | DeclRelation::Alias;
-  for (const Decl *D : getDeclAtPosition(AST, SourceLoc, Relations)) {
-    const Decl *Def = getDefinition(D);
-    const Decl *Preferred = Def ? Def : D;
+  for (const NamedDecl *D : getDeclAtPosition(AST, SourceLoc, Relations)) {
+    const NamedDecl *Def = getDefinition(D);
+    const NamedDecl *Preferred = Def ? Def : D;
 
     // If we're at the point of declaration of a template specialization,
     // it's more useful to navigate to the template declaration.
     if (SM.getMacroArgExpandedLocation(Preferred->getLocation()) ==
         IdentStartLoc) {
       if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(Preferred)) {
         D = CTSD->getSpecializedTemplate();
         Def = getDefinition(D);
         Preferred = Def ? Def : D;
       }
     }
 
     auto Loc = makeLocation(AST.getASTContext(), nameLocation(*Preferred, SM),
                             *MainFilePath);
     if (!Loc)
       continue;
 
     Result.emplace_back();
-    if (auto *ND = dyn_cast<NamedDecl>(Preferred))
-      Result.back().Name = printName(AST.getASTContext(), *ND);
+    Result.back().Name = printName(AST.getASTContext(), *D);
     Result.back().PreferredDeclaration = *Loc;
     // Preferred is always a definition if possible, so this check works.
     if (Def == Preferred)
       Result.back().Definition = *Loc;
 
     // Record SymbolID for index lookup later.
     if (auto ID = getSymbolID(Preferred))
       ResultIndex[*ID] = Result.size() - 1;

 class ReferenceFinder : public index::IndexDataConsumer {
 public:
   struct Reference {
     SourceLocation Loc;
     index::SymbolRoleSet Role;
   };
 
   ReferenceFinder(ASTContext &AST, Preprocessor &PP,
-                  const std::vector<const Decl *> &TargetDecls)
+                  const std::vector<const NamedDecl *> &TargetDecls)
       : AST(AST) {
-    for (const Decl *D : TargetDecls)
+    for (const NamedDecl *D : TargetDecls)
       CanonicalTargets.insert(D->getCanonicalDecl());
   }
 
   std::vector<Reference> take() && {
     llvm::sort(References, [](const Reference &L, const Reference &R) {
       return std::tie(L.Loc, L.Role) < std::tie(R.Loc, R.Role);
     });
     // We sometimes see duplicates when parts of the AST get traversed twice.

 
 private:
   llvm::SmallSet<const Decl *, 4> CanonicalTargets;
   std::vector<Reference> References;
   const ASTContext &AST;
 };
 
 std::vector<ReferenceFinder::Reference>
-findRefs(const std::vector<const Decl *> &Decls, ParsedAST &AST) {
+findRefs(const std::vector<const NamedDecl *> &Decls, ParsedAST &AST) {
   ReferenceFinder RefFinder(AST.getASTContext(), AST.getPreprocessor(), Decls);
   index::IndexingOptions IndexOpts;
   IndexOpts.SystemSymbolFilter =
       index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
   IndexOpts.IndexParametersInDeclarations = true;
   IndexOpts.IndexTemplateParameters = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
auto Loc = SM.getMacroArgExpandedLocation(
       getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
 
   std::vector<SymbolDetails> Results;
 
   // We also want the targets of using-decls, so we include
   // DeclRelation::Underlying.
   DeclRelationSet Relations = DeclRelation::TemplatePattern |
                               DeclRelation::Alias | DeclRelation::Underlying;
-  for (const Decl *D : getDeclAtPosition(AST, Loc, Relations)) {
+  for (const NamedDecl *D : getDeclAtPosition(AST, Loc, Relations)) {
     SymbolDetails NewSymbol;
-    if (const NamedDecl *ND = dyn_cast<NamedDecl>(D)) {
-      std::string QName = printQualifiedName(*ND);
-      std::tie(NewSymbol.containerName, NewSymbol.name) =
-          splitQualifiedName(QName);
+    std::string QName = printQualifiedName(*D);
 
-      if (NewSymbol.containerName.empty()) {
-        if (const auto *ParentND =
-                dyn_cast_or_null<NamedDecl>(ND->getDeclContext()))
-          NewSymbol.containerName = printQualifiedName(*ParentND);
-      }
-    }
+              dyn_cast_or_null<NamedDecl>(D->getDeclContext()))
     llvm::SmallString<32> USR;
     if (!index::generateUSRForDecl(D, USR)) {
       NewSymbol.USR = USR.str();
       NewSymbol.ID = SymbolID(NewSymbol.USR);
     }
     Results.push_back(std::move(NewSymbol));
   }
 
const CXXRecordDecl *findRecordTypeAt(ParsedAST &AST, Position Pos) {
   SourceLocation SourceLocationBeg = SM.getMacroArgExpandedLocation(
       getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
   DeclRelationSet Relations =
       DeclRelation::TemplatePattern | DeclRelation::Underlying;
   auto Decls = getDeclAtPosition(AST, SourceLocationBeg, Relations);
   if (Decls.empty())
     return nullptr;
 
-  const Decl *D = Decls[0];
+  const NamedDecl *D = Decls[0];
 
   if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
     // If this is a variable, use the type of the variable.
     return VD->getType().getTypePtr()->getAsCXXRecordDecl();
   }
 
   if (const CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(D)) {
     // If this is a method, use the type of the class.

clang-tools-extra/clangd/refactor/Rename.cpp

unsigned Offset =
       AST.getSourceManager().getDecomposedSpellingLoc(TokenStartLoc).second;
 
   SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
   const SelectionTree::Node *SelectedNode = Selection.commonAncestor();
   if (!SelectedNode)
     return {};
 
   llvm::DenseSet<const Decl *> Result;
-  for (const auto *D :
+  for (const NamedDecl *D :
        targetDecl(SelectedNode->ASTNode,
                   DeclRelation::Alias | DeclRelation::TemplatePattern)) {
-    const auto *ND = llvm::dyn_cast<NamedDecl>(D);
-    if (!ND)
-      continue;
     // Get to CXXRecordDecl from constructor or destructor.
-    ND = tooling::getCanonicalSymbolDeclaration(ND);
-    Result.insert(ND);
+    D = tooling::getCanonicalSymbolDeclaration(D);
+    Result.insert(D);
   }
   return Result;
 }
 
 enum ReasonToReject {
   NoSymbolFound,
   NoIndexProvided,
   NonIndexable,

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

 // There's no great way to assert on the "content" of a Decl in the general case
 // that's both expressive and unambiguous (e.g. clearly distinguishes between
 // templated decls and their specializations).
 //
 // We use the result of pretty-printing the decl, with the {body} truncated.
 struct PrintedDecl {
   PrintedDecl(const char *Name, DeclRelationSet Relations = {})
       : Name(Name), Relations(Relations) {}
-  PrintedDecl(const Decl *D, DeclRelationSet Relations = {})
+  PrintedDecl(const NamedDecl *D, DeclRelationSet Relations = {})
       : Relations(Relations) {
     std::string S;
     llvm::raw_string_ostream OS(S);
     D->print(OS);
     llvm::StringRef FirstLine =
         llvm::StringRef(OS.str()).take_until([](char C) { return C == '\n'; });
     FirstLine = FirstLine.rtrim(" {");
     Name = FirstLine.rtrim(" {");

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

llvm::StringRef Tests[] = {
       R"cpp(// Template auto parameter. Nothing (Not useful).
             template<^auto T>
             void func() {
             }
             void foo() {
                func<1>();
             }
           )cpp",
+      R"cpp(// non-named decls don't get hover. Don't crash!
+            ^static_assert(1, "");
+          )cpp",
   };
 
   for (const auto &Test : Tests) {
     SCOPED_TRACE(Test);
 
     Annotations T(Test);
     TestTU TU = TestTU::withCode(T.code());
     TU.ExtraArgs.push_back("-std=c++17");