sammccall updated this revision to Diff 300182.
sammccall added a comment.
Add comment+assert to clarify
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89785/new/
https://reviews.llvm.org/D89785
Files:
clang-tools-extra/clangd/AST.cpp
clang-tools-extra/clangd/AST.h
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/ASTTests.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
clang-tools-extra/clangd/unittests/SelectionTests.cpp
Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -415,7 +415,19 @@
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;
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ 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.
Index: clang-tools-extra/clangd/unittests/ASTTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ASTTests.cpp
+++ 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<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
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ 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<Attr>() && N->Parent)
+ N = N->Parent;
llvm::copy(targetDecl(N->ASTNode, Relations),
std::back_inserter(Result));
}
@@ -287,7 +293,7 @@
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;
@@ -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<CXXMethodDecl>(D)) {
- const InheritableAttr *Attr = D->getAttr<OverrideAttr>();
- if (!Attr)
- Attr = D->getAttr<FinalAttr>();
- if (Attr && TouchedIdentifier &&
- SM.getSpellingLoc(Attr->getLocation()) ==
- TouchedIdentifier->location()) {
+ if (NodeKind.isSame(ASTNodeKind::getFromNodeKind<OverrideAttr>()) ||
+ NodeKind.isSame(ASTNodeKind::getFromNodeKind<FinalAttr>())) {
+ if (const auto *CMD = llvm::dyn_cast<CXXMethodDecl>(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));
Index: clang-tools-extra/clangd/Selection.cpp
===================================================================
--- clang-tools-extra/clangd/Selection.cpp
+++ 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<TranslationUnitDecl>(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<DecltypeTypeLoc>())
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));
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ 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<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");
}
@@ -862,6 +877,8 @@
maybeAddCalleeArgInfo(N, *HI, AST.getASTContext().getPrintingPolicy());
} else if (const Expr *E = N->ASTNode.get<Expr>()) {
HI = getHoverContents(E, AST);
+ } else if (const Attr *A = N->ASTNode.get<Attr>()) {
+ HI = getHoverContents(A, AST);
}
// FIXME: support hovers for other nodes?
// - built-in types
Index: clang-tools-extra/clangd/AST.h
===================================================================
--- clang-tools-extra/clangd/AST.h
+++ 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<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.
Index: clang-tools-extra/clangd/AST.cpp
===================================================================
--- clang-tools-extra/clangd/AST.cpp
+++ 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<const Attr *> getAttributes(const DynTypedNode &N) {
+ std::vector<const Attr *> Result;
+ if (const auto *ATLPtr = N.get<AttributedTypeLoc>())
+ for (AttributedTypeLoc ATL = *ATLPtr; !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,
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
