Skip to content

Commit

Permalink
build default move constructors
Browse files Browse the repository at this point in the history
  • Loading branch information
WalterBright committed Jan 10, 2025
1 parent 45e4a09 commit f7e6f7d
Show file tree
Hide file tree
Showing 9 changed files with 214 additions and 109 deletions.
195 changes: 113 additions & 82 deletions compiler/src/dmd/clone.d
Original file line number Diff line number Diff line change
Expand Up @@ -1545,98 +1545,103 @@ 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;
return ccd;
}

/**
* 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());
}
Statement s1 = new ExpStatement(loc, e);
return new CompoundStatement(loc, s1);
}

/**
* Determine if a copy constructor is needed for struct sd,
* if the following conditions are met:
*
* 1. sd does not define a copy constructor
* 2. at least one field of sd defines a copy constructor
*
/******************************************
* Find root `this` constructor for struct sd.
* (root is starting position for overloaded constructors)
* Params:
* sd = the `struct` for which the copy constructor is generated
* hasCpCtor = set to true if a copy constructor is already present
* hasMoveCtor = set to true if a move constructor is already present
*
* Returns:
* `true` if one needs to be generated
* `false` otherwise
* sd = the `struct` to be searched
* ctor = `this` if found, otherwise null
* Result:
* false means `this` found in overload set
*/
bool needCopyCtor(StructDeclaration sd, out bool hasCpCtor, out bool hasMoveCtor)
private bool findStructConstructorRoot(StructDeclaration sd, out Dsymbol ctor)
{
//printf("needCopyCtor() %s\n", sd.toChars());
if (global.errors)
return false;

auto ctor = sd.search(sd.loc, Id.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;
}

CtorDeclaration cpCtor;
CtorDeclaration rvalueCtor;

if (!ctor)
goto LcheckFields;

/***********************************************
* 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())
Expand All @@ -1645,33 +1650,63 @@ bool needCopyCtor(StructDeclaration sd, out bool hasCpCtor, out bool hasMoveCtor
assert(ctorDecl);
if (ctorDecl.isCpCtor)
{
if (!cpCtor)
cpCtor = ctorDecl;
return 0;
if (!copyCtor)
copyCtor = ctorDecl;
}
else if (ctorDecl.isMoveCtor)
{
if (!moveCtor)
moveCtor = ctorDecl;
}

if (ctorDecl.isMoveCtor)
rvalueCtor = ctorDecl;
return 0;
});
}

/**
* Determine if a copy constructor is needed for struct sd,
* if the following conditions are met:
*
* 1. sd does not define a copy constructor
* 2. at least one field of sd defines a copy constructor
*
* Params:
* sd = the `struct` for which the copy constructor is generated
* 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
*/
void needCopyOrMoveCtor(StructDeclaration sd, out bool hasCopyCtor, out bool hasMoveCtor, out bool needCopyCtor, out bool needMoveCtor)
{
//printf("needCopyOrMoveCtor() %s\n", sd.toChars());
if (global.errors)
return;

Dsymbol ctor;
if (!findStructConstructorRoot(sd, ctor))
return;

if (rvalueCtor)
CtorDeclaration copyCtor;
CtorDeclaration moveCtor;

if (ctor)
findMoveAndCopyConstructors(ctor, copyCtor, moveCtor);

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)
{
Expand All @@ -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");

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

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/clone.d#L1734

Added line #L1734 was not covered by tests
errorSupplemental(fieldWithCpCtor.loc, "field with copy constructor defined here");
return false;
return;

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

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/clone.d#L1736

Added line #L1736 was not covered by tests
}
else if (!fieldWithCpCtor)
return false;
return true;

if (fieldWithCpCtor && !hasCopyCtor)
needCopyCtor = true;
if (fieldWithMoveCtor && !hasMoveCtor)
needMoveCtor = true;
}

/**
Expand All @@ -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();
Expand All @@ -1751,5 +1783,4 @@ bool buildCopyCtor(StructDeclaration sd, Scope* sc, out bool hasMoveCtor)
ccd.storage_class |= STC.disable;
ccd.fbody = null;
}
return true;
}
4 changes: 3 additions & 1 deletion compiler/src/dmd/dstruct.d
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 29 additions & 1 deletion compiler/src/dmd/dsymbolsem.d
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
}
Expand Down Expand Up @@ -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;

Check warning on line 3024 in compiler/src/dmd/dsymbolsem.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/dsymbolsem.d#L3024

Added line #L3024 was not covered by tests
}

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);
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dmd/expressionsem.d
Original file line number Diff line number Diff line change
Expand Up @@ -6164,7 +6164,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;
}

Expand Down Expand Up @@ -17042,6 +17042,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());
Expand Down
4 changes: 1 addition & 3 deletions compiler/src/dmd/func.d
Original file line number Diff line number Diff line change
Expand Up @@ -1372,11 +1372,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);
}

Expand Down
Loading

0 comments on commit f7e6f7d

Please sign in to comment.