Merge pull request #5893 from WalterBright/fix15192

fix Issue 15192 - DIP25: Nested ref returns are type checked unsoundly
dlang · Jul 1, 2016 · feb06a7 · feb06a7
2 parents c9e4291 + ad419f2
commit feb06a7
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 37 deletions.
diff --git a/src/escape.d b/src/escape.d
@@ -246,7 +246,11 @@ bool checkEscape(Scope* sc, Expression e, bool gag)
  */
 bool checkEscapeRef(Scope* sc, Expression e, bool gag)
 {
+    //import core.stdc.stdio : printf;
     //printf("[%s] checkEscapeRef, e = %s\n", e.loc.toChars(), e.toChars());
+    //printf("current function %s\n", sc.func.toChars());
+    //printf("parent2 function %s\n", sc.func.toParent2().toChars());
+
     extern (C++) final class EscapeRefVisitor : Visitor
     {
         alias visit = super.visit;
@@ -272,49 +276,68 @@ bool checkEscapeRef(Scope* sc, Expression e, bool gag)
         {
             assert(d);
             VarDeclaration v = d.isVarDeclaration();
-            if (v && v.toParent2() == sc.func)
+            if (!v)
+                return;
+
+            if (v.isDataseg())
+                return;
+
+            if ((v.storage_class & (STCref | STCout)) == 0 && v.toParent2() == sc.func)
             {
-                if (v.isDataseg())
-                    return;
-                if ((v.storage_class & (STCref | STCout)) == 0)
-                {
-                    error(loc, "escaping reference to local variable %s", v);
-                    return;
-                }
-                if (global.params.useDIP25 && (v.storage_class & (STCref | STCout)) && !(v.storage_class & (STCreturn | STCforeach)))
+                error(loc, "escaping reference to local variable %s", v);
+                return;
+            }
+
+            if (global.params.useDIP25 &&
+                (v.storage_class & (STCref | STCout)) &&
+                !(v.storage_class & (STCreturn | STCforeach)))
+            {
+                if (sc.func.flags & FUNCFLAGreturnInprocess && v.toParent2() == sc.func)
                 {
-                    if (sc.func.flags & FUNCFLAGreturnInprocess)
+                    //printf("inferring 'return' for variable '%s'\n", v.toChars());
+                    v.storage_class |= STCreturn;
+                    if (v == sc.func.vthis)
                     {
-                        //printf("inferring 'return' for variable '%s'\n", v.toChars());
-                        v.storage_class |= STCreturn;
-                        if (v == sc.func.vthis)
+                        TypeFunction tf = cast(TypeFunction)sc.func.type;
+                        if (tf.ty == Tfunction)
                         {
-                            TypeFunction tf = cast(TypeFunction)sc.func.type;
-                            if (tf.ty == Tfunction)
-                            {
-                                //printf("'this' too\n");
-                                tf.isreturn = true;
-                            }
+                            //printf("'this' too\n");
+                            tf.isreturn = true;
                         }
                     }
-                    else if (sc._module && sc._module.isRoot())
+                }
+                else if (sc._module && sc._module.isRoot())
+                {
+                    Dsymbol p = v.toParent2();
+                    if (p == sc.func)
                     {
                         //printf("escaping reference to local ref variable %s\n", v.toChars());
                         //printf("storage class = x%llx\n", v.storage_class);
                         error(loc, "escaping reference to local ref variable %s", v);
+                        return;
                     }
-                    return;
-                }
-                if (v.storage_class & STCref && v.storage_class & (STCforeach | STCtemp) && v._init)
-                {
-                    // (ref v = ex; ex)
-                    if (ExpInitializer ez = v._init.isExpInitializer())
+                    // Don't need to be concerned if v's parent does not return a ref
+                    FuncDeclaration fd = p.isFuncDeclaration();
+                    if (fd && fd.type && fd.type.ty == Tfunction)
                     {
-                        assert(ez.exp && ez.exp.op == TOKconstruct);
-                        Expression ex = (cast(ConstructExp)ez.exp).e2;
-                        ex.accept(this);
-                        return;
+                        TypeFunction tf = cast(TypeFunction)fd.type;
+                        if (tf.isref)
+                            error(loc, "escaping reference to outer local ref variable %s", v);
                     }
+
+                }
+                return;
+            }
+
+            if (v.storage_class & STCref && v.storage_class & (STCforeach | STCtemp) && v._init)
+            {
+                // (ref v = ex; ex)
+                if (ExpInitializer ez = v._init.isExpInitializer())
+                {
+                    assert(ez.exp && ez.exp.op == TOKconstruct);
+                    Expression ex = (cast(ConstructExp)ez.exp).e2;
+                    ex.accept(this);
+                    return;
                 }
             }
         }

diff --git a/test/fail_compilation/test15192.d b/test/fail_compilation/test15192.d
@@ -0,0 +1,23 @@
+/*
+REQUIRED_ARGS: -dip25
+TEST_OUTPUT:
+---
+fail_compilation/test15192.d(14): Error: escaping reference to outer local ref variable x
+---
+*/
+
+
+// https://issues.dlang.org/show_bug.cgi?id=15192
+
+ref int fun(ref int x) @safe
+{
+    ref int bar(){ return x; }
+    return bar();
+}
+
+ref int fun2(return ref int x) @safe
+{
+    ref int bar(){ return x; }
+    return bar();
+}
+
diff --git a/test/runnable/opover2.d b/test/runnable/opover2.d
@@ -1,4 +1,5 @@
 // PERMUTE_ARGS: -inline -O -property
+// REQUIRED_ARGS: -dip25
 
 // Test operator overloading
 
@@ -1363,7 +1364,7 @@ void test9453()
 
 struct S9496
 {
-	static S9496* ptr;
+    static S9496* ptr;
 
     size_t opDollar()
     {
@@ -1748,8 +1749,8 @@ void test11311()
     static struct Arr
     {
         S data;
-        ref S opIndex(int) { return data; }
-        ref S opSlice(int, int) { return data; }
+        ref S opIndex(int) return { return data; }
+        ref S opSlice(int, int) return { return data; }
     }
 
     {
@@ -1846,7 +1847,7 @@ void test20a()
     {
         A1 a1;
         alias a1 this;
-        ref int opIndexAssign(int) { return b; }
+        int opIndexAssign(int) { return b; }
     }
 
     stompStack();
@@ -1942,7 +1943,7 @@ void test14624()
     struct A1
     {
         int x;
-        ref int opIndex() { return x; }
+        ref int opIndex() return { return x; }
         ref int opSlice() { assert(0); }
     }
     {
@@ -1960,10 +1961,10 @@ void test14624()
     struct A2
     {
         int x;
-        ref int opIndex() { x = 10; return x; }
+        ref int opIndex() return               { x = 10; return x; }
         ref int opSlice() { assert(0); }
         ref int opSliceUnary(alias op)()       { x = 11; return x; }
-        ref int opSliceAssign(int)             { x = 12; return x; }
+        ref int opSliceAssign(int) return      { x = 12; return x; }
         ref int opSliceOpAssign(alias op)(int) { x = 13; return x; }
     }
     {