Skip to content

Commit

Permalink
Fix Bugzilla Issue 20876: Implement the generation of multiple copy c…
Browse files Browse the repository at this point in the history
…onstructors for fields that define copy constructors
  • Loading branch information
RazvanN7 committed Apr 30, 2024
1 parent 748fab1 commit 8fe5dcb
Show file tree
Hide file tree
Showing 11 changed files with 176 additions and 56 deletions.
82 changes: 61 additions & 21 deletions compiler/src/dmd/clone.d
Original file line number Diff line number Diff line change
Expand Up @@ -1695,27 +1695,67 @@ bool buildCopyCtor(StructDeclaration sd, Scope* sc)
if (!needCopyCtor(sd, hasCpCtor))
return hasCpCtor;

//printf("generating copy constructor for %s\n", sd.toChars());
const MOD paramMod = MODFlags.wild;
const MOD funcMod = MODFlags.wild;
auto ccd = generateCopyCtorDeclaration(sd, ModToStc(paramMod), ModToStc(funcMod));
auto copyCtorBody = generateCopyCtorBody(sd);
ccd.fbody = copyCtorBody;
sd.members.push(ccd);
ccd.addMember(sc, sd);
const errors = global.startGagging();
Scope* sc2 = sc.push();
sc2.stc = 0;
sc2.linkage = LINK.d;
ccd.dsymbolSemantic(sc2);
ccd.semantic2(sc2);
ccd.semantic3(sc2);
//printf("ccd semantic: %s\n", ccd.type.toChars());
sc2.pop();
if (global.endGagging(errors) || sd.isUnionDeclaration())
// some fields have copy ctors
// so we need to collect the copy ctor types

// hashtable used to store what copy constructors should be generated
bool[ModBits] copyCtorTable;

// see if any struct members define a copy constructor
foreach (v; sd.fields)
{
if (v.storage_class & STC.ref_)
continue;

Check warning on line 1708 in compiler/src/dmd/clone.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/clone.d#L1708

Added line #L1708 was not covered by tests
if (v.overlapped)
continue;

Check warning on line 1710 in compiler/src/dmd/clone.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/clone.d#L1710

Added line #L1710 was not covered by tests
Type tv = v.type.baseElemOf();
if (tv.ty != Tstruct)
continue;

Check warning on line 1713 in compiler/src/dmd/clone.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/clone.d#L1713

Added line #L1713 was not covered by tests

auto ts = v.type.baseElemOf().isTypeStruct();
if (!ts)
continue;

Check warning on line 1717 in compiler/src/dmd/clone.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/clone.d#L1717

Added line #L1717 was not covered by tests
if (ts.sym.hasCopyCtor)
{
foreach(key; ts.sym.copyCtorsQualifiers.keys())
{
copyCtorTable[key] = true;
}
}
}

// if any field defines a copy constructor
if (copyCtorTable.length)
{
ccd.storage_class |= STC.disable;
ccd.fbody = null;
// generate the body that does memberwise initialization
auto copyCtorBody = generateCopyCtorBody(sd);
foreach (key; copyCtorTable.keys)
{
MOD paramMod = cast(MOD)(key >> 8);
MOD funcMod = cast(MOD)key;
auto ccd = generateCopyCtorDeclaration(sd, ModToStc(paramMod), ModToStc(funcMod));
//printf("generating for %s\n", ccd.type.toChars());
ccd.fbody = copyCtorBody.syntaxCopy();
sd.members.push(ccd);
ccd.addMember(sc, sd);
const errors = global.startGagging();
Scope* sc2 = sc.push();
sc2.stc = 0;
sc2.linkage = LINK.d;
ccd.dsymbolSemantic(sc2);
ccd.semantic2(sc2);
ccd.semantic3(sc2);
//printf("ccd semantic: %s\n", ccd.type.toChars());
sc2.pop();
if (global.endGagging(errors) || sd.isUnionDeclaration())
{
ccd.storage_class |= STC.disable;
ccd.fbody = null;
}
}

return true;
}
return true;

return false;

Check warning on line 1760 in compiler/src/dmd/clone.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/clone.d#L1760

Added line #L1760 was not covered by tests
}
3 changes: 2 additions & 1 deletion compiler/src/dmd/declaration.d
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,8 @@ extern (C++) abstract class Declaration : Dsymbol
{
if (ctor.isCpCtor && ctor.isGenerated())
{
.error(loc, "generating an `inout` copy constructor for `struct %s` failed, therefore instances of it are uncopyable", parent.toPrettyChars());
.error(loc, "generated copy constructor of type `%s` is disabled", ctor.type.toChars());
.errorSupplemental(loc, "some of the field types of struct `%s` do not define a copy constructor that can handle such copies", toParent().toChars());

Check warning on line 370 in compiler/src/dmd/declaration.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/declaration.d#L369-L370

Added lines #L369 - L370 were not covered by tests
return true;
}
}
Expand Down
3 changes: 3 additions & 0 deletions compiler/src/dmd/dstruct.d
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,16 @@ enum StructFlags : int
hasPointers = 0x1, // NB: should use noPointers as in ClassFlags
}

alias ModBits = ushort;

/***********************************************************
* All `struct` declarations are an instance of this.
*/
extern (C++) class StructDeclaration : AggregateDeclaration
{
FuncDeclarations postblits; // Array of postblit functions
FuncDeclaration postblit; // aggregate postblit
bool[ModBits] copyCtorsQualifiers; // source-destination qualifiers for the struct cpctors

FuncDeclaration xeq; // TypeInfo_Struct.xopEquals
FuncDeclaration xcmp; // TypeInfo_Struct.xopCmp
Expand Down
20 changes: 20 additions & 0 deletions compiler/src/dmd/dsymbolsem.d
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,24 @@ bool checkDeprecated(Dsymbol d, const ref Loc loc, Scope* sc)
return true;
}

/**
Fit two mods in a ushort. This is used to create a
mod key that represents the source and destination
qualifiers for a copy constructor. The result of this
function is used as a key in a hashtable.
Params:
mod1 = qualifier for source
mod2 = qualifier for destination
Returns:
The qualifiers key
*/
ModBits createModKey(MOD mod1, MOD mod2)
{
return ((mod1 << 8) | mod2);
}

/*********************************
* Check type to see if it is based on a deprecated symbol.
*/
Expand Down Expand Up @@ -2422,6 +2440,8 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor
{
//printf("copy constructor\n");
ctd.isCpCtor = true;
ModBits key = createModKey(param.type.mod, tf.mod);
sd.copyCtorsQualifiers[key] = true;
}
}
}
Expand Down
25 changes: 25 additions & 0 deletions compiler/test/compilable/test20876.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// https://issues.dlang.org/show_bug.cgi?id=20876

struct Array
{
void opSliceAssign(Foo) {}
void opSliceAssign(Foo, size_t, size_t) {}
}

struct Foo {
Bar _bar;
}

struct Bar {
version (Bug)
this(ref Bar) { }
else
this(Bar) { }
}

void main()
{
Foo foo;
Array arr;
arr[] = foo;
}
17 changes: 17 additions & 0 deletions compiler/test/compilable/test21204.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// https://issues.dlang.org/show_bug.cgi?id=21204

struct A
{
this(ref A other) {}
}

struct B
{
A a;
}

void example()
{
B b1;
B b2 = b1;
}
5 changes: 0 additions & 5 deletions compiler/test/compilable/test23097.d
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,6 @@
REQUIRED_ARGS: -verrors=spec
TEST_OUTPUT:
---
(spec:2) compilable/test23097.d(14): Error: `inout` constructor `test23097.S23097.this` creates const object, not mutable
(spec:2) compilable/test23097.d(14): Error: `inout` constructor `test23097.S23097.this` creates const object, not mutable
(spec:1) compilable/test23097.d(14): Error: generated function `test23097.S23097.opAssign(S23097 p)` is not callable using argument types `(const(S23097))`
(spec:2) compilable/test23097.d(14): Error: `inout` constructor `test23097.S23097.this` creates const object, not mutable
(spec:1) compilable/test23097.d(14): `struct S23097` does not define a copy constructor for `const(S23097)` to `S23097` copies
---
*/
void emplaceRef(UT, Args)(UT chunk, Args args)
Expand Down
7 changes: 2 additions & 5 deletions compiler/test/fail_compilation/ice23097.d
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
/* https://issues.dlang.org/show_bug.cgi?id=23097
TEST_OUTPUT:
---
fail_compilation/ice23097.d(13): Error: undefined identifier `ICE`
fail_compilation/ice23097.d(28): Error: template instance `ice23097.ice23097!(S23097)` error instantiating
fail_compilation/ice23097.d(28): Error: function `ice23097` is not callable using argument types `(S23097)`
fail_compilation/ice23097.d(28): generating a copy constructor for `struct S23097` failed, therefore instances of it are uncopyable
fail_compilation/ice23097.d(11): `ice23097.ice23097!(S23097).ice23097(S23097 __param_0)` declared here
fail_compilation/ice23097.d(10): Error: undefined identifier `ICE`
fail_compilation/ice23097.d(25): Error: template instance `ice23097.ice23097!(S23097)` error instantiating
---
*/
auto ice23097(I)(I)
Expand Down
3 changes: 2 additions & 1 deletion compiler/test/fail_compilation/test21198.d
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
/*
TEST_OUTPUT:
---
fail_compilation/test21198.d(23): Error: generating an `inout` copy constructor for `struct test21198.U` failed, therefore instances of it are uncopyable
fail_compilation/test21198.d(24): Error: generated copy constructor of type `inout ref @system inout(U)(return ref scope inout(U) p)` is disabled
fail_compilation/test21198.d(24): some of the field types of struct `U` do not define a copy constructor that can handle such copies
---
*/

Expand Down
23 changes: 0 additions & 23 deletions compiler/test/fail_compilation/test21204.d

This file was deleted.

44 changes: 44 additions & 0 deletions compiler/test/runnable/testCopyCtor.d
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,50 @@ void test5()
assert(result == "A");
}

struct Foo
{
this(ref const Foo _) const
{
result ~= "A";
}

this(ref const Foo _) immutable
{
result ~= "B";
}
}

struct Bar
{
this(ref const Bar _) const
{
result ~= "C";
}

this(ref const Bar _) immutable
{
result ~= "C";
}
}

struct FooBar
{
Foo foo;
Bar bar;
}

void test6()
{
result = "";

FooBar a;
const FooBar b = a;
assert(result == "AC");

immutable FooBar c = a;
assert(result == "ACBD");
}

void main()
{
test1();
Expand Down

0 comments on commit 8fe5dcb

Please sign in to comment.