diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -27,6 +27,7 @@ namespace clang { class SourceManager; class Decl; +class DynTypedNode; namespace clangd { @@ -121,6 +122,9 @@ /// If the type is an undeduced auto, returns the type itself. llvm::Optional getDeducedType(ASTContext &, SourceLocation Loc); +/// Return attributes attached directly to a node. +std::vector getAttributes(const DynTypedNode &); + /// Gets the nested name specifier necessary for spelling \p ND in \p /// DestContext, at \p InsertionPoint. It selects the shortest suffix of \p ND /// such that it is visible in \p DestContext. diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -20,7 +20,9 @@ #include "clang/AST/NestedNameSpecifier.h" #include "clang/AST/PrettyPrinter.h" #include "clang/AST/RecursiveASTVisitor.h" +#include "clang/AST/Stmt.h" #include "clang/AST/TemplateBase.h" +#include "clang/AST/TypeLoc.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/Specifiers.h" @@ -481,6 +483,23 @@ return V.DeducedType; } +std::vector getAttributes(const DynTypedNode &N) { + std::vector Result; + if (const auto *TL = N.get()) { + for (AttributedTypeLoc ATL = TL->getAs(); !ATL.isNull(); + ATL = ATL.getModifiedLoc().getAs()) { + Result.push_back(ATL.getAttr()); + assert(!ATL.getModifiedLoc().isNull()); + } + } + if (const auto *S = N.get()) + for (; S != nullptr; S = dyn_cast(S->getSubStmt())) + llvm::copy(S->getAttrs(), std::back_inserter(Result)); + if (const auto *D = N.get()) + llvm::copy(D->attrs(), std::back_inserter(Result)); + return Result; +} + std::string getQualification(ASTContext &Context, const DeclContext *DestContext, SourceLocation InsertionPoint, diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp --- a/clang-tools-extra/clangd/Hover.cpp +++ b/clang-tools-extra/clangd/Hover.cpp @@ -19,6 +19,7 @@ #include "support/Markup.h" #include "clang/AST/ASTContext.h" #include "clang/AST/ASTTypeTraits.h" +#include "clang/AST/Attr.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclBase.h" #include "clang/AST/DeclCXX.h" @@ -720,6 +721,20 @@ return llvm::None; } +// Generates hover info for attributes. +llvm::Optional getHoverContents(const Attr *A, ParsedAST &AST) { + HoverInfo HI; + HI.Name = A->getSpelling(); + if (A->hasScope()) + HI.LocalScope = A->getScopeName()->getName().str(); + { + llvm::raw_string_ostream OS(HI.Definition); + A->printPretty(OS, AST.getASTContext().getPrintingPolicy()); + } + // FIXME: attributes have documentation, can we get at that? + return HI; +} + bool isParagraphBreak(llvm::StringRef Rest) { return Rest.ltrim(" \t").startswith("\n"); } @@ -960,6 +975,8 @@ maybeAddCalleeArgInfo(N, *HI, PP); } else if (const Expr *E = N->ASTNode.get()) { HI = getHoverContents(E, AST, PP, Index); + } else if (const Attr *A = N->ASTNode.get()) { + HI = getHoverContents(A, AST); } // FIXME: support hovers for other nodes? // - built-in types diff --git a/clang-tools-extra/clangd/Selection.cpp b/clang-tools-extra/clangd/Selection.cpp --- a/clang-tools-extra/clangd/Selection.cpp +++ b/clang-tools-extra/clangd/Selection.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "Selection.h" +#include "AST.h" #include "SourceCode.h" #include "support/Logger.h" #include "support/Trace.h" @@ -490,7 +491,6 @@ // Two categories of nodes are not "well-behaved": // - those without source range information, we don't record those // - those that can't be stored in DynTypedNode. - // We're missing some interesting things like Attr due to the latter. bool TraverseDecl(Decl *X) { if (X && isa(X)) return Base::TraverseDecl(X); // Already pushed by constructor. @@ -517,6 +517,9 @@ bool TraverseCXXBaseSpecifier(const CXXBaseSpecifier &X) { return traverseNode(&X, [&] { return Base::TraverseCXXBaseSpecifier(X); }); } + bool TraverseAttr(Attr *X) { + return traverseNode(X, [&] { return Base::TraverseAttr(X); }); + } // Stmt is the same, but this form allows the data recursion optimization. bool dataTraverseStmtPre(Stmt *X) { if (!X || isImplicit(X)) @@ -651,6 +654,11 @@ if (auto AT = TL->getAs()) S = AT.getModifiedLoc().getSourceRange(); } + // SourceRange often doesn't manage to accurately cover attributes. + // Fortunately, attributes are rare. + if (llvm::any_of(getAttributes(N), + [](const Attr *A) { return !A->isImplicit(); })) + return false; if (!SelChecker.mayHit(S)) { dlog("{1}skip: {0}", printNodeToString(N, PrintPolicy), indent()); dlog("{1}skipped range = {0}", S.printToString(SM), indent(1)); diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -183,6 +183,12 @@ if (const SelectionTree::Node *N = ST.commonAncestor()) { if (NodeKind) *NodeKind = N->ASTNode.getNodeKind(); + // Attributes don't target decls, look at the + // thing it's attached to. + // We still report the original NodeKind! + // This makes the `override` hack work. + if (N->ASTNode.get() && N->Parent) + N = N->Parent; llvm::copy_if(allTargetDecls(N->ASTNode, AST.getHeuristicResolver()), std::back_inserter(Result), [&](auto &Entry) { return !(Entry.second & ~Relations); }); @@ -343,7 +349,7 @@ std::vector locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier, ParsedAST &AST, llvm::StringRef MainFilePath, - const SymbolIndex *Index, ASTNodeKind *NodeKind) { + const SymbolIndex *Index, ASTNodeKind &NodeKind) { const SourceManager &SM = AST.getSourceManager(); // Results follow the order of Symbols.Decls. std::vector Result; @@ -376,7 +382,7 @@ DeclRelationSet Relations = DeclRelation::TemplatePattern | DeclRelation::Alias; auto Candidates = - getDeclAtPositionWithRelations(AST, CurLoc, Relations, NodeKind); + getDeclAtPositionWithRelations(AST, CurLoc, Relations, &NodeKind); llvm::DenseSet VirtualMethods; for (const auto &E : Candidates) { const NamedDecl *D = E.first; @@ -392,13 +398,8 @@ } } // Special case: void foo() ^override: jump to the overridden method. - const InheritableAttr *Attr = D->getAttr(); - if (!Attr) - Attr = D->getAttr(); - if (Attr && TouchedIdentifier && - SM.getSpellingLoc(Attr->getLocation()) == - TouchedIdentifier->location()) { - LocateASTReferentMetric.record(1, "method-to-base"); + if (NodeKind.isSame(ASTNodeKind::getFromNodeKind()) || + NodeKind.isSame(ASTNodeKind::getFromNodeKind())) { // We may be overridding multiple methods - offer them all. for (const NamedDecl *ND : CMD->overridden_methods()) AddResultDecl(ND); @@ -800,7 +801,7 @@ ASTNodeKind NodeKind; auto ASTResults = locateASTReferent(*CurLoc, TouchedIdentifier, AST, - *MainFilePath, Index, &NodeKind); + *MainFilePath, Index, NodeKind); if (!ASTResults.empty()) return ASTResults; @@ -816,9 +817,8 @@ Word->Text); return {*std::move(Macro)}; } - ASTResults = - locateASTReferent(NearbyIdent->location(), NearbyIdent, AST, - *MainFilePath, Index, /*NodeKind=*/nullptr); + ASTResults = locateASTReferent(NearbyIdent->location(), NearbyIdent, AST, + *MainFilePath, Index, NodeKind); if (!ASTResults.empty()) { log("Found definition heuristically using nearby identifier {0}", NearbyIdent->text(SM)); @@ -1987,4 +1987,4 @@ } } // namespace clangd -} // namespace clang \ No newline at end of file +} // namespace clang diff --git a/clang-tools-extra/clangd/unittests/ASTTests.cpp b/clang-tools-extra/clangd/unittests/ASTTests.cpp --- a/clang-tools-extra/clangd/unittests/ASTTests.cpp +++ b/clang-tools-extra/clangd/unittests/ASTTests.cpp @@ -11,8 +11,11 @@ #include "Annotations.h" #include "ParsedAST.h" #include "TestTU.h" +#include "clang/AST/ASTTypeTraits.h" +#include "clang/AST/Attr.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclBase.h" +#include "clang/Basic/AttrKinds.h" #include "clang/Basic/SourceManager.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Casting.h" @@ -25,6 +28,8 @@ namespace clang { namespace clangd { namespace { +using testing::Contains; +using testing::Each; TEST(GetDeducedType, KwAutoKwDecltypeExpansion) { struct Test { @@ -389,6 +394,40 @@ EXPECT_FALSE( isDeeplyNested(&findUnqualifiedDecl(AST, "Bar"), /*MaxDepth=*/4)); } + +MATCHER_P(attrKind, K, "") { return arg->getKind() == K; } + +MATCHER(implicitAttr, "") { return arg->isImplicit(); } + +TEST(ClangdAST, GetAttributes) { + const char *Code = R"cpp( + class X{}; + class [[nodiscard]] Y{}; + void f(int * a, int * __attribute__((nonnull)) b); + void foo(bool c) { + if (c) + [[unlikely]] return; + } + )cpp"; + ParsedAST AST = TestTU::withCode(Code).build(); + auto DeclAttrs = [&](llvm::StringRef Name) { + return getAttributes(DynTypedNode::create(findUnqualifiedDecl(AST, Name))); + }; + // Implicit attributes may be present (e.g. visibility on windows). + ASSERT_THAT(DeclAttrs("X"), Each(implicitAttr())); + ASSERT_THAT(DeclAttrs("Y"), Contains(attrKind(attr::WarnUnusedResult))); + ASSERT_THAT(DeclAttrs("f"), Each(implicitAttr())); + ASSERT_THAT(DeclAttrs("a"), Each(implicitAttr())); + ASSERT_THAT(DeclAttrs("b"), Contains(attrKind(attr::NonNull))); + + Stmt *FooBody = cast(findDecl(AST, "foo")).getBody(); + IfStmt *FooIf = cast(cast(FooBody)->body_front()); + ASSERT_THAT(getAttributes(DynTypedNode::create(*FooIf)), + Each(implicitAttr())); + ASSERT_THAT(getAttributes(DynTypedNode::create(*FooIf->getThen())), + Contains(attrKind(attr::Unlikely))); +} + } // namespace } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp --- a/clang-tools-extra/clangd/unittests/HoverTests.cpp +++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp @@ -2377,6 +2377,15 @@ HI.NamespaceScope = ""; HI.Value = "0"; }}, + {R"cpp( + void foo(int * __attribute__(([[non^null]], noescape)) ); + )cpp", + [](HoverInfo &HI) { + HI.Name = "nonnull"; + HI.Kind = index::SymbolKind::Unknown; // FIXME: no suitable value + HI.Definition = "__attribute__((nonnull))"; + HI.Documentation = ""; // FIXME + }}, }; // Create a tiny index, so tests above can verify documentation is fetched. diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp b/clang-tools-extra/clangd/unittests/SelectionTests.cpp --- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp +++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp @@ -453,7 +453,19 @@ template