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

[sammccall]

Summary

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).

[sammccall]

Only deoptimize selection for *explicit* attributes

[hokein]

clang-tools-extra/clangd/AST.cpp

490

this looks like not safe, getNextTypeLoc() may return a null TypeLoc.

clang-tools-extra/clangd/unittests/ASTTests.cpp

390

sorry, I'm not familiar with attributes, what is an implicit attr? It is unclear to me why there is an attr for class X, the source code doesn't have any attribute label for X (the same question for f and a)

[sammccall]

Add comment+assert to clarify

[sammccall]

clang-tools-extra/clangd/AST.cpp

490

I don't think it can - an AttributedType always modifies a real type.
I've added an assert.

clang-tools-extra/clangd/unittests/ASTTests.cpp

390

Right, implicit attributes are when there's nothing written in the source ,but something else modifies the semantics in a way that clang authors decided to model as an attribute (e.g. because semantics match that of an explicit attribute).

I'm not familiar with many examples either, but a couple:

• when targeting windows, top-level classes appear to have an implicit "type visibility" attribute that I guess models the difference between default unix/windows symbol visibility.
• Aaron Ballman gave an example of [[interrupt(...)]] which also adds an implicit [[used]] attribute.

The windows example was why I assert there are no explicit attributes, instead of that there are none at all. I've added a comment.

[hokein]

clang-tools-extra/clangd/unittests/ASTTests.cpp

390

ah, thanks for the explanation.

clang-tools-extra/clangd/AST.h

 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/StringRef.h"
 #include <string>
 #include <vector>
 
 namespace clang {
 class SourceManager;
 class Decl;
+class DynTypedNode;
 
 namespace clangd {
 
 /// Returns true if the declaration is considered implementation detail based on
 /// heuristics. For example, a declaration whose name is not explicitly spelled
 /// in code is considered implementation detail.
 bool isImplementationDetail(const Decl *D);
 

 // (i.e. vector<T*> rather than vector<type-parameter-0-0 *>.
 QualType declaredType(const TypeDecl *D);
 
 /// Retrieves the deduced type at a given location (auto, decltype).
 /// It will return the underlying type.
 /// 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.
 /// Returns an empty string if no qualification is necessary. For example, if
 /// you want to qualify clang::clangd::bar::foo in clang::clangd::x, this
 /// function will return bar. Note that the result might be sub-optimal for
 /// classes, e.g. when the \p ND is a member of the base class.
 ///

clang-tools-extra/clangd/AST.cpp

 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DeclarationName.h"
 #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"
 #include "clang/Index/USRGeneration.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
if (!Loc.isValid())
     return {};
   DeducedTypeVisitor V(Loc);
   V.TraverseAST(ASTCtx);
   if (V.DeducedType.isNull())
     return llvm::None;
   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,
                              const NamedDecl *ND) {
   auto VisibleNamespaceDecls =
       getUsingNamespaceDirectives(DestContext, InsertionPoint);
   return getQualification(
       Context, DestContext, ND->getDeclContext(),

clang-tools-extra/clangd/Hover.cpp

 #include "ParsedAST.h"
 #include "Selection.h"
 #include "SourceCode.h"
 #include "index/SymbolCollector.h"
 #include "support/Logger.h"
 #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"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/OperationKinds.h"
if (auto Val = printExprValue(E, AST.getASTContext())) {
     HI.Type = printType(E->getType(), PP);
     HI.Value = *Val;
     HI.Name = std::string(getNameForExpr(E));
     return HI;
   }
   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");
 }
 
 bool punctuationIndicatesLineBreak(llvm::StringRef Line) {
   constexpr llvm::StringLiteral Punctuation = R"txt(.:,;!?)txt";
 
   Line = Line.rtrim();
if (const SelectionTree::Node *N = ST.commonAncestor()) {
         if (Decls.front() == N->ASTNode.get<Decl>())
           addLayoutInfo(*Decls.front(), *HI);
         // Look for a close enclosing expression to show the value of.
         if (!HI->Value)
           HI->Value = printExprValue(N, AST.getASTContext());
         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
     }
   }
 
   if (!HI)
     return llvm::None;

clang-tools-extra/clangd/Selection.cpp

 //===--- Selection.cpp ----------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Selection.h"
+#include "AST.h"
 #include "SourceCode.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
public:
   //  - traverse its children recursively
   //  - pop it from the stack
   //  - hit testing: is intersection(node, selection) - union(children) empty?
   //  - attach it to the tree if it or any children hit the selection
   //
   // 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.
     // Base::TraverseDecl will suppress children, but not this node itself.
     if (X && X->isImplicit())
       return true;
     return traverseNode(X, [&] { return Base::TraverseDecl(X); });
   }
public:
   }
   bool TraverseConstructorInitializer(CXXCtorInitializer *X) {
     return traverseNode(
         X, [&] { return Base::TraverseConstructorInitializer(X); });
   }
   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))
       return false;
     auto N = DynTypedNode::create(*X);
     if (canSafelySkipNode(N))
       return false;
     push(std::move(N));
if (auto *TL = N.get<TypeLoc>()) {
       // rid of this patch.
       if (auto DT = TL->getAs<DecltypeTypeLoc>())
         S.setEnd(DT.getUnderlyingExpr()->getEndLoc());
       // AttributedTypeLoc may point to the attribute's range, NOT the modified
       // type's range.
       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));
       return true;
     }
     return false;
   }
 

clang-tools-extra/clangd/XRefs.cpp

getDeclAtPositionWithRelations(ParsedAST &AST, SourceLocation Pos,
                                DeclRelationSet Relations,
                                ASTNodeKind *NodeKind = nullptr) {
   unsigned Offset = AST.getSourceManager().getDecomposedSpellingLoc(Pos).second;
   std::vector<std::pair<const NamedDecl *, DeclRelationSet>> Result;
   auto ResultFromTree = [&](SelectionTree ST) {
     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); });
     }
     return !Result.empty();
   };
   SelectionTree::createEach(AST.getASTContext(), AST.getTokens(), Offset,
                             Offset, ResultFromTree);

 
 // Decls are more complicated.
 // The AST contains at least a declaration, maybe a definition.
 // These are up-to-date, and so generally preferred over index results.
 // We perform a single batch index lookup to find additional definitions.
 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;
   // Keep track of SymbolID -> index mapping, to fill in index data later.
   llvm::DenseMap<SymbolID, size_t> ResultIndex;
 
   static constexpr trace::Metric LocateASTReferentMetric(
       "locate_ast_referent", trace::Metric::Counter, "case");
auto AddResultDecl = [&](const NamedDecl *D) {
     if (auto ID = getSymbolID(D))
       ResultIndex[ID] = Result.size() - 1;
   };
 
   // Emit all symbol locations (declaration or definition) from AST.
   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;
     if (const auto *CMD = llvm::dyn_cast<CXXMethodDecl>(D)) {
       // Special case: virtual void ^method() = 0: jump to all overrides.
       // FIXME: extend it to ^virtual, unfortunately, virtual location is not
       // saved in the AST.
       if (CMD->isPure()) {
         if (TouchedIdentifier && SM.getSpellingLoc(CMD->getLocation()) ==
                                      TouchedIdentifier->location()) {
           VirtualMethods.insert(getSymbolID(CMD));
           LocateASTReferentMetric.record(1, "method-to-override");
         }
       }
       // 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);
         continue;
       }
     }
 
     // Special case: the cursor is on an alias, prefer other results.
if (Tok.kind() == tok::kw_auto || Tok.kind() == tok::kw_decltype) {
         if (!LocSym.empty())
           return LocSym;
       }
     }
   }
 
   ASTNodeKind NodeKind;
   auto ASTResults = locateASTReferent(*CurLoc, TouchedIdentifier, AST,
-                                      *MainFilePath, Index, &NodeKind);
+                                      *MainFilePath, Index, NodeKind);
   if (!ASTResults.empty())
     return ASTResults;
 
   // If the cursor can't be resolved directly, try fallback strategies.
   auto Word =
       SpelledWord::touching(*CurLoc, AST.getTokens(), AST.getLangOpts());
   if (Word) {
     // Is the same word nearby a real identifier that might refer to something?
     if (const syntax::Token *NearbyIdent =
             findNearbyIdentifier(*Word, AST.getTokens())) {
       if (auto Macro = locateMacroReferent(*NearbyIdent, AST, *MainFilePath)) {
         log("Found macro definition heuristically using nearby identifier {0}",
             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));
         return ASTResults;
       } else {
         vlog("No definition found using nearby identifier {0} at {1}",
              Word->Text, Word->Location.printToString(SM));
       }
findExplicitReferences(
             DeclRefs.insert(D);
         }
       },
       AST.getHeuristicResolver());
   return DeclRefs;
 }
 
 } // namespace clangd
-} // namespace clang
+} // namespace clang
 

clang-tools-extra/clangd/unittests/ASTTests.cpp

 //===-- ASTTests.cpp --------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "AST.h"
 
 #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"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <cstddef>
 #include <string>
 #include <vector>
 
 namespace clang {
 namespace clangd {
 namespace {
+using testing::Contains;
+using testing::Each;
 
 TEST(GetDeducedType, KwAutoKwDecltypeExpansion) {
   struct Test {
     StringRef AnnotatedCode;
     const char *DeducedType;
   } Tests[] = {
       {"^auto i = 0;", "int"},
       {"^auto f(){ return 1;};", "int"},
TEST(ClangdAST, IsDeeplyNested) {
   TestTU TU = TestTU::withCode(Test.code());
   ParsedAST AST = TU.build();
 
   EXPECT_TRUE(isDeeplyNested(&findUnqualifiedDecl(AST, "Foo"), /*MaxDepth=*/1));
   EXPECT_FALSE(
       isDeeplyNested(&findUnqualifiedDecl(AST, "Foo"), /*MaxDepth=*/2));
 
   EXPECT_TRUE(isDeeplyNested(&findUnqualifiedDecl(AST, "bar"), /*MaxDepth=*/2));
   EXPECT_FALSE(
       isDeeplyNested(&findUnqualifiedDecl(AST, "bar"), /*MaxDepth=*/3));
 
   EXPECT_TRUE(isDeeplyNested(&findUnqualifiedDecl(AST, "Bar"), /*MaxDepth=*/3));
   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

clang-tools-extra/clangd/unittests/HoverTests.cpp

          HI.Name = "result";
          HI.Definition = "int result = 0";
          HI.Kind = index::SymbolKind::Variable;
          HI.Type = "int";
          HI.LocalScope = "-[Foo variadicArgMethod:, ...]::";
          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.
   Symbol IndexSym = func("indexSymbol");
   IndexSym.Documentation = "comment from index";
   SymbolSlab::Builder Symbols;
   Symbols.insert(IndexSym);
   auto Index =

clang-tools-extra/clangd/unittests/SelectionTests.cpp

Case Cases[] = {
        "DeclRefExpr"}, // DeclRefExpr to the "operator->" method.
 
       // Template template argument.
       {R"cpp(
         template <typename> class Vector {};
         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;
     Annotations Test(C.Code);
 
     TestTU TU;
     TU.Code = std::string(Test.code());