steveire added a comment. In D56444#1351194 <https://reviews.llvm.org/D56444#1351194>, @JonasToth wrote:
> >> 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. > > I understand that you are working on a clean solution which is fine and can > be done in 9.0, but: > > - this patch is small, almost non-intrusive and can be obsolete in 9.0, i > think no one would be sad about that > - we need to fix this issue, lambda traversal is absolutly normal in > ASTMatchers. Once you include the STL in your code, you automatically > encounter this use case > - we break current code that is in trunk already (for a longer time) > - this patch does not break any other tests so far, so it does not seem to > inccur bigger issues, on the other hand redoing AST matching most likely will. > > The semantic issues that came up make sense to address (within this or a > similar complex patch), but the current change does not seem to change a lot. > Even if it does, introducing false positives/negatives for lambda-cases is > still better then leaving the crashing as is. Yes, I was mostly responding to the suggestion to treat lambdas as functionDecls(), which I think is too drastic to do now, with other solutions on the horizon which may be better. I don't object to something targeted to fixing the assert as you describe and which this patch seems to be. Sorry if that wasn't clear. 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
