fix Issue 16037 - assigning delegate to a scope variable shouldn't al…

…locate closure
dlang · Mar 4, 2018 · 4b93dde · 4b93dde
1 parent acacaad
commit 4b93dde
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 5 deletions.
diff --git a/src/dmd/escape.d b/src/dmd/escape.d
@@ -46,7 +46,8 @@ import dmd.arraytypes;
  */
 bool checkParamArgumentEscape(Scope* sc, FuncDeclaration fdc, Identifier par, Expression arg, bool gag)
 {
-    //printf("checkParamArgumentEscape(arg: %s par: %s)\n", arg.toChars(), par.toChars());
+    enum log = false;
+    if (log) printf("checkParamArgumentEscape(arg: %s par: %s)\n", arg.toChars(), par.toChars());
     //printf("type = %s, %d\n", arg.type.toChars(), arg.type.hasPointers());
 
     if (!arg.type.hasPointers())
@@ -78,7 +79,7 @@ bool checkParamArgumentEscape(Scope* sc, FuncDeclaration fdc, Identifier par, Ex
 
     foreach (VarDeclaration v; er.byvalue)
     {
-        //printf("byvalue %s\n", v.toChars());
+        if (log) printf("byvalue %s\n", v.toChars());
         if (v.isDataseg())
             continue;
 
@@ -103,6 +104,8 @@ bool checkParamArgumentEscape(Scope* sc, FuncDeclaration fdc, Identifier par, Ex
             /* v is not 'scope', and is assigned to a parameter that may escape.
              * Therefore, v can never be 'scope'.
              */
+            if (log) printf("no infer for %s in %s, fdc %s, %d\n",
+                v.toChars(), sc.func.ident.toChars(), fdc.ident.toChars(),  __LINE__);
             v.doNotInferScope = true;
         }
     }
@@ -315,6 +318,7 @@ bool checkAssignEscape(Scope* sc, Expression e, bool gag)
             /* v is not 'scope', and we didn't check the scope of where we assigned it to.
              * It may escape via that assignment, therefore, v can never be 'scope'.
              */
+            //printf("no infer for %s in %s, %d\n", v.toChars(), sc.func.ident.toChars(), __LINE__);
             v.doNotInferScope = true;
         }
     }
@@ -390,6 +394,14 @@ ByRef:
         VarDeclarations vars;
         findAllOuterAccessedVariables(fd, &vars);
 
+        /* https://issues.dlang.org/show_bug.cgi?id=16037
+         * If assigning the address of a delegate to a scope variable,
+         * then uncount that address of. This is so it won't cause a
+         * closure to be allocated.
+         */
+        if (va && va.isScope() && fd.tookAddressOf && global.params.vsafe)
+            --fd.tookAddressOf;
+
         foreach (v; vars)
         {
             //printf("v = %s\n", v.toChars());
@@ -504,7 +516,7 @@ bool checkThrowEscape(Scope* sc, Expression e, bool gag)
         }
         else
         {
-            //printf("no infer for %s\n", v.toChars());
+            //printf("no infer for %s in %s, %d\n", v.toChars(), sc.func.ident.toChars(), __LINE__);
             v.doNotInferScope = true;
         }
     }
@@ -623,7 +635,7 @@ private bool checkReturnEscapeImpl(Scope* sc, Expression e, bool refs, bool gag)
         }
         else
         {
-            //printf("no infer for %s\n", v.toChars());
+            //printf("no infer for %s in %s, %d\n", v.toChars(), sc.func.ident.toChars(), __LINE__);
             v.doNotInferScope = true;
         }
     }

diff --git a/test/compilable/test16037.d b/test/compilable/test16037.d
@@ -0,0 +1,23 @@
+/* REQUIRED_ARGS: -dip1000
+ */
+
+// https://issues.dlang.org/show_bug.cgi?id=16037
+
+@safe:
+
+void testXXX () @nogc
+{
+    Object o;
+    scope bool delegate (Object) alwaysFalse = (Object y) { return false; };
+    scope c1 = o !is null ? (Object y) { return o is y; } : alwaysFalse;
+}
+
+auto f() @nogc
+{
+    int a;
+    void g(){ a=1; }
+    scope h=&g;
+    h();
+}
+
+
diff --git a/test/runnable/test42.d b/test/runnable/test42.d
@@ -4611,11 +4611,13 @@ void test7290()
     int add = 2;
     scope dg = (int a) => a + add;
 
+    // This will break with -dip1000 because a closure will no longer be allocated
     assert(GC.addrOf(dg.ptr) == null);
 
     foo7290a!dg();
     foo7290b(dg);
-    foo7290c(dg);
+    foo7290c(dg); // this will fail with -dip1000 and @safe because a scope delegate gets
+                  // assigned to @system delegate, but no closure was allocated
 }
 
 /***************************************************/