Skip to content

Commit

Permalink
Fix Issue 18719 - Doubly-called constructor against member when using…
Browse files Browse the repository at this point in the history
… forwarding constructors
  • Loading branch information
RazvanN7 committed Apr 16, 2018
1 parent 1edd074 commit cbb49cb
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 2 deletions.
47 changes: 47 additions & 0 deletions changelog/deprecate_double_initialization.dd
Original file line number Diff line number Diff line change
@@ -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.
3 changes: 2 additions & 1 deletion src/dmd/ctorflow.d
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
}

/***********
Expand Down
10 changes: 9 additions & 1 deletion src/dmd/declaration.d
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
11 changes: 11 additions & 0 deletions src/dmd/expressionsem.d
Original file line number Diff line number Diff line change
Expand Up @@ -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)*/)
Expand Down
40 changes: 40 additions & 0 deletions test/fail_compilation/fail18719.d
Original file line number Diff line number Diff line change
@@ -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;
}

0 comments on commit cbb49cb

Please sign in to comment.