ioeric added inline comments.
================ Comment at: clangd/index/SymbolCollector.cpp:80 + : decl(unless(isExpansionInMainFile())), + hasDeclContext(anyOf(namespaceDecl(), translationUnitDecl())))), + *D, *ASTCtx) ---------------- ilya-biryukov wrote: > ioeric wrote: > > ilya-biryukov wrote: > > > hokein wrote: > > > > These ast matchers seems to be relatively simple, maybe we can use the > > > > `Decl` methods to implement them at the moment. Or will we add more > > > > complex filters in the future? > > > I second this. Also the code would probably be more readable without > > > matchers in this particular example. > > Yes, I expect to borrow more matchers from include-fixer's > > find-all-symbols. Also, although `isExpansionInMainFile` seems easy to > > implement, I am inclined to reuse the existing code if possible. > Could you add a comment pointing to the code in include fixer, so that > everyone reading the code knows where the code comes from? > (It's generally useful in case there are bugs and borrowing more code from > include-fixer fixes that) Actually, I'd say we are borrowing ideas of filtering instead of the actual code from include-fixer, and the code behavior is also quite different, so it's probably not worth adding pointers to another project in the code. (But as a reference for us, the include-fixer code is .../extra/include-fixer/find-all-symbols/FindAllSymbols.cpp:119 :) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41823 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
