Fix bug 8788 The super constructor call can be prevented by mentionin…

…g "return"

Fixes the flow analysis by clearly distinguishing "ALL branches have called a
constructor" from "ANY branches have called a ctor".
There are a large number of special cases.

src/expression.c

@@ -8090,6 +8090,8 @@ Expression *CallExp::semantic(Scope *sc)
                         error("constructor calls not allowed in loops or after labels");
                     if (sc->callSuper & (CSXsuper_ctor | CSXthis_ctor))
                         error("multiple constructor calls");
+                    if ((sc->callSuper & CSXreturn) && !(sc->callSuper & CSXany_ctor))
+                        error("an earlier return statement skips constructor");
                     sc->callSuper |= CSXany_ctor | CSXsuper_ctor;
                 }
 
@@ -8130,6 +8132,8 @@ Expression *CallExp::semantic(Scope *sc)
                     error("constructor calls not allowed in loops or after labels");
                 if (sc->callSuper & (CSXsuper_ctor | CSXthis_ctor))
                     error("multiple constructor calls");
+                if ((sc->callSuper & CSXreturn) && !(sc->callSuper & CSXany_ctor))
+                    error("an earlier return statement skips constructor");
                 sc->callSuper |= CSXany_ctor | CSXthis_ctor;
             }
 

src/scope.c

@@ -196,25 +196,48 @@ void Scope::mergeCallSuper(Loc loc, unsigned cs)
     // The two paths are callSuper and cs; the result is merged into callSuper.
 
     if (cs != callSuper)
-    {   int a;
-        int b;
+    {   // Have ALL branches called a constructor?
+        int aAll = (cs        & (CSXthis_ctor | CSXsuper_ctor)) != 0;
+        int bAll = (callSuper & (CSXthis_ctor | CSXsuper_ctor)) != 0;
 
-        callSuper |= cs & (CSXany_ctor | CSXlabel);
-        if (cs & CSXreturn)
+        // Have ANY branches called a constructor?
+        bool aAny = (cs        & CSXany_ctor) != 0;
+        bool bAny = (callSuper & CSXany_ctor) != 0;
+
+        // Have any branches returned?
+        bool aRet = (cs        & CSXreturn) != 0;
+        bool bRet = (callSuper & CSXreturn) != 0;
+
+        bool ok = true;
+
+        // If one has returned without a constructor call, there must be never
+        // have been ctor calls in the other.
+        if ( (aRet && !aAny && bAny) ||
+             (bRet && !bAny && aAny))
+        {   ok = false;
+        }
+        // If one branch has called a ctor and then exited, anything the
+        // other branch has done is OK (except returning without a
+        // ctor call, but we already checked that).
+        else if (aRet && aAll)
         {
+            callSuper |= cs & (CSXany_ctor | CSXlabel);
         }
-        else if (callSuper & CSXreturn)
+        else if (bRet && bAll)
         {
             callSuper = cs | (callSuper & (CSXany_ctor | CSXlabel));
         }
         else
-        {
-            a = (cs        & (CSXthis_ctor | CSXsuper_ctor)) != 0;
-            b = (callSuper & (CSXthis_ctor | CSXsuper_ctor)) != 0;
-            if (a != b)
-                error(loc, "one path skips constructor");
-            callSuper |= cs;
+        {   // Both branches must have called ctors, or both not.
+            ok = (aAll == bAll);
+            // If one returned without a ctor, we must remember that
+            // (Don't bother if we've already found an error)
+            if (ok && aRet && !aAny)
+                callSuper |= CSXreturn;
+            callSuper |= cs & (CSXany_ctor | CSXlabel);
         }
+        if (!ok)
+            error(loc, "one path skips constructor");
     }
 }
 

src/statement.c

@@ -4009,13 +4009,7 @@ Statement *ReturnStatement::semantic(Scope *sc)
         }
     }
 
-    /* BUG: need to issue an error on:
-     *  this
-     *  {   if (x) return;
-     *      super();
-     *  }
-     */
-
+    // If any branches have called a ctor, but this branch hasn't, it's an error
     if (sc->callSuper & CSXany_ctor &&
         !(sc->callSuper & (CSXthis_ctor | CSXsuper_ctor)))
         error("return without calling constructor");

test/compilable/compile1.d

@@ -191,6 +191,110 @@ struct Foo1099 {
     mixin Mix1099!(2);
 }
 
+/**************************************************
+    8788 - super() and return
+**************************************************/
+
+class B8788 {
+        this ( ) { }
+}
+
+class C8788(int test) : B8788
+{
+    this ( int y )
+    {   // TESTS WHICH SHOULD PASS
+        static if (test == 1) {
+            if (y == 3) {
+                super();
+                return;
+            }
+            super();
+            return;
+        } else static if (test == 2) {
+            if (y == 3) {
+                super();
+                return;
+            }
+            super();
+        } else static if (test == 3) {
+            if (y > 3) {
+                if (y == 7) {
+                   super();
+                   return;
+                }
+                super();
+                return;
+            }
+            super();
+        } else static if (test == 4) {
+            if (y > 3) {
+                if (y == 7) {
+                   super();
+                   return;
+                }
+                else if (y> 5)
+                    super();
+                else super();
+                return;
+            }
+            super();
+        }
+        // TESTS WHICH SHOULD FAIL
+        else static if (test == 5) {
+            if (y == 3) {
+                super();
+                return;
+            }
+            return; // no super
+        } else static if (test == 6) {
+            if (y > 3) {
+                if (y == 7) {
+                   super();
+                   return;
+                }
+                super();
+            }
+            super(); // two calls
+        } else static if (test == 7) {
+            if (y == 3) {
+                return; // no super
+            }
+            super();
+        } else static if (test == 8) {
+            if (y > 3) {
+                if (y == 7) {
+                   return; // no super
+                }
+                super();
+                return;
+            }
+            super();
+        } else static if (test == 9) {
+            if (y > 3) {
+                if (y == 7) {
+                   super();
+                   return;
+                }
+                else if (y> 5)
+                    super();
+                else return; // no super
+                return;
+            }
+            super();
+        }
+    }
+}
+
+static assert( is(typeof( { new C8788!(1)(0); } )));
+static assert( is(typeof( { new C8788!(2)(0); } )));
+static assert( is(typeof( { new C8788!(3)(0); } )));
+static assert( is(typeof( { new C8788!(4)(0); } )));
+static assert(!is(typeof( { new C8788!(5)(0); } )));
+static assert(!is(typeof( { new C8788!(6)(0); } )));
+static assert(!is(typeof( { new C8788!(7)(0); } )));
+static assert(!is(typeof( { new C8788!(8)(0); } )));
+static assert(!is(typeof( { new C8788!(9)(0); } )));
+
 /**************************************************
     4967, 7058
 **************************************************/