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,10 @@ /// It will return the underlying type. llvm::Optional getDeducedType(ASTContext &, SourceLocation Loc); +/// Does this node have any attributes (Attr) attached directly to it? +/// (Such nodes often have incorrect SourceRanges). +bool hasAttributes(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,16 @@ return V.DeducedType; } +bool hasAttributes(const DynTypedNode &N) { + if (N.get() || N.get()) + return true; + if (const auto *S = N.get()) + return true; + if (const auto *D = N.get()) + return D->hasAttrs(); + return false; +} + 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" @@ -491,6 +492,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 +621,10 @@ if (auto DT = TL->getAs()) S.setEnd(DT.getUnderlyingExpr()->getEndLoc()); } + // SourceRange often doesn't manage to accurately cover attributes. + // Fortunately, attributes are rare. + if (hasAttributes(N)) + 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,6 +11,7 @@ #include "Annotations.h" #include "ParsedAST.h" #include "TestTU.h" +#include "clang/AST/ASTTypeTraits.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclBase.h" #include "clang/Basic/SourceManager.h" @@ -206,6 +207,31 @@ } } } + +TEST(ClangdAST, HasAttributes) { + 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(); + ASSERT_FALSE(hasAttributes(DynTypedNode::create(findDecl(AST, "X")))); + ASSERT_TRUE(hasAttributes(DynTypedNode::create(findDecl(AST, "Y")))); + ASSERT_FALSE(hasAttributes(DynTypedNode::create(findDecl(AST, "f")))); + ASSERT_FALSE( + hasAttributes(DynTypedNode::create(findUnqualifiedDecl(AST, "a")))); + ASSERT_TRUE( + hasAttributes(DynTypedNode::create(findUnqualifiedDecl(AST, "b")))); + + Stmt *FooBody = cast(findDecl(AST, "foo")).getBody(); + IfStmt *FooIf = cast(cast(FooBody)->body_front()); + ASSERT_FALSE(hasAttributes(DynTypedNode::create(*FooIf))); + ASSERT_TRUE(hasAttributes(DynTypedNode::create(*FooIf->getThen()))); +} } // 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