Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement move constructor #16876

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions compiler/src/dmd/aggregate.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,8 @@ class StructDeclaration : public AggregateDeclaration
bool zeroInit(bool v);
bool hasIdentityAssign() const; // true if has identity opAssign
bool hasIdentityAssign(bool v);
bool hasMoveAssign() const; // true if has identity opAssign
bool hasMoveAssign(bool v);
bool hasBlitAssign() const; // true if opAssign is a blit
bool hasBlitAssign(bool v);
bool hasIdentityEquals() const; // true if has identity opEquals
Expand All @@ -185,6 +187,8 @@ class StructDeclaration : public AggregateDeclaration
bool hasNoFields(bool v);
bool hasCopyCtor() const; // copy constructor
bool hasCopyCtor(bool v);
bool hasMoveCtor() const; // copy constructor
bool hasMoveCtor(bool v);
// Even if struct is defined as non-root symbol, some built-in operations
// (e.g. TypeidExp, NewExp, ArrayLiteralExp, etc) request its TypeInfo.
// For those, today TypeInfo_Struct is generated in COMDAT.
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dmd/backend/debugprint.d
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@
@trusted
void WRfunc(const char* msg, Symbol* sfunc, block* startblock)
{
printf("............%s...%s().............\n", msg, sfunc.Sident.ptr);
printf("............%s...%s()\n", msg, sfunc.Sident.ptr);

Check warning on line 512 in compiler/src/dmd/backend/debugprint.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/backend/debugprint.d#L512

Added line #L512 was not covered by tests
numberBlocks(startblock);
for (block *b = startblock; b; b = b.Bnext)
WRblock(b);
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dmd/backend/dout.d
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,7 @@ private void writefunc2(Symbol *sfunc, ref GlobalOptimizer go)
{
func_t *f = sfunc.Sfunc;

//printf("writefunc(%s)\n",sfunc.Sident.ptr);
debugb && printf("===================== writefunc %s =================\n",sfunc.Sident.ptr);
//symbol_print(sfunc);
debug debugy && printf("writefunc(%s)\n",sfunc.Sident.ptr);

Expand Down
209 changes: 209 additions & 0 deletions compiler/src/dmd/clone.d
Original file line number Diff line number Diff line change
Expand Up @@ -1533,6 +1533,9 @@
return xpostblit;
}

/* ===================================== Copy Constructor ========================== */
static if (1) {

/**
* Generates a copy constructor declaration with the specified storage
* class for the parameter and the function.
Expand Down Expand Up @@ -1736,3 +1739,209 @@
}
return true;
}

}

/* ===================================== Move Constructor ========================== */
static if (1) {

/**
* Generates a move constructor declaration with the specified storage
* class for the parameter and the function.
*
* Params:
* sd = the `struct` that contains the move constructor
* paramStc = the storage class of the move constructor parameter
* funcStc = the storage class for the move constructor declaration
*
* Returns:
* The move constructor declaration for struct `sd`.
*/
private CtorDeclaration generateMoveCtorDeclaration(StructDeclaration sd, const StorageClass paramStc, const StorageClass funcStc)
{
/* Although the move constructor is declared as `this(S s) { ... }`,
* it is implemented as `this(ref S s) { ... }`
*/
return generateCopyCtorDeclaration(sd, paramStc, funcStc);
}

/**
* Generates a trivial move constructor body that simply does memberwise
* initialization:
*
* this.field1 = rhs.field1;
* this.field2 = rhs.field2;
* ...
*
* Params:
* sd = the `struct` declaration that contains the copy constructor
*
* Returns:
* A `CompoundStatement` containing the body of the copy constructor.
*/
private Statement generateMoveCtorBody(StructDeclaration sd)
{
Loc loc;
Expression e;
foreach (v; sd.fields)
{
auto ec = new AssignExp(loc,
new DotVarExp(loc, new ThisExp(loc), v),
new DotVarExp(loc, new IdentifierExp(loc, Id.p), v));
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 move constructor is needed for struct sd,
* if the following conditions are met:
*
* 1. sd does not define a move constructor
* 2. at least one field of sd defines a move constructor
*
* Params:
* sd = the `struct` for which the move constructor is generated
* hasMoveCtor = set to true if a move constructor is already present
*
* Returns:
* `true` if one needs to be generated
* `false` otherwise
*/
bool needMoveCtor(StructDeclaration sd, out bool hasMoveCtor)
{
if (global.errors)
return false;

auto ctor = sd.search(sd.loc, Id.ctor);
if (ctor)
{
if (ctor.isOverloadSet())
return false;
if (auto td = ctor.isTemplateDeclaration())
ctor = td.funcroot;
}

CtorDeclaration moveCtor;
CtorDeclaration rvalueCtor;

if (!ctor)
goto LcheckFields;

overloadApply(ctor, (Dsymbol s)
{
if (s.isTemplateDeclaration())
return 0;
auto ctorDecl = s.isCtorDeclaration();
assert(ctorDecl);
if (ctorDecl.isMoveCtor)
{
if (!moveCtor)
moveCtor = ctorDecl;
return 0;
}

if (isRvalueConstructor(sd, ctorDecl))
rvalueCtor = ctorDecl;

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

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/clone.d#L1847

Added line #L1847 was not covered by tests
return 0;
});

if (moveCtor)
{
if (rvalueCtor)
{
.error(sd.loc, "`struct %s` may not define both a rvalue constructor and a move constructor", sd.toChars());
errorSupplemental(rvalueCtor.loc,"rvalue constructor defined here");
errorSupplemental(moveCtor.loc, "move constructor defined here");

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

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/clone.d#L1855-L1857

Added lines #L1855 - L1857 were not covered by tests
}
hasMoveCtor = true;
return false;
}

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

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

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/clone.d#L1869

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

auto ts = v.type.baseElemOf().isTypeStruct();
if (!ts)
continue;
if (ts.sym.hasMoveCtor)
{
fieldWithMoveCtor = v;
break;
}
}

if (fieldWithMoveCtor && rvalueCtor)
{
.error(sd.loc, "`struct %s` may not define a rvalue constructor and have fields with move constructors", sd.toChars());
errorSupplemental(rvalueCtor.loc,"rvalue constructor defined here");
errorSupplemental(fieldWithMoveCtor.loc, "field with move constructor defined here");
return false;

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

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/clone.d#L1885-L1888

Added lines #L1885 - L1888 were not covered by tests
}
else if (!fieldWithMoveCtor)
return false;
return true;
}

/**
* Generates a move constructor if needMoveCtor() returns true.
* The generated move constructor will be of the form:
* this(ref return scope inout(S) rhs) inout
* {
* this.field1 = rhs.field1;
* this.field2 = rhs.field2;
* ...
* }
*
* Params:
* sd = the `struct` for which the copy constructor is generated
* sc = the scope where the copy constructor is generated
*
* 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 buildMoveCtor(StructDeclaration sd, Scope* sc)
{
//printf("buildMoveCtor() %s\n", sd.toChars());
bool hasMoveCtor;
if (!needMoveCtor(sd, hasMoveCtor))
return hasMoveCtor;

//printf("generating move constructor for %s\n", sd.toChars());
const MOD paramMod = MODFlags.wild;
const MOD funcMod = MODFlags.wild;
auto ccd = generateMoveCtorDeclaration(sd, ModToStc(paramMod), ModToStc(funcMod));
auto moveCtorBody = generateMoveCtorBody(sd);
ccd.fbody = moveCtorBody;
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;
}

}
10 changes: 8 additions & 2 deletions compiler/src/dmd/dstruct.d
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,11 @@ extern (C++) class StructDeclaration : AggregateDeclaration
bool zeroInit; // !=0 if initialize with 0 fill
bool hasIdentityAssign; // true if has identity opAssign
bool hasBlitAssign; // true if opAssign is a blit
bool hasMoveAssign; // true if move assignment
bool hasIdentityEquals; // true if has identity opEquals
bool hasNoFields; // has no fields
bool hasCopyCtor; // copy constructor
bool hasMoveCtor; // move constructor
bool hasPointerField; // members with indirections
bool hasVoidInitPointers; // void-initialized unsafe fields
bool hasUnsafeBitpatterns; // @system members, pointers, bool
Expand Down Expand Up @@ -417,7 +419,7 @@ extern (C++) class StructDeclaration : AggregateDeclaration
* POD is defined as:
* $(OL
* $(LI not nested)
* $(LI no postblits, destructors, or assignment operators)
* $(LI no postblits, copy constructors, move constructors, destructors, or assignment operators)
* $(LI no `ref` fields or fields that are themselves non-POD)
* )
* The idea being these are compatible with C structs.
Expand All @@ -436,10 +438,14 @@ extern (C++) class StructDeclaration : AggregateDeclaration
bool hasCpCtorLocal;
needCopyCtor(this, hasCpCtorLocal);

bool hasMoveCtorLocal;
needMoveCtor(this, hasMoveCtorLocal);

if (enclosing || // is nested
search(this, loc, Id.postblit) || // has postblit
search(this, loc, Id.dtor) || // has destructor
hasCpCtorLocal) // has copy constructor
hasCpCtorLocal || // has copy constructor
hasMoveCtorLocal) // has move constructor
{
ispod = ThreeState.no;
return false;
Expand Down
20 changes: 15 additions & 5 deletions compiler/src/dmd/dsymbolsem.d
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ bool isRvalueConstructor(StructDeclaration sd, CtorDeclaration ctor)
{
auto tf = ctor.type.isTypeFunction();
const dim = tf.parameterList.length;
if (dim == 1 || (dim > 1 && tf.parameterList[1].defaultArg))
if (dim > 1 && tf.parameterList[1].defaultArg)
{
auto param = tf.parameterList[0];
if (!(param.storageClass & STC.ref_) && param.type.mutableOf().unSharedOf() == sd.type.mutableOf().unSharedOf())
Expand Down Expand Up @@ -554,6 +554,7 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor

override void visit(VarDeclaration dsym)
{
//printf("VarDeclaration %s\n", dsym.toChars());
version (none)
{
printf("VarDeclaration::semantic('%s', parent = '%s') sem = %d\n",
Expand Down Expand Up @@ -2480,14 +2481,22 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor
.error(ctd.loc, "%s `%s` all parameters have default arguments, "~
"but structs cannot have default constructors.", ctd.kind, ctd.toPrettyChars);
}
else if ((dim == 1 || (dim > 1 && tf.parameterList[1].defaultArg)))
else if (dim == 1 || (dim > 1 && tf.parameterList[1].defaultArg))
{
//printf("tf: %s\n", tf.toChars());
auto param = tf.parameterList[0];
if (param.storageClass & STC.ref_ && param.type.mutableOf().unSharedOf() == sd.type.mutableOf().unSharedOf())
if (param.type.mutableOf().unSharedOf() == sd.type.mutableOf().unSharedOf())
{
//printf("copy constructor\n");
ctd.isCpCtor = true;
if (param.storageClass & STC.ref_)
{
//printf("found copy constructor\n");
ctd.isCpCtor = true;
}
else
{
//printf("found move constructor\n");
ctd.isMoveCtor = true;
}
}
}
}
Expand Down Expand Up @@ -2979,6 +2988,7 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor
buildDtors(sd, sc2);

sd.hasCopyCtor = buildCopyCtor(sd, sc2);
sd.hasMoveCtor = buildMoveCtor(sd, sc2);
sd.postblit = buildPostBlit(sd, sc2);

buildOpAssign(sd, sc2);
Expand Down
6 changes: 6 additions & 0 deletions compiler/src/dmd/e2ir.d
Original file line number Diff line number Diff line change
Expand Up @@ -5509,6 +5509,12 @@ elem *callfunc(const ref Loc loc,
bool copy = !(v && v.isArgDtorVar); // copy unless the destructor is going to be run on it
// then assume the frontend took care of the copying and pass it by ref

if (ea.Eoper == OPind && ea.E1.Eoper == OPcall && arg.type.isTypeStruct())
{
if (auto ctor = fd.isCtorDeclaration())
copy = !ctor.isMoveCtor;
}

elems[i] = addressElem(ea, arg.type, copy);
continue;
}
Expand Down
Loading
Loading