[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

This revision was automatically updated to reflect the committed changes. Closed by commit rG3587b15abe68: [Clang] [P2025] More exhaustive tests for NRVO (authored by Izaron). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119927/new/ https://reviews

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

Izaron added a comment. @Quuxplusone, thank you for the test idea! I added `test25` that resembles the pattern. Could you please look if we're okay with `test25`? If everything is OK, I could merge the patch myself: I've got repository merge access =) P. S. Sorry for the long delay, I was rea

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

Izaron updated this revision to Diff 415904. Izaron added a comment. Herald added a project: All. Add the reverse_iterator-like pattern to tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119927/new/ https://reviews.llvm.org/D119927 Files: cl

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

Quuxplusone added inline comments. Comment at: clang/test/CodeGenCXX/nrvo.cpp:1537 + } + return x; // FIXME: NRVO could happen if B == false, but doesn't +} Quuxplusone wrote: > Nit: `s/if/when/` Serendipitously, I just ran into almost this exact scenario in D1

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

Izaron marked an inline comment as done. Izaron added a comment. @Quuxplusone Thanks for reviewing the patch! We can wait some time if someone else wants to take a look. Though I doubt if there may be major complaints on extendind the tests (especially with comments and references to a proposal)

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

Izaron marked an inline comment as done. Izaron added inline comments. Comment at: clang/test/CodeGenCXX/nrvo.cpp:165 if (B) -return y; - return x; +return y; // NRVO is impossible + return x; // NRVO is impossible Quuxplusone wrote: > Technically,

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

Izaron updated this revision to Diff 409389. Izaron added a comment. I placed links to corresponding p2025 examples. Some of the examples are reasonably absent from the test, such as 1st (RVO, not NRVO), 13th (consteval methods are not getting to LLVM IR), 17th (there are no NRVOs for non-class

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

Quuxplusone accepted this revision. Quuxplusone added a comment. This revision is now accepted and ready to land. LGTM FWIW. You might want to wait for someone more authoritative to take a look; but it's also only adding test coverage, so it seems pretty uncontroversial, I would think. ==

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

Izaron updated this revision to Diff 409374. Izaron added a comment. Thanks! Yes I should've write comments that are understandable not only for me =) I added comments to the existing tests as well. Though NRVO attribute is bound to the variable, I'm also more comfortable to place comments in th

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

Quuxplusone added a comment. Excellent! Comment at: clang/test/CodeGenCXX/nrvo.cpp:622 + if (b) +return x; + else Similar to what you did in `test5`, I think it'd be helpful to comment //on the return line itself// whether NRVO is (done|possible|impossib

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

Izaron created this revision. Izaron added reviewers: mizvekov, Quuxplusone, sammccall, rsmith, doug.gregor. Izaron requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is a preliminary patch ahead of D119792