steveire added a comment. In D56444#1351145 <https://reviews.llvm.org/D56444#1351145>, @sammccall wrote:
> In D56444#1351128 <https://reviews.llvm.org/D56444#1351128>, @aaron.ballman > wrote: > > > In D56444#1351127 <https://reviews.llvm.org/D56444#1351127>, @steveire > > wrote: > > > > > I suggest not trying to make any such drastic changes for 8.0, try to fix > > > the bug in a minimal way if possible, and have a more considered approach > > > to the future of AST Matchers for after the release. > > > > > > +1 > > > Can you elaborate? > > This patch is an attempt to solve the problem "some nodes associated with > lambdas have no parents". This is not a new kind of issue. I reported a variant of it too months ago: https://bugs.llvm.org/show_bug.cgi?id=37629. As such I do think that this is not so urgent and can be fixed after the traverser class is extracted. One of the problems is that visually tracking through parents/children of an AST dump does not represent the same relationships as can be expressed with hasParent() AST Matcher, because AST Matchers use the RAV and AST dumps do not. However, we can make both use the same class (the generic traverser I am extracting from ASTDumper), and then that class of bug will be solved as I understand it. > This manifests as assertion failures (or with assertions off, incorrect > results) for some matchers, on some code - people have encountered several > examples where this happens, it's hard to know where to target a more > targeted fix. > The invariant violated is "RAV traverses every AST node (when implicit > traversal is enabled)" - is there a way to fix this issue sufficiently > without addressing that? Yes - don't use RAV to traverse parents when AST-matching. > Is the suggestion to make that change, but then modify the semantics of the > `functionDecl()` etc matchers to hide it? Or something else? My suggestion is to extract the traverser from ASTDumper first, then fix this issue. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56444/new/ https://reviews.llvm.org/D56444 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
