Merge pull request #8167 from RazvanN7/Issue_18719

Fix Issue 18719 - Doubly-called constructor against member when using forwarding constructors
merged-on-behalf-of: Andrei Alexandrescu <andralex@users.noreply.github.com>

changelog/deprecate_double_initialization.dd

@@ -0,0 +1,47 @@
+Deprecate double initialization of immutable fields inside constructor
+
+Inside a constructor scope, assigning to aggregate declaration (class/struct)
+members is done by considering the first assignment as initialization and
+subsequent assignments as modifications of the initially constructed object.
+For `const`/`immutable` fields the initialization is accepted in the constructor,
+but subsequent modifications are not. Example:
+
+---
+struct A
+{
+    int a;
+    immutable int b;
+    this(int a, int b)
+    {
+        this.a = a;
+        this.b = b;
+
+        this.a = 7; // OK, a is mutable
+        this.b = 9; // Error: immutable field b initialized multiple times
+    }
+}
+---
+
+However, $(BUGZILLA 18719) shows that this rule does not apply when inside
+a constructor scope there is a call to a different constructor:
+
+---
+struct A
+{
+    immmutable int a;
+    this()
+    {
+        this(42);
+        this.a = 5;  // second initialization of immutable field
+    }
+
+    this(int a)
+    {
+        this.a = a;
+    }
+}
+---
+
+The above code wrongfully compiled succesfully before this patch, accepting the double
+initialization of the `immutable` field `a`. After this patch, `this.a = 5` will issue
+a deprecation warning stating that `a` is initialized multiple times.

src/dmd/ctorflow.d

@@ -18,7 +18,7 @@ import core.stdc.stdio;
 
 import dmd.root.rmem;
 
-enum CSX : ubyte
+enum CSX : ushort
 {
     none            = 0,
     this_ctor       = 0x01,     /// called this()
@@ -29,6 +29,7 @@ enum CSX : ubyte
     return_         = 0x20,     /// seen a return statement
     any_ctor        = 0x40,     /// either this() or super() was called
     halt            = 0x80,     /// assert(0)
+    deprecate_18719 = 0x100,    // issue deprecation for Issue 18719 - delete when deprecation period is over
 }
 
 /***********

src/dmd/declaration.d

@@ -121,7 +121,15 @@ private int modifyFieldVar(Loc loc, Scope* sc, VarDeclaration var, Expression e1
                     else
                     {
                         const(char)* modStr = !var.type.isMutable() ? MODtoChars(var.type.mod) : MODtoChars(e1.type.mod);
-                        .error(loc, "%s field `%s` initialized multiple times", modStr, var.toChars());
+                        // Deprecated in 2018-04.
+                        // Change to error in 2019-04 by deleting the following
+                        // if-branch and the deprecate_18719 enum member in the
+                        // dmd.ctorflow.CSX enum.
+                        // @@@DEPRECATED_2019-01@@@.
+                        if (fi & CSX.deprecate_18719)
+                            .deprecation(loc, "%s field `%s` initialized multiple times", modStr, var.toChars());
+                        else
+                            .error(loc, "%s field `%s` initialized multiple times", modStr, var.toChars());
                     }
                 }
                 else if (sc.inLoop || (fi & CSX.label))

src/dmd/expressionsem.d

@@ -3271,11 +3271,22 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
             }
             else
             {
+                // `this` call expression must be inside a
+                // constructor
                 if (!ad || !sc.func.isCtorDeclaration())
                 {
                     exp.error("constructor call must be in a constructor");
                     return setError();
                 }
+
+                // https://issues.dlang.org/show_bug.cgi?id=18719
+                // If `exp` is a call expression to another constructor
+                // then it means that all struct/class fields will be
+                // initialized after this call.
+                foreach (ref field; sc.ctorflow.fieldinit)
+                {
+                    field |= CSX.this_ctor | CSX.deprecate_18719;
+                }
             }
 
             if (!sc.intypeof && !(sc.ctorflow.callSuper & CSX.halt))

test/fail_compilation/fail18719.d

@@ -0,0 +1,40 @@
+// https://issues.dlang.org/show_bug.cgi?id=18719
+
+// REQUIRED_ARGS: -de
+/*
+TEST_OUTPUT:
+---
+fail_compilation/fail18719.d(29): Deprecation: immutable field `x` initialized multiple times
+---
+*/
+
+struct S
+{
+    int x = -1;
+    this(int y) immutable
+    {
+        x = y;
+        import std.stdio;
+        writeln("Ctor called with ", y);
+    }
+    void opAssign(int) immutable;
+}
+
+class C
+{
+    S x;
+    this() immutable
+    {
+        this(42); /* Initializes x. */
+        x = 13; /* Breaking immutable, or ok? */
+    }
+    this(int x) immutable
+    {
+        this.x = x;
+    }
+}
+
+void main()
+{
+    new immutable C;
+}