[clangd] Support indexing MACROs.

[ioeric]

Summary

This is not enabled in the global-symbol-builder or dynamic index yet.

[ioeric]

• Some cleanup.

[ioeric]

• Another minor cleanup.

[sammccall]

clangd/index/SymbolCollector.cpp

309

why this change? I think this makes us run generateUSR much more often (once per non-unique reference in *any* file, vs unique refeneces in main file), keeping track of a few extra referenced symbols by pointer should be much cheaper

349

drop the message unless it has something new to say

349

(why) do we require PP to be set if CollectMacro is false?

356

not sure what this means. Are you talking about code completion? This isn't CodeComplete :-)

Header guard macros are clearly not useful in the index, but are probably worth a comment. Builtin macros don't have useful locations, and also aren't needed for code completion as you say.

360

as above, can we avoid generating the USR for every reference?
The macro name or hash thereof should be enough to identify it, right?

[ioeric]

• Addressed review comments.

[ioeric]

clangd/index/SymbolCollector.cpp

360

I thought I needed to store both the name and the location in order to calculate USR for macros, and just calculating USRs seems easier and not too expensive. But after another look, it turned out that IdentifierInfo + Preprocessor would also work. Thanks for pressing on this! :)

clangd/index/SymbolCollector.h

struct Options {
     bool CollectIncludePath = false;
     /// If set, this is used to map symbol #include path to a potentially
     /// different #include path.
     const CanonicalIncludes *Includes = nullptr;
     // Populate the Symbol.References field.
     bool CountReferences = false;
     // Every symbol collected will be stamped with this origin.
     SymbolOrigin Origin = SymbolOrigin::Unknown;
+    /// Collect macros.
+    /// Note that SymbolCollector must be run with preprocessor in order to
+    /// collect macros. For example, `indexTopLevelDecls` will not index any
+    /// macro even if this is true.
+    bool CollectMacro = false;
   };
 
   SymbolCollector(Options Opts);
 
   /// Returns true is \p ND should be collected.
   /// AST matchers require non-const ASTContext.
   static bool shouldCollectSymbol(const NamedDecl &ND, ASTContext &ASTCtx,
                                   const Options &Opts);
 
   void initialize(ASTContext &Ctx) override;
 
   void setPreprocessor(std::shared_ptr<Preprocessor> PP) override {
     this->PP = std::move(PP);
   }
 
   bool
   handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
                       ArrayRef<index::SymbolRelation> Relations,
                       SourceLocation Loc,
                       index::IndexDataConsumer::ASTNodeInfo ASTNode) override;
 
+  bool handleMacroOccurence(const IdentifierInfo &Name, const MacroInfo &MI,
+                            index::SymbolRoleSet Roles,
+                            SourceLocation Loc) override;
+
   SymbolSlab takeSymbols() { return std::move(Symbols).build(); }
 
   void finish() override;
 
 private:
   const Symbol *addDeclaration(const NamedDecl &, SymbolID);
   void addDefinition(const NamedDecl &, const Symbol &DeclSymbol);
 
   // All Symbols collected from the AST.
   SymbolSlab::Builder Symbols;
   ASTContext *ASTCtx;
   std::shared_ptr<Preprocessor> PP;
   std::shared_ptr<GlobalCodeCompletionAllocator> CompletionAllocator;
   std::unique_ptr<CodeCompletionTUInfo> CompletionTUInfo;
   Options Opts;
-  // Decls referenced from the current TU, flushed on finish().
-  llvm::DenseSet<const NamedDecl *> ReferencedDecls;
+  // Macros referenced from the current TU, flushed on finish().
+  llvm::DenseSet<SymbolID> ReferencedSymbols;
   // Maps canonical declaration provided by clang to canonical declaration for
   // an index symbol, if clangd prefers a different declaration than that
   // provided by clang. For example, friend declaration might be considered
   // canonical by clang but should not be considered canonical in the index
   // unless it's a definition.
   llvm::DenseMap<const Decl *, const Decl *> CanonicalDecls;
 };
 
 } // namespace clangd
 } // namespace clang

clangd/index/SymbolCollector.cpp

getIncludeHeader(llvm::StringRef QName, const SourceManager &SM,
   if (Opts.Includes) {
     Header = Opts.Includes->mapHeader(Headers, QName);
     if (Header.startswith("<") || Header.startswith("\""))
       return Header.str();
   }
   return toURI(SM, Header, Opts);
 }
 
-// Return the symbol location of the given declaration `D`.
-//
-// For symbols defined inside macros:
-//   * use expansion location, if the symbol is formed via macro concatenation.
-//   * use spelling location, otherwise.
-llvm::Optional<SymbolLocation> getSymbolLocation(
-    const NamedDecl &D, SourceManager &SM, const SymbolCollector::Options &Opts,
-    const clang::LangOptions &LangOpts, std::string &FileURIStorage) {
-  SourceLocation NameLoc = findNameLoc(&D);
-  auto U = toURI(SM, SM.getFilename(NameLoc), Opts);
+// Return the symbol location of the token at \p Loc.
+llvm::Optional<SymbolLocation>
+getTokenLocation(SourceLocation TokLoc, const SourceManager &SM,
+                 const SymbolCollector::Options &Opts,
+                 const clang::LangOptions &LangOpts,
+                 std::string &FileURIStorage) {
+  auto U = toURI(SM, SM.getFilename(TokLoc), Opts);
   if (!U)
     return llvm::None;
   FileURIStorage = std::move(*U);
   SymbolLocation Result;
   Result.FileURI = FileURIStorage;
-  auto TokenLength = clang::Lexer::MeasureTokenLength(NameLoc, SM, LangOpts);
+  auto TokenLength = clang::Lexer::MeasureTokenLength(TokLoc, SM, LangOpts);
 
   auto CreatePosition = [&SM](SourceLocation Loc) {
     auto LSPLoc = sourceLocToPosition(SM, Loc);
     SymbolLocation::Position Pos;
     Pos.Line = LSPLoc.line;
     Pos.Column = LSPLoc.character;
     return Pos;
   };
 
-  Result.Start = CreatePosition(NameLoc);
-  auto EndLoc = NameLoc.getLocWithOffset(TokenLength);
+  Result.Start = CreatePosition(TokLoc);
+  auto EndLoc = TokLoc.getLocWithOffset(TokenLength);
   Result.End = CreatePosition(EndLoc);
 
   return std::move(Result);
 }
 
 // Checks whether \p ND is a definition of a TagDecl (class/struct/enum/union)
 // in a header file, in which case clangd would prefer to use ND as a canonical
 // declaration.
bool SymbolCollector::handleDeclOccurence(
   // canonical declaration in the index. Use OrigD instead, unless we've already
   // picked a replacement for D
   if (D->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None)
     D = CanonicalDecls.try_emplace(D, ASTNode.OrigD).first->second;
   const NamedDecl *ND = llvm::dyn_cast<NamedDecl>(D);
   if (!ND)
     return true;
 
+  llvm::SmallString<128> USR;
+  if (index::generateUSRForDecl(ND, USR))
+    return true;
+  SymbolID ID(USR);
+
   // Mark D as referenced if this is a reference coming from the main file.
   // D may not be an interesting symbol, but it's cheaper to check at the end.
   auto &SM = ASTCtx->getSourceManager();
   if (Opts.CountReferences &&
       (Roles & static_cast<unsigned>(index::SymbolRole::Reference)) &&
       SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
-    ReferencedDecls.insert(ND);
+    ReferencedSymbols.insert(ID);
 
   // Don't continue indexing if this is a mere reference.
   if (!(Roles & static_cast<unsigned>(index::SymbolRole::Declaration) ||
         Roles & static_cast<unsigned>(index::SymbolRole::Definition)))
     return true;
   if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
     return true;
 
-  llvm::SmallString<128> USR;
-  if (index::generateUSRForDecl(ND, USR))
-    return true;
-  SymbolID ID(USR);
-
   const NamedDecl &OriginalDecl = *cast<NamedDecl>(ASTNode.OrigD);
   const Symbol *BasicSymbol = Symbols.find(ID);
   if (!BasicSymbol) // Regardless of role, ND is the canonical declaration.
     BasicSymbol = addDeclaration(*ND, std::move(ID));
   else if (isPreferredDeclaration(OriginalDecl, Roles))
     // If OriginalDecl is preferred, replace the existing canonical
     // declaration (e.g. a class forward declaration). There should be at most
     // one duplicate as we expect to see only one preferred declaration per
     // TU, because in practice they are definitions.
     BasicSymbol = addDeclaration(OriginalDecl, std::move(ID));
 
   if (Roles & static_cast<unsigned>(index::SymbolRole::Definition))
     addDefinition(OriginalDecl, *BasicSymbol);
   return true;
 }
 
-void SymbolCollector::finish() {
-  // At the end of the TU, add 1 to the refcount of the ReferencedDecls.
-  for (const auto *ND : ReferencedDecls) {
-    llvm::SmallString<128> USR;
-    if (!index::generateUSRForDecl(ND, USR))
-      if (const auto *S = Symbols.find(SymbolID(USR))) {
-        Symbol Inc = *S;
-        ++Inc.References;
-        Symbols.insert(Inc);
-      }
-  }
-  ReferencedDecls.clear();
+bool SymbolCollector::handleMacroOccurence(const IdentifierInfo &Name,
+                                           const MacroInfo &MI,
+                                           index::SymbolRoleSet Roles,
+                                           SourceLocation Loc) {
+  assert(PP.get() && "Preprocessor must be set.");
+  if (!Opts.CollectMacro)
+    return true;
+
+  const auto &SM = PP->getSourceManager();
+  if (SM.isInMainFile(SM.getExpansionLoc(MI.getDefinitionLoc())))
+    return true;
+  // Builtin macro should already be available in sema.
+  if (MI.isUsedForHeaderGuard() || MI.isBuiltinMacro())
+    return true;
+
+  if (index::generateUSRForMacro(Name.getName(), MI.getDefinitionLoc(), SM,
+                                 USR))
+    return true;
+  SymbolID ID(USR);
+
+  // Mark the macro as referenced if this is a reference coming from the main
+  // file. The macro may not be an interesting symbol, but it's cheaper to check
+  // at the end.
+  if (Opts.CountReferences &&
+      (Roles & static_cast<unsigned>(index::SymbolRole::Reference)) &&
+      SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
+    ReferencedSymbols.insert(ID);
+  // Don't continue indexing if this is a mere reference.
+  // FIXME: remove macro with ID if it is undefined.
+  if (!(Roles & static_cast<unsigned>(index::SymbolRole::Declaration) ||
+        Roles & static_cast<unsigned>(index::SymbolRole::Definition)))
+    return true;
+
+  // Only collect one instance in case there are multiple.
+  if (Symbols.find(ID) != nullptr)
+    return true;
+
+  Symbol S;
+  S.ID = std::move(ID);
+  S.Name = Name.getName();
+  S.IsIndexedForCodeCompletion = true;
+  S.SymInfo = index::getSymbolInfoForMacro(MI);
+  std::string FileURI;
+  if (auto DeclLoc = getTokenLocation(MI.getDefinitionLoc(), SM, Opts,
+                                      PP->getLangOpts(), FileURI))
+    S.CanonicalDeclaration = *DeclLoc;
+
+  CodeCompletionResult SymbolCompletion(&Name);
+  const auto *CCS = SymbolCompletion.CreateCodeCompletionStringForMacro(
+      *PP, *CompletionAllocator, *CompletionTUInfo);
+  std::string Signature;
+  std::string SnippetSuffix;
+  getSignature(*CCS, &Signature, &SnippetSuffix);
+
+  std::string Include;
+  if (Opts.CollectIncludePath && shouldCollectIncludePath(S.SymInfo.Kind)) {
+    if (auto Header =
+            getIncludeHeader(Name.getName(), SM,
+                             SM.getExpansionLoc(MI.getDefinitionLoc()), Opts))
+      Include = std::move(*Header);
+  }
+  S.Signature = Signature;
+  S.CompletionSnippetSuffix = SnippetSuffix;
+  Symbol::Details Detail;
+  Detail.IncludeHeader = Include;
+  S.Detail = &Detail;
+  Symbols.insert(S);
+  return true;
+}
+
+void SymbolCollector::finish() {
+  // At the end of the TU, add 1 to the refcount of all referenced symbols.
+  for (const auto &ID : ReferencedSymbols)
+    if (const auto *S = Symbols.find(ID)) {
+  ReferencedSymbols.clear();
 }
 
 const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND,
                                               SymbolID ID) {
   auto &Ctx = ND.getASTContext();
   auto &SM = Ctx.getSourceManager();
 
   Symbol S;
   S.ID = std::move(ID);
   std::string QName = printQualifiedName(ND);
   std::tie(S.Scope, S.Name) = splitQualifiedName(QName);
   // FIXME: this returns foo:bar: for objective-C methods, we prefer only foo:
   // for consistency with CodeCompletionString and a clean name/signature split.
 
   S.IsIndexedForCodeCompletion = isIndexedForCodeCompletion(ND, Ctx);
   S.SymInfo = index::getSymbolInfo(&ND);
   std::string FileURI;
-  if (auto DeclLoc =
-          getSymbolLocation(ND, SM, Opts, ASTCtx->getLangOpts(), FileURI))
+  if (auto DeclLoc = getTokenLocation(findNameLoc(&ND), SM, Opts,
+                                      ASTCtx->getLangOpts(), FileURI))
     S.CanonicalDeclaration = *DeclLoc;
 
   // Add completion info.
   // FIXME: we may want to choose a different redecl, or combine from several.
   assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
   // We use the primary template, as clang does during code completion.
   CodeCompletionResult SymbolCompletion(&getTemplateOrThis(ND), 0);
   const auto *CCS = SymbolCompletion.CreateCodeCompletionString(
void SymbolCollector::addDefinition(const NamedDecl &ND,
                                     const Symbol &DeclSym) {
   if (DeclSym.Definition)
     return;
   // If we saw some forward declaration, we end up copying the symbol.
   // This is not ideal, but avoids duplicating the "is this a definition" check
   // in clang::index. We should only see one definition.
   Symbol S = DeclSym;
   std::string FileURI;
-  if (auto DefLoc = getSymbolLocation(ND, ND.getASTContext().getSourceManager(),
-                                      Opts, ASTCtx->getLangOpts(), FileURI))
+  if (auto DefLoc = getTokenLocation(findNameLoc(&ND),
+                                     ND.getASTContext().getSourceManager(),
     S.Definition = *DefLoc;
   Symbols.insert(S);
 }
 
 } // namespace clangd
 } // namespace clang

unittests/clangd/SymbolCollectorTests.cpp

 
 TEST_F(SymbolCollectorTest, Origin) {
   CollectorOpts.Origin = SymbolOrigin::Static;
   runSymbolCollector("class Foo {};", /*Main=*/"");
   EXPECT_THAT(Symbols, UnorderedElementsAre(
                            Field(&Symbol::Origin, SymbolOrigin::Static)));
 }
 
+TEST_F(SymbolCollectorTest, CollectMacros) {
+  CollectorOpts.CollectIncludePath = true;
+  Annotations Header(R"(
+    #define X 1
+    #define $mac[[MAC]](x) int x
+    #define $used[[USED]](y) float y;
+
+    MAC(p);
+  )");
+  const std::string Main = R"(
+    #define MAIN 1  // not indexed
+    USED(t);
+  )";
+  CollectorOpts.CountReferences = true;
+  CollectorOpts.CollectMacro = true;
+  runSymbolCollector(Header.code(), Main);
+  EXPECT_THAT(
+      Symbols,
+      UnorderedElementsAre(
+          QName("p"),
+          AllOf(QName("X"), DeclURI(TestHeaderURI),
+                IncludeHeader(TestHeaderURI)),
+          AllOf(Labeled("MAC(x)"), Refs(0), DeclRange(Header.range("mac"))),
+          AllOf(Labeled("USED(y)"), Refs(1), DeclRange(Header.range("used")))));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang