martong added a comment. In D136684#3884983 <https://reviews.llvm.org/D136684#3884983>, @balazske wrote:
> This is really a NFC-like change but not NFC because it has visible effects > of removing some crashes. I could not produce a test that provokes the wrong > case (when `getParents` returns an empty list). Is it enough to have no new > test here? If we can get a test for the crash it needs more investigation to > check if `RecursiveASTVisitor` works correct and probably a fix at other > place. In my opinion, this case a very rare case when the change is justified even without the tests. Of course, it would be better if you had a test that is a minimal reproducer for the crash you are talking about. But, since you remove a sub-optimal infrastructural element, that is the parent map, which relies on the mentioned `RecursiveASTVisitor`, I don't think that we need to hunt down bugs for those components that you just dumped out from the ASTImporter code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136684/new/ https://reviews.llvm.org/D136684 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
