fix Issue 20567 returning the result of a constructor should be NRVO (#20568)

compiler/src/dmd/declaration.d

@@ -95,6 +95,7 @@ extern (C++) abstract class Declaration : Dsymbol
       enum ignoreRead = 2; // ignore any reads of AliasDeclaration
       enum nounderscore = 4; // don't prepend _ to mangled name
       enum hidden       = 8; // don't print this in .di files
+      enum nrvo = 0x10;      /// forward to fd.nrvo_var when generating code
 
     // overridden symbol with pragma(mangle, "...")
     const(char)[] mangleOverride;

compiler/src/dmd/e2ir.d

@@ -632,7 +632,7 @@ elem* toElem(Expression e, ref IRState irs)
         if (se.var.toParent2())
             fd = se.var.toParent2().isFuncDeclaration();
 
-        const bool nrvo = fd && fd.isNRVO() && fd.nrvo_var == se.var;
+        const bool nrvo = fd && (fd.isNRVO() && fd.nrvo_var == se.var || se.var.adFlags & Declaration.nrvo && fd.shidden);
         if (nrvo)
             s = fd.shidden;
 

compiler/src/dmd/expressionsem.d

@@ -816,21 +816,22 @@ extern(D) bool arrayExpressionSemantic(
  * Params:
  *   sc = the scope where the expression is encountered
  *   e = the expression the needs to be moved or copied (source)
- *   t = if the struct defines a copy constructor, the type of the destination
+ *   t = if the struct defines a copy constructor, the type of the destination (can be NULL)
+ *   nrvo = true if the generated copy can be treated as NRVO
  *
  * Returns:
  *  The expression that copy constructs or moves the value.
  */
-extern (D) Expression doCopyOrMove(Scope *sc, Expression e, Type t = null)
+extern (D) Expression doCopyOrMove(Scope *sc, Expression e, Type t, bool nrvo)
 {
     if (auto ce = e.isCondExp())
     {
-        ce.e1 = doCopyOrMove(sc, ce.e1);
+        ce.e1 = doCopyOrMove(sc, ce.e1, null, nrvo);
-        ce.e2 = doCopyOrMove(sc, ce.e2);
+        ce.e2 = doCopyOrMove(sc, ce.e2, null, nrvo);
     }
     else
     {
-        e = e.isLvalue() ? callCpCtor(sc, e, t) : valueNoDtor(e);
+        e = e.isLvalue() ? callCpCtor(sc, e, t, nrvo) : valueNoDtor(e);
     }
     return e;
 }
@@ -839,12 +840,13 @@ extern (D) Expression doCopyOrMove(Scope *sc, Expression e, Type t = null)
  * If e is an instance of a struct, and that struct has a copy constructor,
  * rewrite e as:
  *    (tmp = e),tmp
- * Input:
+ * Params:
  *      sc = just used to specify the scope of created temporary variable
  *      destinationType = the type of the object on which the copy constructor is called;
  *                        may be null if the struct defines a postblit
+ *	nrvo = true if the generated copy can be treated as NRVO
  */
-private Expression callCpCtor(Scope* sc, Expression e, Type destinationType)
+private Expression callCpCtor(Scope* sc, Expression e, Type destinationType, bool nrvo)
 {
     //printf("callCpCtor(e: %s et: %s destinationType: %s\n", toChars(e), toChars(e.type), toChars(destinationType));
     auto ts = e.type.baseElemOf().isTypeStruct();
@@ -861,7 +863,9 @@ private Expression callCpCtor(Scope* sc, Expression e, Type destinationType)
      * This is not the most efficient, ideally tmp would be constructed
      * directly onto the stack.
      */
-    auto tmp = copyToTemp(STC.rvalue, "__copytmp", e);
+    VarDeclaration tmp = copyToTemp(STC.rvalue, "__copytmp", e);
+    if (nrvo)
+	tmp.adFlags |= Declaration.nrvo;
     if (sd.hasCopyCtor && destinationType)
     {
         // https://issues.dlang.org/show_bug.cgi?id=22619
@@ -2707,7 +2711,7 @@ private Type arrayExpressionToCommonType(Scope* sc, ref Expressions exps)
             continue;
         }
 
-        e = doCopyOrMove(sc, e);
+        e = doCopyOrMove(sc, e, null, false);
 
         if (!foundType && t0 && !t0.equals(e.type))
         {
@@ -3678,7 +3682,7 @@ private bool functionParameters(const ref Loc loc, Scope* sc,
                  */
                 Type tv = arg.type.baseElemOf();
                 if (!isRef && tv.ty == Tstruct)
-                    arg = doCopyOrMove(sc, arg, parameter ? parameter.type : null);
+                    arg = doCopyOrMove(sc, arg, parameter ? parameter.type : null, false);
             }
 
             (*arguments)[i] = arg;
@@ -12026,7 +12030,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
                 ce.trusted = true;
 
             exp = new CatElemAssignExp(exp.loc, exp.type, exp.e1, ecast);
-            exp.e2 = doCopyOrMove(sc, exp.e2);
+            exp.e2 = doCopyOrMove(sc, exp.e2, null, false);
         }
         else if (tb1.ty == Tarray &&
                  (tb1next.ty == Tchar || tb1next.ty == Twchar) &&
@@ -12651,7 +12655,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
         {
             if (exp.e1.op == EXP.arrayLiteral)
             {
-                exp.e2 = doCopyOrMove(sc, exp.e2);
+                exp.e2 = doCopyOrMove(sc, exp.e2, null, false);
                 // https://issues.dlang.org/show_bug.cgi?id=14686
                 // Postblit call appears in AST, and this is
                 // finally translated  to an ArrayLiteralExp in below optimize().
@@ -12690,7 +12694,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
         {
             if (exp.e2.op == EXP.arrayLiteral)
             {
-                exp.e1 = doCopyOrMove(sc, exp.e1);
+                exp.e1 = doCopyOrMove(sc, exp.e1, null, false);
             }
             else if (exp.e2.op == EXP.string_)
             {
@@ -16552,7 +16556,7 @@ private bool fit(StructDeclaration sd, const ref Loc loc, Scope* sc, Expressions
         if (e.op == EXP.error)
             return false;
 
-        (*elements)[i] = doCopyOrMove(sc, e);
+        (*elements)[i] = doCopyOrMove(sc, e, null, false);
     }
     return true;
 }

compiler/src/dmd/frontend.h

@@ -6567,6 +6567,8 @@ public:
 
     enum : int32_t { hidden = 8 };
 
+    enum : int32_t { nrvo = 16 };
+
     _d_dynamicArray< const char > mangleOverride;
     const char* kind() const override;
     uinteger_t size(const Loc& loc) final override;

compiler/src/dmd/funcsem.d

@@ -2962,6 +2962,7 @@ extern (D) void checkMain(FuncDeclaration fd)
  */
 extern (D) bool checkNRVO(FuncDeclaration fd)
 {
+    //printf("checkNRVO*() %s\n", fd.ident.toChars());
     if (!fd.isNRVO() || fd.returns is null)
         return false;
 

compiler/src/dmd/initsem.d

@@ -1630,7 +1630,7 @@ Expressions* resolveStructLiteralNamedArgs(StructDeclaration sd, Type t, Scope*
             continue;
         }
 
-        elems[fieldi] = doCopyOrMove(sc, ex);
+        elems[fieldi] = doCopyOrMove(sc, ex, null, false);
         ++fieldi;
     }
     if (errors)

compiler/src/dmd/s2ir.d

@@ -534,7 +534,7 @@ void Statement_toIR(Statement s, ref IRState irs, StmtState* stmtstate)
 
     void visitReturn(ReturnStatement s)
     {
-        //printf("s2ir.ReturnStatement: %s\n", s.toChars());
+        //printf("s2ir.ReturnStatement: %s\n", toChars(s.exp));
         BlockState *blx = irs.blx;
         BC bc;
 

compiler/src/dmd/semantic3.d

@@ -940,7 +940,7 @@ private extern(C++) final class Semantic3Visitor : Visitor
                              * If NRVO is not possible, all returned lvalues should call their postblits.
                              */
                             if (!funcdecl.isNRVO())
-                                exp = doCopyOrMove(sc2, exp, f.next);
+                                exp = doCopyOrMove(sc2, exp, f.next, true);
 
                             if (tret.hasPointers())
                                 checkReturnEscape(*sc2, exp, false);

compiler/src/dmd/statementsem.d

@@ -2460,7 +2460,7 @@ Statement statementSemanticVisit(Statement s, Scope* sc)
         /* https://dlang.org/spec/statement.html#return-statement
          */
 
-        //printf("ReturnStatement.dsymbolSemantic() %p, %s\n", rs, rs.toChars());
+        //printf("ReturnStatement.dsymbolSemantic() %s\n", toChars(rs));
 
         FuncDeclaration fd = sc.parent.isFuncDeclaration();
         if (fd.fes)

compiler/test/runnable/nrvo.d

@@ -22,9 +22,35 @@ void test1()
     assert(&r == s1ptr);
 }
 
+/***************************************************/
+// https://github.com/dlang/dmd/issues/20567
+
+struct S2
+{
+    int x;
+    this(ref S2 s) { x = s.x; }
+}
+
+S2 returnRval(ref S2 arg1, ref S2 arg2, int i)
+{
+    return i ? arg1 : arg2;
+}
+
+void test2()
+{
+    S2 s1, s2;
+    s1.x = 3;
+    s2.x = 4;
+    S2 s = returnRval(s1, s2, 0);
+    assert(s.x == 4);
+    s = returnRval(s1, s2, 1);
+    assert(s.x == 3);
+}
+
 /***************************************************/
 
 void main()
 {
     test1();
+    test2();
 }