[PATCH] D41823: [clangd] Add more filters for collected symbols.

Ilya Biryukov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 10 08:05:03 PST 2018


ilya-biryukov added inline comments.


================
Comment at: clangd/index/SymbolCollector.cpp:80
+                                    : decl(unless(isExpansionInMainFile())),
+                hasDeclContext(anyOf(namespaceDecl(), translationUnitDecl())))),
+            *D, *ASTCtx)
----------------
ioeric wrote:
> 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 :)
I thought we were borrowing code after reading this comment:
> I expect to borrow more matchers from include-fixer's find-all-symbols. 

Thanks for the reference, I'd say it's still useful to have a reference in the code:
```// The code is similar to include fixer, see include-fixer/find-all-symbols/FindAllSymbols.cpp for more details.```
But it's up to you, of course.


Repository:
  rL LLVM

https://reviews.llvm.org/D41823





More information about the llvm-commits mailing list