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.
pr12208-2.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
