Izaron added a comment. In D119792#3324354 <https://reviews.llvm.org/D119792#3324354>, @Quuxplusone wrote:
> I think it would be an extremely good idea to commit all 20 of these test > cases to clang/test/CodeGenCXX/, with their current behavior, as a > preliminary patch. Then, D119792 <https://reviews.llvm.org/D119792> can more > clearly show (1) what behavior it's changing, (2) what behavior it's keeping > the same, and (3) the fact that it's not regressing any behavior. Also, > you'll help future-maintainers by giving them some extra test cases that have > already been identified as interesting, even if you personally aren't > changing behavior related to those particular test cases. > > (I recently took the same tactic with D119772 > <https://reviews.llvm.org/D119772> as a preliminary for D119184 > <https://reviews.llvm.org/D119184> + D119778 > <https://reviews.llvm.org/D119778>, and it was very helpful, at least to me. > :)) That's a superb idea, thanks! I will soon add all the cases to `clang/test/CodeGenCXX/nrvo.cpp` in an isolated patch. It indeed will explicitly show improvements in future patches and prevent silent NRVO regression. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119792/new/ https://reviews.llvm.org/D119792 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
