[PATCH] PR12208 - Problem with -fno-elide-constructors

[Arthur O'Dwyer]

On Wed, Dec 25, 2013 at 12:30 AM, David Wiberg <dwiberg at gmail.com> 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).

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

my $.02,
–Arthur