[PATCH] D44954: [clangd] Add "member" symbols to the index

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.


================
Comment at: clangd/index/SymbolCollector.cpp:113
 
-  // We only want:
-  //   * symbols in namespaces or translation unit scopes (e.g. no class
-  //     members)
-  //   * enum constants in unscoped enum decl (e.g. "red" in "enum {red};")
-  auto InTopLevelScope = hasDeclContext(
-      anyOf(namespaceDecl(), translationUnitDecl(), linkageSpecDecl()));
-  // Don't index template specializations.
+  // Don't index template specializations and expansions in main files.
   auto IsSpecialization =
----------------
We should be careful when removing the `InTopLevelScope` filter. According to clang/AST/DeclBase.h, `DeclContext` can be any of the following. For example, indexing symbols in functions seems to be out of the scope of this patch. I think we should keep a whitelist instead of turning them all at once. It's unfortunately we don't have tests to catch those...
```
///   TranslationUnitDecl
///   NamespaceDecl
///   FunctionDecl
///   TagDecl
///   ObjCMethodDecl
///   ObjCContainerDecl
///   LinkageSpecDecl
///   ExportDecl
///   BlockDecl
///   OMPDeclareReductionDecl
```

For class members, I would be good to add more tests for symbol collector e.g. more realistic class layouts with constructor, friends etc.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44954