ioeric added a comment. In https://reviews.llvm.org/D24862#550637, @aaron.ballman wrote:
> In https://reviews.llvm.org/D24862#550628, @ioeric wrote: > > > In https://reviews.llvm.org/D24862#550615, @aaron.ballman wrote: > > > > > Should this perhaps be fixed in the AST matchers rather than in the check > > > itself? > > > > > > I'll file bugs for the crashes, but the fix in this patch is not that > > "dirty" - it simply changes the order of matchers to workaround the > > crashes. So I guess this doesn't need to wait for the fix in matcher > > library. > > > Whenever possible, we should be fixing the underlying bugs, not working > around them. Acked, and I totally agree with you :) It's just that the change in this patch would still be valid after the underlying bugs are fixed, so I thought it was fine to fix those bugs after this. https://reviews.llvm.org/D24862 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
