Ping 2013/12/25 David Wiberg <[email protected]>: > 2013/12/25 David Wiberg <[email protected]>: >> 2013/12/25 Arthur O'Dwyer <[email protected]>: >>> On Wed, Dec 25, 2013 at 12:30 AM, David Wiberg <[email protected]> wrote: >>>> >>>> The attached patch adds a test-case for which incorrect code is >>>> generated and a change which makes the test pass. >>>> >>>> From what I could tell the following happened: >>>> 1. Sema::computeNRVO marked a variable as suitable for NRVO >>>> 2. CodeGenFunction::EmitAutoVarAlloca handled allocating variables to >>>> the return slot based on getLangOpts().ElideConstructors and the flag >>>> set by Sema. >>>> 3. CodeGenFunction::EmitReturnStmt only looked at the flag set by Sema >>>> to determine if necessary to emit a return statement. >>>> >>>> The patch changes EmitReturnStmt to require that the ElideConstructors >>>> flag is set to perform NRVO. Another option (which perhaps is better) >>>> would be to let Sema::computeNRVO return early if ElideConstructors >>>> wasn't set and remove the ElideConstructors check within >>>> EmitAutoVarAlloca. Since I haven't worked with this code before I >>>> opted for the least intrusive change. >>> >>> Speaking as the last commenter on PR12208, I heartily approve anything >>> that has the potential to fix -fno-elide-constructors. :) >>> However, I think you should add at least one test of the same thing in >>> C++11 mode (where we expect the move-constructor to be called, not the >>> copy-constructor). >>> >> This sounds like a good idea, I will have a look at what changes are >> required. >>> Would it make sense to add the exact std::string and std::map test >>> cases from the bug report? >>> http://llvm.org/bugs/show_bug.cgi?id=12208 >> The reason I left those out was to avoid the dependencies and keep the >> test as simple as possible. I can add those if people think it's a >> good idea. >>> >>> my $.02, >>> –Arthur >> >> Thank you for the comments! >> >> Best regards >> David > > The attached patch checks that move constructor is used in C++11 mode > as per Arthur's suggestion.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
