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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to