aaron.ballman added a comment. In https://reviews.llvm.org/D39722#933838, @xazax.hun wrote:
> In https://reviews.llvm.org/D39722#933828, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D39722#933699, @a.sidorin wrote: > > > > > Hello Takafumi, > > > > > > This is almost OK to me but there is an inline comment we need to resolve > > > in order to avoid Windows buildbot failures. > > > In addition, as Gabor pointed, when we add a new matcher, we need to > > > update matcher documentation as well. To update the docs, you should > > > perform just three steps. > > > > > > 1. Rebase your patch onto latest master > > > 2. Launch script docs/tools/dump_ast_matchers.py > > > 3. Add changes made by this script to your commit. > > > > > > I'm a bit confused -- I don't see any ASTMatcher changes as part of this > > patch aside from what's in the test file. Are you suggesting the matcher > > there should be hoisted as a separate patch? > > > In a previous version the matcher was public. I think it either should be > public with documentation or it can remain in the test file and no doc update > required. I am fine with both solution. I'm fine with either exposing it as a public matcher or leaving it private for the test file, but if it gets exposed, I think it should be in a separate patch. https://reviews.llvm.org/D39722 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
