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 @@ -26,6 +26,7 @@ namespace clang { class SourceManager; class Decl; +class DynTypedNode; namespace clangd { @@ -112,6 +113,9 @@ /// It will return the underlying type. 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" @@ -425,6 +427,22 @@ return V.DeducedType; } +std::vector getAttributes(const DynTypedNode &N) { + std::vector Result; + if (const auto *ATLPtr = N.get()) + for (AttributedTypeLoc ATL = *ATLPtr; !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" @@ -651,6 +652,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"); } @@ -862,6 +877,8 @@ maybeAddCalleeArgInfo(N, *HI, AST.getASTContext().getPrintingPolicy()); } else if (const Expr *E = N->ASTNode.get()) { HI = getHoverContents(E, AST); + } 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" @@ -467,7 +468,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. @@ -491,6 +491,9 @@ return traverseNode( X, [&] { return Base::TraverseConstructorInitializer(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)) @@ -617,6 +620,11 @@ if (auto DT = TL->getAs()) S.setEnd(DT.getUnderlyingExpr()->getEndLoc()); } + // 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 @@ -182,6 +182,12 @@ 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(targetDecl(N->ASTNode, Relations), std::back_inserter(Result)); } @@ -287,7 +293,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; @@ -317,15 +323,11 @@ DeclRelationSet Relations = DeclRelation::TemplatePattern | DeclRelation::Alias; for (const NamedDecl *D : - getDeclAtPosition(AST, CurLoc, Relations, NodeKind)) { + getDeclAtPosition(AST, CurLoc, Relations, &NodeKind)) { // Special case: void foo() ^override: jump to the overridden method. - if (const auto *CMD = llvm::dyn_cast(D)) { - const InheritableAttr *Attr = D->getAttr(); - if (!Attr) - Attr = D->getAttr(); - if (Attr && TouchedIdentifier && - SM.getSpellingLoc(Attr->getLocation()) == - TouchedIdentifier->location()) { + if (NodeKind.isSame(ASTNodeKind::getFromNodeKind()) || + NodeKind.isSame(ASTNodeKind::getFromNodeKind())) { + if (const auto *CMD = llvm::dyn_cast(D)) { // We may be overridding multiple methods - offer them all. for (const NamedDecl *ND : CMD->overridden_methods()) AddResultDecl(ND); @@ -653,7 +655,7 @@ ASTNodeKind NodeKind; auto ASTResults = locateASTReferent(*CurLoc, TouchedIdentifier, AST, - *MainFilePath, Index, &NodeKind); + *MainFilePath, Index, NodeKind); if (!ASTResults.empty()) return ASTResults; @@ -669,9 +671,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)); 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, KwAutoExpansion) { struct Test { @@ -206,6 +211,39 @@ } } } + +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 @@ -1991,6 +1991,16 @@ HI.NamespaceScope = "ObjC::"; // FIXME: fix it HI.Definition = "char data"; }}, + { + 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 @@ -415,7 +415,19 @@ template