Merge pull request #6963 from JohanEngelen/closurenonstatic

Fix Issue 17590 - Unnecessary GC alloc on returning static local struct
merged-on-behalf-of: Sebastian Wilzbach <sebi.wilzbach@gmail.com>

src/ddmd/func.d

@@ -3388,13 +3388,15 @@ extern (C++) class FuncDeclaration : Declaration
          * 2) has its address taken
          * 3) has a parent that escapes
          * 4) calls another nested function that needs a closure
-         * -or-
-         * 5) this function returns a local struct/class
          *
          * Note that since a non-virtual function can be called by
          * a virtual one, if that non-virtual function accesses a closure
          * var, the closure still has to be taken. Hence, we check for isThis()
          * instead of isVirtual(). (thanks to David Friedman)
+         *
+         * When the function returns a local struct or class, `requiresClosure`
+         * is already set to `true` upon entering this function when the
+         * struct/class refers to a local variable and a closure is needed.
          */
 
         //printf("FuncDeclaration::needsClosure() %s\n", toChars());
@@ -3452,30 +3454,6 @@ extern (C++) class FuncDeclaration : Declaration
         if (requiresClosure)
             goto Lyes;
 
-        /* Look for case (5)
-         */
-        if (closureVars.dim)
-        {
-            Type tret = type.toTypeFunction().next;
-            assert(tret);
-            tret = tret.toBasetype();
-            //printf("\t\treturning %s\n", tret.toChars());
-            if (tret.ty == Tclass || tret.ty == Tstruct)
-            {
-                Dsymbol st = tret.toDsymbol(null);
-                //printf("\t\treturning class/struct %s\n", tret.toChars());
-                for (Dsymbol s = st.parent; s; s = s.parent)
-                {
-                    //printf("\t\t\tparent = %s %s\n", s.kind(), s.toChars());
-                    if (s == this)
-                    {
-                        //printf("\t\treturning local %s\n", st.toChars());
-                        goto Lyes;
-                    }
-                }
-            }
-        }
-
         return false;
 
     Lyes:

test/compilable/test17590.d

@@ -0,0 +1,40 @@
+// REQUIRED_ARGS: -o-
+
+void lazyfun(scope lazy int a) @nogc;
+
+// Test that returning a local _static_ struct does not lead to allocation of a closure.
+auto foo_static(int a, bool b) @nogc {
+    static struct SInside {}
+
+    SInside res;
+
+    lazyfun(a);
+
+    return res;
+}
+
+// Test that returning a local _non-static_ struct that does not reference any local variable does not lead to allocation of a closure.
+auto foo_nonstatic(int a, bool b) @nogc {
+    struct SInside {}
+
+    SInside res;
+
+    lazyfun(a);
+
+    return res;
+}
+
+// Test that returning a local non-static struct that references a local variable does lead to allocation of a closure.
+static assert(!__traits(compiles, () @nogc => goo(1)));
+static assert(__traits(compiles, () => goo(1)));
+auto goo(T)(T a) {
+    struct SInside {
+        T foo() { return a; }
+    }
+
+    SInside res;
+
+    lazyfun(a);
+
+    return res;
+}