diff --git a/compiler/src/dmd/clone.d b/compiler/src/dmd/clone.d index 1b838603c7..c37ae613a3 100644 --- a/compiler/src/dmd/clone.d +++ b/compiler/src/dmd/clone.d @@ -1545,25 +1545,27 @@ FuncDeclaration buildPostBlit(StructDeclaration sd, Scope* sc) } /** - * Generates a copy constructor declaration with the specified storage + * Generates a copy or move constructor declaration with the specified storage * class for the parameter and the function. * * Params: - * sd = the `struct` that contains the copy constructor - * paramStc = the storage class of the copy constructor parameter - * funcStc = the storage class for the copy constructor declaration + * sd = the `struct` that contains the constructor + * paramStc = the storage class of the constructor parameter + * funcStc = the storage class for the constructor declaration + * move = true for move constructor, false for copy constructor * * Returns: * The copy constructor declaration for struct `sd`. */ -private CtorDeclaration generateCopyCtorDeclaration(StructDeclaration sd, const StorageClass paramStc, const StorageClass funcStc) +private CtorDeclaration generateCtorDeclaration(StructDeclaration sd, const StorageClass paramStc, const StorageClass funcStc, bool move) { auto fparams = new Parameters(); auto structType = sd.type; - fparams.push(new Parameter(Loc.initial, paramStc | STC.ref_ | STC.return_ | STC.scope_, structType, Id.p, null, null)); + StorageClass stc = move ? 0 : STC.ref_; // the only difference between copy or move + fparams.push(new Parameter(Loc.initial, paramStc | stc | STC.return_ | STC.scope_, structType, Id.p, null, null)); ParameterList pList = ParameterList(fparams); auto tf = new TypeFunction(pList, structType, LINK.d, STC.ref_); - auto ccd = new CtorDeclaration(sd.loc, Loc.initial, STC.ref_, tf, true); + auto ccd = new CtorDeclaration(sd.loc, Loc.initial, STC.ref_, tf); ccd.storage_class |= funcStc; ccd.storage_class |= STC.inference; ccd.isGenerated = true; @@ -1571,28 +1573,37 @@ private CtorDeclaration generateCopyCtorDeclaration(StructDeclaration sd, const } /** - * Generates a trivial copy constructor body that simply does memberwise - * initialization: + * Generates a trivial copy or move constructor body that simply does memberwise + * initialization. * + * for copy construction: * this.field1 = rhs.field1; * this.field2 = rhs.field2; * ... + * for move construction: + * this.field1 = __rvalue(rhs.field1); + * this.field2 = __rvalue(rhs.field2); + * ... * * Params: - * sd = the `struct` declaration that contains the copy constructor + * sd = the `struct` declaration that contains the constructor + * move = true for move constructor, false for copy constructor * * Returns: - * A `CompoundStatement` containing the body of the copy constructor. + * A `CompoundStatement` containing the body of the constructor. */ -private Statement generateCopyCtorBody(StructDeclaration sd) +private Statement generateCtorBody(StructDeclaration sd, bool move) { Loc loc; Expression e; foreach (v; sd.fields) { + Expression rhs = new DotVarExp(loc, new IdentifierExp(loc, Id.p), v); + if (move) + rhs.rvalue = true; auto ec = new AssignExp(loc, new DotVarExp(loc, new ThisExp(loc), v), - new DotVarExp(loc, new IdentifierExp(loc, Id.p), v)); + rhs); e = Expression.combine(e, ec); //printf("e.toChars = %s\n", e.toChars()); } @@ -1600,6 +1611,57 @@ private Statement generateCopyCtorBody(StructDeclaration sd) return new CompoundStatement(loc, s1); } +/****************************************** + * Find root `this` constructor for struct sd. + * (root is starting position for overloaded constructors) + * Params: + * sd = the `struct` to be searched + * ctor = `this` if found, otherwise null + * Result: + * false means `this` found in overload set + */ +private bool findStructConstructorRoot(StructDeclaration sd, out Dsymbol ctor) +{ + ctor = sd.search(sd.loc, Id.ctor); // Aggregate.searchCtor() ? + if (ctor) + { + if (ctor.isOverloadSet()) + return false; + if (auto td = ctor.isTemplateDeclaration()) + ctor = td.funcroot; + } + return true; +} + +/*********************************************** + * Find move and copy constructors (if any) starting at `ctor` + * Params: + * ctor = `this` constructor root + * copyCtor = set to first copy constructor found, or null + * moveCtor = set to first move constructor found, or null + */ +private void findMoveAndCopyConstructors(Dsymbol ctor, out CtorDeclaration copyCtor, out CtorDeclaration moveCtor) +{ + overloadApply(ctor, (Dsymbol s) + { + if (s.isTemplateDeclaration()) + return 0; + auto ctorDecl = s.isCtorDeclaration(); + assert(ctorDecl); + if (ctorDecl.isCpCtor) + { + if (!copyCtor) + copyCtor = ctorDecl; + } + else if (ctorDecl.isMoveCtor) + { + if (!moveCtor) + moveCtor = ctorDecl; + } + return 0; + }); +} + /** * Determine if a copy constructor is needed for struct sd, * if the following conditions are met: @@ -1609,69 +1671,42 @@ private Statement generateCopyCtorBody(StructDeclaration sd) * * Params: * sd = the `struct` for which the copy constructor is generated - * hasCpCtor = set to true if a copy constructor is already present + * hasCopyCtor = set to true if a copy constructor is already present * hasMoveCtor = set to true if a move constructor is already present + * needCopyCtor = set to true if a copy constructor is not present, but needed + * needMoveCtor = set to true if a move constructor is not present, but needed * * Returns: * `true` if one needs to be generated * `false` otherwise */ -bool needCopyCtor(StructDeclaration sd, out bool hasCpCtor, out bool hasMoveCtor) +void needCopyOrMoveCtor(StructDeclaration sd, out bool hasCopyCtor, out bool hasMoveCtor, out bool needCopyCtor, out bool needMoveCtor) { - //printf("needCopyCtor() %s\n", sd.toChars()); + //printf("needCopyOrMoveCtor() %s\n", sd.toChars()); if (global.errors) - return false; + return; + + Dsymbol ctor; + if (!findStructConstructorRoot(sd, ctor)) + return; + + CtorDeclaration copyCtor; + CtorDeclaration moveCtor; - auto ctor = sd.search(sd.loc, Id.ctor); if (ctor) - { - if (ctor.isOverloadSet()) - return false; - if (auto td = ctor.isTemplateDeclaration()) - ctor = td.funcroot; - } + findMoveAndCopyConstructors(ctor, copyCtor, moveCtor); - CtorDeclaration cpCtor; - CtorDeclaration rvalueCtor; - - if (!ctor) - goto LcheckFields; - - overloadApply(ctor, (Dsymbol s) - { - if (s.isTemplateDeclaration()) - return 0; - auto ctorDecl = s.isCtorDeclaration(); - assert(ctorDecl); - if (ctorDecl.isCpCtor) - { - if (!cpCtor) - cpCtor = ctorDecl; - return 0; - } - - if (ctorDecl.isMoveCtor) - rvalueCtor = ctorDecl; - return 0; - }); - - if (rvalueCtor) + if (moveCtor) hasMoveCtor = true; - if (cpCtor) - { - if (0 && rvalueCtor) - { - .error(sd.loc, "`struct %s` may not define both a rvalue constructor and a copy constructor", sd.toChars()); - errorSupplemental(rvalueCtor.loc,"rvalue constructor defined here"); - errorSupplemental(cpCtor.loc, "copy constructor defined here"); - } - hasCpCtor = true; - return false; - } + if (copyCtor) + hasCopyCtor = true; + + if (hasMoveCtor && hasCopyCtor) + return; -LcheckFields: VarDeclaration fieldWithCpCtor; + VarDeclaration fieldWithMoveCtor; // see if any struct members define a copy constructor foreach (v; sd.fields) { @@ -1686,20 +1721,25 @@ LcheckFields: if (ts.sym.hasCopyCtor) { fieldWithCpCtor = v; - break; + } + if (ts.sym.hasMoveCtor) + { + fieldWithMoveCtor = v; } } - if (fieldWithCpCtor && rvalueCtor) + if (0 && fieldWithCpCtor && moveCtor) { .error(sd.loc, "`struct %s` may not define a rvalue constructor and have fields with copy constructors", sd.toChars()); - errorSupplemental(rvalueCtor.loc,"rvalue constructor defined here"); + errorSupplemental(moveCtor.loc,"rvalue constructor defined here"); errorSupplemental(fieldWithCpCtor.loc, "field with copy constructor defined here"); - return false; + return; } - else if (!fieldWithCpCtor) - return false; - return true; + + if (fieldWithCpCtor && !hasCopyCtor) + needCopyCtor = true; + if (fieldWithMoveCtor && !hasMoveCtor) + needMoveCtor = true; } /** @@ -1713,28 +1753,20 @@ LcheckFields: * } * * Params: - * sd = the `struct` for which the copy constructor is generated - * sc = the scope where the copy constructor is generated - * hasMoveCtor = set to true when a move constructor is also detected - * - * Returns: - * `true` if `struct` sd defines a copy constructor (explicitly or generated), - * `false` otherwise. + * sd = the `struct` for which the constructor is generated + * sc = the scope where the constructor is generated + * move = true means generate the move constructor, otherwise copy constructor * References: * https://dlang.org/spec/struct.html#struct-copy-constructor */ -bool buildCopyCtor(StructDeclaration sd, Scope* sc, out bool hasMoveCtor) +void buildCopyOrMoveCtor(StructDeclaration sd, Scope* sc, bool move) { - bool hasCpCtor; - if (!needCopyCtor(sd, hasCpCtor, hasMoveCtor)) - return hasCpCtor; - - //printf("generating copy constructor for %s\n", sd.toChars()); + //printf("buildCopyOrMoveCtor() generating %s constructor for %s\n", move ? "move".ptr : "copy".ptr, 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; + auto ccd = generateCtorDeclaration(sd, ModToStc(paramMod), ModToStc(funcMod), move); + auto ctorBody = generateCtorBody(sd, move); + ccd.fbody = ctorBody; sd.members.push(ccd); ccd.addMember(sc, sd); const errors = global.startGagging(); @@ -1751,5 +1783,4 @@ bool buildCopyCtor(StructDeclaration sd, Scope* sc, out bool hasMoveCtor) ccd.storage_class |= STC.disable; ccd.fbody = null; } - return true; } diff --git a/compiler/src/dmd/dstruct.d b/compiler/src/dmd/dstruct.d index abeffcb39d..9a9058f61d 100644 --- a/compiler/src/dmd/dstruct.d +++ b/compiler/src/dmd/dstruct.d @@ -323,7 +323,9 @@ extern (C++) class StructDeclaration : AggregateDeclaration bool hasCpCtorLocal; bool hasMoveCtorLocal; - needCopyCtor(this, hasCpCtorLocal, hasMoveCtorLocal); + bool needCopyCtor; + bool needMoveCtor; + needCopyOrMoveCtor(this, hasCpCtorLocal, hasMoveCtorLocal, needCopyCtor, needMoveCtor); if (enclosing || // is nested search(this, loc, Id.postblit) || // has postblit diff --git a/compiler/src/dmd/dsymbolsem.d b/compiler/src/dmd/dsymbolsem.d index 1ab646f413..e021a79f18 100644 --- a/compiler/src/dmd/dsymbolsem.d +++ b/compiler/src/dmd/dsymbolsem.d @@ -2513,10 +2513,12 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor if (param.type.mutableOf().unSharedOf() == sd.type.mutableOf().unSharedOf()) { //printf("copy constructor %p\n", ctd); + assert(!ctd.isCpCtor && !ctd.isMoveCtor); if (param.storageClass & STC.ref_) ctd.isCpCtor = true; // copy constructor else ctd.isMoveCtor = true; // move constructor + assert(!(ctd.isCpCtor && ctd.isMoveCtor)); } } } @@ -3007,8 +3009,34 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor buildDtors(sd, sc2); + bool hasCopyCtor; bool hasMoveCtor; - sd.hasCopyCtor = buildCopyCtor(sd, sc2, hasMoveCtor); + bool needCopyCtor; + bool needMoveCtor; + needCopyOrMoveCtor(sd, hasCopyCtor, hasMoveCtor, needCopyCtor, needMoveCtor); + //printf("%s hasCopy %d hasMove %d needCopy %d needMove %d\n", sd.toChars(), hasCopyCtor, hasMoveCtor, needCopyCtor, needMoveCtor); + + /* When generating a move ctor, generate a copy ctor too, otherwise + * https://github.com/s-ludwig/taggedalgebraic/issues/75 + */ + if (0 && needMoveCtor && !hasCopyCtor) + { + needCopyCtor = true; + } + + if (needCopyCtor) + { + assert(hasCopyCtor == false); + buildCopyOrMoveCtor(sd, sc2, false); // build copy constructor + hasCopyCtor = true; + } + if (needMoveCtor) + { + assert(hasMoveCtor == false); + buildCopyOrMoveCtor(sd, sc2, true); // build move constructor + hasMoveCtor = true; + } + sd.hasCopyCtor = hasCopyCtor; sd.hasMoveCtor = hasMoveCtor; sd.postblit = buildPostBlit(sd, sc2); diff --git a/compiler/src/dmd/expressionsem.d b/compiler/src/dmd/expressionsem.d index 27b9704e42..cd7548ea27 100644 --- a/compiler/src/dmd/expressionsem.d +++ b/compiler/src/dmd/expressionsem.d @@ -6162,7 +6162,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor if (sd.ctor) { auto ctor = sd.ctor.isCtorDeclaration(); - if (ctor && ctor.isCpCtor && ctor.isGenerated()) + if (ctor && (ctor.isCpCtor || ctor.isMoveCtor) && ctor.isGenerated()) sd.ctor = null; } @@ -17035,6 +17035,7 @@ bool checkDisabled(Declaration d, Loc loc, Scope* sc, bool isAliasedDeclaration if (auto ctor = d.isCtorDeclaration()) { + //printf("checkDisabled() %s %s\n", ctor.toPrettyChars(), toChars(ctor.type)); if (ctor.isCpCtor && ctor.isGenerated()) { .error(loc, "generating an `inout` copy constructor for `struct %s` failed, therefore instances of it are uncopyable", d.parent.toPrettyChars()); diff --git a/compiler/src/dmd/func.d b/compiler/src/dmd/func.d index 40d39aea0e..b42ac768b4 100644 --- a/compiler/src/dmd/func.d +++ b/compiler/src/dmd/func.d @@ -1373,11 +1373,9 @@ extern (C++) final class CtorDeclaration : FuncDeclaration { bool isCpCtor; // copy constructor bool isMoveCtor; // move constructor (aka rvalue constructor) - extern (D) this(const ref Loc loc, const ref Loc endloc, StorageClass stc, Type type, bool isCpCtor = false, bool isMoveCtor = false) + extern (D) this(const ref Loc loc, const ref Loc endloc, StorageClass stc, Type type) { super(loc, endloc, Id.ctor, stc, type); - this.isCpCtor = isCpCtor; - this.isMoveCtor = isMoveCtor; //printf("CtorDeclaration(loc = %s) %s %p\n", loc.toChars(), toChars(), this); } diff --git a/compiler/src/dmd/funcsem.d b/compiler/src/dmd/funcsem.d index 39da0256ee..40f964977d 100644 --- a/compiler/src/dmd/funcsem.d +++ b/compiler/src/dmd/funcsem.d @@ -1536,7 +1536,7 @@ FuncDeclaration resolveFuncCall(const ref Loc loc, Scope* sc, Dsymbol s, version (none) { - printf("resolveFuncCall('%s')\n", s.toChars()); + printf("resolveFuncCall() %s)\n", s.toChars()); if (tthis) printf("\tthis: %s\n", tthis.toChars()); if (fargs) @@ -1548,7 +1548,6 @@ FuncDeclaration resolveFuncCall(const ref Loc loc, Scope* sc, Dsymbol s, printf("\t%s: %s\n", arg.toChars(), arg.type.toChars()); } } - printf("\tfnames: %s\n", fnames ? fnames.toChars() : "null"); } if (tiargs && arrayObjectIsError(*tiargs)) diff --git a/compiler/test/compilable/copyCtor2.d b/compiler/test/compilable/copyCtor2.d new file mode 100644 index 0000000000..084ef3b3f2 --- /dev/null +++ b/compiler/test/compilable/copyCtor2.d @@ -0,0 +1,14 @@ +/* This used to not be allowed + * https://github.com/dlang/dmd/pull/20634 + */ + +struct A +{ + this (ref shared A a) immutable {} +} + +struct B +{ + A a; + this(immutable B b) shared {} +} diff --git a/compiler/test/fail_compilation/failCopyCtor2.d b/compiler/test/fail_compilation/failCopyCtor2.d deleted file mode 100644 index 5e8f8c40f7..0000000000 --- a/compiler/test/fail_compilation/failCopyCtor2.d +++ /dev/null @@ -1,19 +0,0 @@ -/* -TEST_OUTPUT: ---- -fail_compilation/failCopyCtor2.d(15): Error: `struct B` may not define a rvalue constructor and have fields with copy constructors -fail_compilation/failCopyCtor2.d(18): rvalue constructor defined here -fail_compilation/failCopyCtor2.d(17): field with copy constructor defined here ---- -*/ - -struct A -{ - this (ref shared A a) immutable {} -} - -struct B -{ - A a; - this(immutable B b) shared {} -} diff --git a/compiler/test/runnable/rvalue1.d b/compiler/test/runnable/rvalue1.d index f8134f718a..2592f2a9e9 100644 --- a/compiler/test/runnable/rvalue1.d +++ b/compiler/test/runnable/rvalue1.d @@ -194,6 +194,56 @@ void test8() /********************************/ +struct T9 +{ + int i; + inout this(ref inout T9 t) { this.i = t.i - 1; printf("this(ref T9)\n"); } + inout this(inout T9 t) { this.i = t.i + 1; printf("this(T9)\n"); } +} + +struct S9 +{ + T9 t; + //inout this(return ref scope inout S9 t);// { this.i = t.i - 1; printf("this(ref T9)\n"); } + //@system inout this(return scope inout S9 t);// { this.i = t.i + 1; printf("this(T9)\n"); } +} + +void test9() +{ + S9 s; + s.t.i = 3; + S9 u = s; + printf("u.t.i = %d\n", u.t.i); + assert(u.t.i == 2); + + S9 v = __rvalue(u); + printf("v.t.i = %d\n", v.t.i); + assert(v.t.i == 3); +} + +/********************************/ +// https://github.com/s-ludwig/taggedalgebraic/issues/75 + +struct T10 +{ + string s; + this(T10) {} + this(string v) { s = v; } +} + +struct S10 +{ + T10 p; +} + +void test10() +{ + S10 s = S10(T10("hello")); + assert(s.p.s == "hello"); +} + +/********************************/ + int main() { test1(); @@ -204,6 +254,7 @@ int main() test6(); test7(); test8(); + test9(); return 0; }