This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbb81e7083d25: [clangd] Add basic support for attributes 
(selection, hover) (authored by sammccall).
Herald added a project: clang-tools-extra.

Changed prior to commit:
  https://reviews.llvm.org/D89785?vs=300182&id=364877#toc

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
@@ -453,7 +453,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
@@ -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.
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, 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<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
@@ -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<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>
 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 @@
   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 @@
         }
       }
       // 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 @@
 
   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
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"
@@ -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<TranslationUnitDecl>(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<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));
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"
@@ -720,6 +721,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");
 }
@@ -960,6 +975,8 @@
         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
Index: clang-tools-extra/clangd/AST.h
===================================================================
--- clang-tools-extra/clangd/AST.h
+++ 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<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"
@@ -481,6 +483,23 @@
   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,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to