Skip to content

Commit

Permalink
Disallow initializing fields with themself
Browse files Browse the repository at this point in the history
  • Loading branch information
dkorpel committed Jan 20, 2025
1 parent 2ecfa63 commit ec1101f
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 3 deletions.
15 changes: 15 additions & 0 deletions changelog/dmd.deprecation-noop-assignment.dd
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
Initializing a field with itself has been deprecated

This is to prevent a common mistake when typing a simple constructor, where a parameter name is misspelled:

---
struct S
{
int field;

this(int feild)
{
this.field = field; // equal to this.field = this.field
}
}
---
29 changes: 29 additions & 0 deletions compiler/src/dmd/expressionsem.d
Original file line number Diff line number Diff line change
Expand Up @@ -10604,6 +10604,35 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
if (exp.op == EXP.assign
&& exp.e1.checkModifiable(sc) == Modifiable.initialization)
{
// Check common mistake of misspelled parameters in constructors,
// e.g. `this(int feild) { this.field = field; }`
if (auto dve1 = exp.e1.isDotVarExp)
if (auto dve2 = exp.e2.isDotVarExp)
if (sc.func && sc.func.parameters && dve1.e1.isThisExp && dve2.e1.isThisExp()
&& dve1.var.ident.equals(dve2.var.ident))
{
// @@@DEPRECATED_2.121@@@
// Deprecated in 2.111, make it an error in 2.121
deprecation(exp.e1.loc, "cannot initialize field `%s` with itself", dve1.var.toChars());

Check warning on line 10616 in compiler/src/dmd/expressionsem.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/expressionsem.d#L10616

Added line #L10616 was not covered by tests
auto findParameter(const(char)[] s, ref int cost)
{
foreach (p; *sc.func.parameters)

Check warning on line 10619 in compiler/src/dmd/expressionsem.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/expressionsem.d#L10619

Added line #L10619 was not covered by tests
{
if (p.ident.toString == s)

Check warning on line 10621 in compiler/src/dmd/expressionsem.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/expressionsem.d#L10621

Added line #L10621 was not covered by tests
{
cost = 1;
return p.ident.toString;

Check warning on line 10624 in compiler/src/dmd/expressionsem.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/expressionsem.d#L10623-L10624

Added lines #L10623 - L10624 were not covered by tests
}
}
return null;

Check warning on line 10627 in compiler/src/dmd/expressionsem.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/expressionsem.d#L10627

Added line #L10627 was not covered by tests
}
import dmd.root.speller : speller;
if (auto s = speller!findParameter(dve1.var.ident.toString))

Check warning on line 10630 in compiler/src/dmd/expressionsem.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/expressionsem.d#L10630

Added line #L10630 was not covered by tests
{
deprecationSupplemental(sc.func.loc, "did you mean to use parameter `%.*s`?\n", s.fTuple.expand);

Check warning on line 10632 in compiler/src/dmd/expressionsem.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/expressionsem.d#L10632

Added line #L10632 was not covered by tests
}
}

//printf("[%s] change to init - %s\n", exp.loc.toChars(), exp.toChars());
auto t = exp.type;
exp = new ConstructExp(exp.loc, exp.e1, exp.e2);
Expand Down
4 changes: 2 additions & 2 deletions compiler/test/compilable/interpret3.d
Original file line number Diff line number Diff line change
Expand Up @@ -6240,9 +6240,9 @@ struct Coord13831

struct Chunk13831
{
this(Coord13831)
this(Coord13831 coord)
{
coord = coord;
this.coord = coord;
}

Coord13831 coord;
Expand Down
2 changes: 1 addition & 1 deletion compiler/test/compilable/test22510.d
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ struct S
@disable this(this);
this (scope ref inout S) inout
{
this.b = b;
this.b = 0;
}
}

Expand Down
23 changes: 23 additions & 0 deletions compiler/test/fail_compilation/ctor_self_assignment.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/**
REQUIRED_ARGS: -de
TEST_OUTPUT:
---
fail_compilation/ctor_self_assignment.d(17): Deprecation: cannot initialize field `location` with itself
fail_compilation/ctor_self_assignment.d(15): did you mean to use parameter `locaction`?
---
*/
// https://forum.dlang.org/post/[email protected]

alias Location = int;

struct Node
{
this(Location locaction, uint f)
{
this.location = location;
this.f = f;
}

Location location;
uint f;
}

0 comments on commit ec1101f

Please sign in to comment.