[clangd] Add basic support for attributes (selection, hover)

These aren't terribly common, but we currently mishandle them badly.
Not only do we not recogize the attributes themselves, but we often end up
selecting some node other than the parent (because source ranges aren't accurate
in the presence of attributes).

Differential Revision: https://reviews.llvm.org/D89785
This commit is contained in:
Sam McCall 2020-10-20 12:01:48 +02:00
parent c8f148274f
commit bb81e7083d
8 changed files with 124 additions and 16 deletions

View File

@ -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 @@ llvm::Optional<QualType> getDeducedType(ASTContext &ASTCtx,
return V.DeducedType;
}
std::vector<const Attr *> getAttributes(const DynTypedNode &N) {
std::vector<const Attr *> Result;
if (const auto *TL = N.get<TypeLoc>()) {
for (AttributedTypeLoc ATL = TL->getAs<AttributedTypeLoc>(); !ATL.isNull();
ATL = ATL.getModifiedLoc().getAs<AttributedTypeLoc>()) {
Result.push_back(ATL.getAttr());
assert(!ATL.getModifiedLoc().isNull());
}
}
if (const auto *S = N.get<AttributedStmt>())
for (; S != nullptr; S = dyn_cast<AttributedStmt>(S->getSubStmt()))
llvm::copy(S->getAttrs(), std::back_inserter(Result));
if (const auto *D = N.get<Decl>())
llvm::copy(D->attrs(), std::back_inserter(Result));
return Result;
}
std::string getQualification(ASTContext &Context,
const DeclContext *DestContext,
SourceLocation InsertionPoint,

View File

@ -27,6 +27,7 @@
namespace clang {
class SourceManager;
class Decl;
class DynTypedNode;
namespace clangd {
@ -121,6 +122,9 @@ QualType declaredType(const TypeDecl *D);
/// If the type is an undeduced auto, returns the type itself.
llvm::Optional<QualType> getDeducedType(ASTContext &, SourceLocation Loc);
/// Return attributes attached directly to a node.
std::vector<const Attr *> 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.

View File

@ -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 @@ llvm::Optional<HoverInfo> getHoverContents(const Expr *E, ParsedAST &AST,
return llvm::None;
}
// Generates hover info for attributes.
llvm::Optional<HoverInfo> 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 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
maybeAddCalleeArgInfo(N, *HI, PP);
} else if (const Expr *E = N->ASTNode.get<Expr>()) {
HI = getHoverContents(E, AST, PP, Index);
} else if (const Attr *A = N->ASTNode.get<Attr>()) {
HI = getHoverContents(A, AST);
}
// FIXME: support hovers for other nodes?
// - built-in types

View File

@ -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 @@ public:
// 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<TranslationUnitDecl>(X))
return Base::TraverseDecl(X); // Already pushed by constructor.
@ -517,6 +517,9 @@ public:
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 @@ private:
if (auto AT = TL->getAs<AttributedTypeLoc>())
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));

View File

@ -183,6 +183,12 @@ getDeclAtPositionWithRelations(ParsedAST &AST, SourceLocation Pos,
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<Attr>() && 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<LocatedSymbol> findImplementors(llvm::DenseSet<SymbolID> IDs,
std::vector<LocatedSymbol>
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<LocatedSymbol> Result;
@ -376,7 +382,7 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier,
DeclRelationSet Relations =
DeclRelation::TemplatePattern | DeclRelation::Alias;
auto Candidates =
getDeclAtPositionWithRelations(AST, CurLoc, Relations, NodeKind);
getDeclAtPositionWithRelations(AST, CurLoc, Relations, &NodeKind);
llvm::DenseSet<SymbolID> VirtualMethods;
for (const auto &E : Candidates) {
const NamedDecl *D = E.first;
@ -392,13 +398,8 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier,
}
}
// Special case: void foo() ^override: jump to the overridden method.
const InheritableAttr *Attr = D->getAttr<OverrideAttr>();
if (!Attr)
Attr = D->getAttr<FinalAttr>();
if (Attr && TouchedIdentifier &&
SM.getSpellingLoc(Attr->getLocation()) ==
TouchedIdentifier->location()) {
LocateASTReferentMetric.record(1, "method-to-base");
if (NodeKind.isSame(ASTNodeKind::getFromNodeKind<OverrideAttr>()) ||
NodeKind.isSame(ASTNodeKind::getFromNodeKind<FinalAttr>())) {
// We may be overridding multiple methods - offer them all.
for (const NamedDecl *ND : CMD->overridden_methods())
AddResultDecl(ND);
@ -800,7 +801,7 @@ std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
ASTNodeKind NodeKind;
auto ASTResults = locateASTReferent(*CurLoc, TouchedIdentifier, AST,
*MainFilePath, Index, &NodeKind);
*MainFilePath, Index, NodeKind);
if (!ASTResults.empty())
return ASTResults;
@ -816,9 +817,8 @@ std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
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 @@ llvm::DenseSet<const Decl *> getNonLocalDeclRefs(ParsedAST &AST,
}
} // namespace clangd
} // namespace clang
} // namespace clang

View File

@ -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 @@ TEST(ClangdAST, IsDeeplyNested) {
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<FunctionDecl>(findDecl(AST, "foo")).getBody();
IfStmt *FooIf = cast<IfStmt>(cast<CompoundStmt>(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

View File

@ -2377,6 +2377,15 @@ TEST(Hover, All) {
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.

View File

@ -453,7 +453,19 @@ TEST(SelectionTest, CommonAncestor) {
template <template <typename> class Container> class A {};
A<[[V^ector]]> a;
)cpp",
"TemplateArgumentLoc"}};
"TemplateArgumentLoc"},
// Attributes
{R"cpp(
void f(int * __attribute__(([[no^nnull]])) );
)cpp",
"NonNullAttr"},
{R"cpp(
// Digraph syntax for attributes to avoid accidental annotations.
class <:[gsl::Owner([[in^t]])]:> X{};
)cpp",
"BuiltinTypeLoc"}};
for (const Case &C : Cases) {
trace::TestTracer Tracer;