[clangd] Expose 'shouldCollectSymbol' helper from SymbolCollector.

[ioeric]

Summary

This allows tools to examine symbols that would be collected in a symbol index. For example, a tool that measures index-based completion quality would be interested in references to symbols that are collected in the index.

[sammccall]

Can you motivate this change a bit, and add tests?

clangd/index/SymbolCollector.h

61

in principle, this should take const Options&?

[ioeric]

• added tests.

[ioeric]

In D48418#1138984, @sammccall wrote:

Can you motivate this change a bit, and add tests?

Done.

clangd/index/SymbolCollector.h

 /// - Anonymous declarations (anonymous enum/class/struct, etc)
 /// - Declarations in anonymous namespaces
 /// - Local declarations (in function bodies, blocks, etc)
 /// - Declarations in main files
 /// - Template specializations
 /// - Library-specific private declarations (e.g. private declaration generated
 /// by protobuf compiler)
 ///
-/// See also shouldFilterDecl().
+/// See also shouldCollectSymbol(...).
 ///
 /// Clients (e.g. clangd) can use SymbolCollector together with
 /// index::indexTopLevelDecls to retrieve all symbols when the source file is
 /// changed.
 class SymbolCollector : public index::IndexDataConsumer {
 public:
   struct Options {
     /// When symbol paths cannot be resolved to absolute paths (e.g. files in
struct Options {
     /// different #include path.
     const CanonicalIncludes *Includes = nullptr;
     // Populate the Symbol.References field.
     bool CountReferences = 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);
+
   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,

clangd/index/SymbolCollector.cpp

bool isPrivateProtoDecl(const NamedDecl &ND) {
   // indexed. Outer_INNER is private. This heuristic relies on naming style, it
   // will include OUTER_INNER and exclude some_enum_constant.
   // FIXME: the heuristic relies on naming style (i.e. no underscore in
   // user-defined names) and can be improved.
   return (ND.getKind() != Decl::EnumConstant) ||
          std::any_of(Name.begin(), Name.end(), islower);
 }
 
-bool shouldFilterDecl(const NamedDecl *ND, ASTContext *ASTCtx,
-                      const SymbolCollector::Options &Opts) {
-  using namespace clang::ast_matchers;
-  if (ND->isImplicit())
-    return true;
-  // Skip anonymous declarations, e.g (anonymous enum/class/struct).
-  if (ND->getDeclName().isEmpty())
-    return true;
-
-  // FIXME: figure out a way to handle internal linkage symbols (e.g. static
-  // variables, function) defined in the .cc files. Also we skip the symbols
-  // in anonymous namespace as the qualifier names of these symbols are like
-  // `foo::<anonymous>::bar`, which need a special handling.
-  // In real world projects, we have a relatively large set of header files
-  // that define static variables (like "static const int A = 1;"), we still
-  // want to collect these symbols, although they cause potential ODR
-  // violations.
-  if (ND->isInAnonymousNamespace())
-    return true;
-
-  // We want most things but not "local" symbols such as symbols inside
-  // FunctionDecl, BlockDecl, ObjCMethodDecl and OMPDeclareReductionDecl.
-  // FIXME: Need a matcher for ExportDecl in order to include symbols declared
-  // within an export.
-  auto InNonLocalContext = hasDeclContext(anyOf(
-      translationUnitDecl(), namespaceDecl(), linkageSpecDecl(), recordDecl(),
-      enumDecl(), objcProtocolDecl(), objcInterfaceDecl(), objcCategoryDecl(),
-      objcCategoryImplDecl(), objcImplementationDecl()));
-  // Don't index template specializations and expansions in main files.
-  auto IsSpecialization =
-      anyOf(functionDecl(isExplicitTemplateSpecialization()),
-            cxxRecordDecl(isExplicitTemplateSpecialization()),
-            varDecl(isExplicitTemplateSpecialization()));
-  if (match(decl(allOf(unless(isExpansionInMainFile()), InNonLocalContext,
-                       unless(IsSpecialization))),
-            *ND, *ASTCtx)
-          .empty())
-    return true;
-
-  // Avoid indexing internal symbols in protobuf generated headers.
-  if (isPrivateProtoDecl(*ND))
-    return true;
-  return false;
-}
-
 // We only collect #include paths for symbols that are suitable for global code
 // completion, except for namespaces since #include path for a namespace is hard
 // to define.
 bool shouldCollectIncludePath(index::SymbolKind Kind) {
   using SK = index::SymbolKind;
   switch (Kind) {
   case SK::Macro:
   case SK::Enum:

 
 void SymbolCollector::initialize(ASTContext &Ctx) {
   ASTCtx = &Ctx;
   CompletionAllocator = std::make_shared<GlobalCodeCompletionAllocator>();
   CompletionTUInfo =
       llvm::make_unique<CodeCompletionTUInfo>(CompletionAllocator);
 }
 
+bool SymbolCollector::shouldCollectSymbol(const NamedDecl &ND,
+                                          ASTContext &ASTCtx) {
+  using namespace clang::ast_matchers;
+  if (ND.isImplicit())
+    return false;
+  // Skip anonymous declarations, e.g (anonymous enum/class/struct).
+  if (ND.getDeclName().isEmpty())
+    return false;
+
+  // FIXME: figure out a way to handle internal linkage symbols (e.g. static
+  // variables, function) defined in the .cc files. Also we skip the symbols
+  // in anonymous namespace as the qualifier names of these symbols are like
+  // `foo::<anonymous>::bar`, which need a special handling.
+  // In real world projects, we have a relatively large set of header files
+  // that define static variables (like "static const int A = 1;"), we still
+  // want to collect these symbols, although they cause potential ODR
+  // violations.
+  if (ND.isInAnonymousNamespace())
+    return false;
+
+  // We want most things but not "local" symbols such as symbols inside
+  // FunctionDecl, BlockDecl, ObjCMethodDecl and OMPDeclareReductionDecl.
+  // FIXME: Need a matcher for ExportDecl in order to include symbols declared
+  // within an export.
+  auto InNonLocalContext = hasDeclContext(anyOf(
+      translationUnitDecl(), namespaceDecl(), linkageSpecDecl(), recordDecl(),
+      enumDecl(), objcProtocolDecl(), objcInterfaceDecl(), objcCategoryDecl(),
+      objcCategoryImplDecl(), objcImplementationDecl()));
+  // Don't index template specializations and expansions in main files.
+  auto IsSpecialization =
+      anyOf(functionDecl(isExplicitTemplateSpecialization()),
+            cxxRecordDecl(isExplicitTemplateSpecialization()),
+            varDecl(isExplicitTemplateSpecialization()));
+  if (match(decl(allOf(unless(isExpansionInMainFile()), InNonLocalContext,
+                       unless(IsSpecialization))),
+            ND, ASTCtx)
+          .empty())
+    return false;
+
+  // Avoid indexing internal symbols in protobuf generated headers.
+  if (isPrivateProtoDecl(ND))
+    return false;
+  return true;
+}
+
 // Always return true to continue indexing.
 bool SymbolCollector::handleDeclOccurence(
     const Decl *D, index::SymbolRoleSet Roles,
     ArrayRef<index::SymbolRelation> Relations, SourceLocation Loc,
     index::IndexDataConsumer::ASTNodeInfo ASTNode) {
   assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
   assert(CompletionAllocator && CompletionTUInfo);
   assert(ASTNode.OrigD);
if (Opts.CountReferences &&
       (Roles & static_cast<unsigned>(index::SymbolRole::Reference)) &&
       SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID())
     ReferencedDecls.insert(ND);
 
   // 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 (shouldFilterDecl(ND, ASTCtx, Opts))
+  if (!shouldCollectSymbol(*ND, *ASTCtx))
     return true;
 
   llvm::SmallString<128> USR;
   if (index::generateUSRForDecl(ND, USR))
     return true;
   SymbolID ID(USR);
 
   const NamedDecl &OriginalDecl = *cast<NamedDecl>(ASTNode.OrigD);