Merge pull request #9081 from WalterBright/fix16652

fix Issue 16652 - [Reg 2.071] returned rvalue destroyed too early

src/dmd/backend/blockopt.d

@@ -2216,19 +2216,43 @@ private void blassertsplit()
         foreach (el; ListRange(bel))
         {
             elem *e = list_elem(el);
-            if (e.Eoper == OPinfo)
+
+            int accumDctor(elem *e)
             {
-                if (e.EV.E1.Eoper == OPdctor)
-                    ++dctor;
-                else if (e.EV.E1.Eoper == OPddtor)
-                    --dctor;
+                while (1)
+                {
+                    if (OTunary(e.Eoper))
+                    {
+                        e = e.EV.E1;
+                        continue;
+                    }
+                    else if (OTbinary(e.Eoper))
+                    {
+                        accumDctor(e.EV.E1);
+                        e = e.EV.E2;
+                        continue;
+                    }
+                    else if (e.Eoper == OPdctor)
+                        ++dctor;
+                    else if (e.Eoper == OPddtor)
+                        --dctor;
+                    break;
+                }
+                return dctor;
             }
+
             if (dctor == 0 &&   // don't split block between a dctor..ddtor pair
                 e.Eoper == OPoror && e.EV.E2.Eoper == OPcall && e.EV.E2.EV.E1.Eoper == OPvar)
             {
                 Symbol *f = e.EV.E2.EV.E1.EV.Vsym;
                 if (f.Sflags & SFLexit)
                 {
+                    if (accumDctor(e.EV.E1))
+                    {
+                        accumDctor(e.EV.E2);
+                        continue;
+                    }
+
                     // Create exit block
                     ++numblks;
                     maxblks += 3;
@@ -2295,6 +2319,7 @@ private void blassertsplit()
                     goto L1;
                 }
             }
+            accumDctor(e);
         }
         b.Belem = bl_delist(list_reverse(bel));
     }

src/dmd/inline.d

@@ -1173,9 +1173,32 @@ public:
 
         void inlineFd()
         {
-            if (fd && fd != parent && canInline(fd, false, false, asStatements))
+            if (!fd || fd == parent)
+                return;
+
+            /* If the arguments generate temporaries that need destruction, the destruction
+             * must be done after the function body is executed.
+             * The easiest way to accomplish that is to do the inlining as an Expression.
+             * https://issues.dlang.org/show_bug.cgi?id=16652
+             */
+            bool asStates = asStatements;
+            if (asStates)
             {
-                expandInline(e.loc, fd, parent, eret, null, e.arguments, asStatements, eresult, sresult, again);
+                if (fd.inlineStatusExp == ILS.yes)
+                    asStates = false;           // inline as expressions
+                                                // so no need to recompute argumentsNeedDtors()
+                else if (argumentsNeedDtors(e.arguments))
+                    asStates = false;
+            }
+
+            if (canInline(fd, false, false, asStates))
+            {
+                expandInline(e.loc, fd, parent, eret, null, e.arguments, asStates, eresult, sresult, again);
+                if (asStatements && eresult)
+                {
+                    sresult = new ExpStatement(eresult.loc, eresult);
+                    eresult = null;
+                }
             }
         }
 
@@ -2123,3 +2146,122 @@ bool onlyOneAssign(VarDeclaration v, FuncDeclaration fd)
         return true;            // currently the only case handled atm
     return false;
 }
+
+/************************************************************
+ * See if arguments to a function are creating temporaries that
+ * will need destruction after the function is executed.
+ * Params:
+ *      arguments = arguments to function
+ * Returns:
+ *      true if temporaries need destruction
+ */
+
+private bool argumentsNeedDtors(Expressions* arguments)
+{
+    if (arguments)
+    {
+        foreach (arg; *arguments)
+        {
+            if (argNeedsDtor(arg))
+                return true;
+        }
+    }
+    return false;
+}
+
+/************************************************************
+ * See if argument to a function is creating temporaries that
+ * will need destruction after the function is executed.
+ * Params:
+ *      arg = argument to function
+ * Returns:
+ *      true if temporaries need destruction
+ */
+
+private bool argNeedsDtor(Expression arg)
+{
+    extern (C++) final class NeedsDtor : StoppableVisitor
+    {
+        alias visit = typeof(super).visit;
+        Expression arg;
+
+    public:
+        extern (D) this(Expression arg)
+        {
+            this.arg = arg;
+        }
+
+        override void visit(Expression)
+        {
+        }
+
+        override void visit(DeclarationExp de)
+        {
+            if (de != arg)
+                Dsymbol_needsDtor(de.declaration);
+        }
+
+        void Dsymbol_needsDtor(Dsymbol s)
+        {
+            /* This mirrors logic of Dsymbol_toElem() in e2ir.d
+             * perhaps they can be combined.
+             */
+            if (auto vd = s.isVarDeclaration())
+            {
+                s = s.toAlias();
+                if (s != vd)
+                    return Dsymbol_needsDtor(s);
+                else if (vd.isStatic() || vd.storage_class & (STC.extern_ | STC.tls | STC.gshared | STC.manifest))
+                    return;
+                if (vd.needsScopeDtor())
+                {
+                    stop = true;
+                }
+            }
+            else if (TemplateMixin tm = s.isTemplateMixin())
+            {
+                //printf("%s\n", tm.toChars());
+                if (tm.members)
+                {
+                    foreach (sm; *tm.members)
+                    {
+                        Dsymbol_needsDtor(sm);
+                    }
+                }
+            }
+            else if (TupleDeclaration td = s.isTupleDeclaration())
+            {
+                foreach (o; *td.objects)
+                {
+                    import dmd.root.rootobject;
+
+                    if (o.dyncast() == DYNCAST.expression)
+                    {
+                        Expression eo = cast(Expression)o;
+                        if (eo.op == TOK.dSymbol)
+                        {
+                            DsymbolExp se = cast(DsymbolExp)eo;
+                            Dsymbol_needsDtor(se.s);
+                        }
+                    }
+                }
+            }
+            else if (AttribDeclaration ad = s.isAttribDeclaration())
+            {
+                Dsymbols *decl = ad.include(null);
+                if (decl)
+                {
+                    foreach (d; *decl)
+                    {
+                        Dsymbol_needsDtor(d);
+                    }
+                }
+            }
+
+
+        }
+    }
+
+    scope NeedsDtor ct = new NeedsDtor(arg);
+    return walkPostorder(arg, ct);
+}

test/runnable/sdtor.d

@@ -883,7 +883,7 @@ void test34()
     for (int j = 0; j < xs.length; j++) {
         j++,j--;
         auto x = xs[j];
-        //foreach(x; xs) {
+        //foreach(x; xs)
         //printf("foreach x.i = %d\n", x[0].i);
         //assert(x[0].i == 1);
         printf("foreach x.i = %d\n", x.i);
@@ -4613,6 +4613,39 @@ class C66
     this() nothrow { notthrow(); }
 }
 
+/**********************************/
+// https://issues.dlang.org/show_bug.cgi?id=16652
+
+struct Vector
+{
+    this(ubyte a)
+    {
+        pragma(inline, false);
+        buf = a;
+    }
+
+    ~this()
+    {
+        pragma(inline, false);
+        buf = 0;
+    }
+
+    ubyte buf;
+}
+
+int bar16652(ubyte* v)
+{
+    pragma(inline, true);
+    assert(*v == 1);
+    return 0;
+}
+
+void test16652()
+{
+    bar16652(&Vector(1).buf);
+}
+
+
 /**********************************/
 
 int main()
@@ -4747,6 +4780,7 @@ int main()
     test65();
     test15661();
     test18045();
+    test16652();
 
     printf("Success\n");
     return 0;