build default move constructors (#20634)

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
+ *  sd = the `struct` that contains the constructor
- *  paramStc = the storage class of the copy constructor parameter
+ *  paramStc = the storage class of the constructor parameter
- *  funcStc = the storage class for the copy constructor declaration
+ *  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
+ * Generates a trivial copy or move constructor body that simply does memberwise
- * initialization:
+ * 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)
-    {
+        findMoveAndCopyConstructors(ctor, copyCtor, moveCtor);
-        if (ctor.isOverloadSet())
-            return false;
-        if (auto td = ctor.isTemplateDeclaration())
-            ctor = td.funcroot;
-    }
 
-    CtorDeclaration cpCtor;
+    if (moveCtor)
-    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)
         hasMoveCtor = true;
 
-    if (cpCtor)
+    if (copyCtor)
-    {
+        hasCopyCtor = true;
-        if (0 && rvalueCtor)
+
-        {
+    if (hasMoveCtor && hasCopyCtor)
-            .error(sd.loc, "`struct %s` may not define both a rvalue constructor and a copy constructor", sd.toChars());
+	return;
-            errorSupplemental(rvalueCtor.loc,"rvalue constructor defined here");
-            errorSupplemental(cpCtor.loc, "copy constructor defined here");
-        }
-        hasCpCtor = true;
-        return false;
-    }
 
-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;
+    if (fieldWithCpCtor && !hasCopyCtor)
-    return true;
+	needCopyCtor = true;
+    if (fieldWithMoveCtor && !hasMoveCtor)
+	needMoveCtor = true;
 }
 
 /**
@@ -1713,28 +1753,20 @@ LcheckFields:
  *   }
  *
  * Params:
- *  sd = the `struct` for which the copy constructor is generated
+ *  sd = the `struct` for which the constructor is generated
- *  sc = the scope where the copy constructor is generated
+ *  sc = the scope where the constructor is generated
- *  hasMoveCtor = set to true when a move constructor is also detected
+ *  move = true means generate the move constructor, otherwise copy constructor
- *
- * Returns:
- *  `true` if `struct` sd defines a copy constructor (explicitly or generated),
- *  `false` otherwise.
  * 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;
+    //printf("buildCopyOrMoveCtor() generating %s constructor for %s\n", move ? "move".ptr : "copy".ptr, sd.toChars());
-    if (!needCopyCtor(sd, hasCpCtor, hasMoveCtor))
-        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 ccd = generateCtorDeclaration(sd, ModToStc(paramMod), ModToStc(funcMod), move);
-    auto copyCtorBody = generateCopyCtorBody(sd);
+    auto ctorBody = generateCtorBody(sd, move);
-    ccd.fbody = copyCtorBody;
+    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;
 }

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

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);

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());

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);
     }
 

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))

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 {}
+}

compiler/test/fail_compilation/failCopyCtor2.d

@@ -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 {}
-}

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;
 }