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

2nd try at __rvalue(expression) #17494

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
2 changes: 2 additions & 0 deletions compiler/src/dmd/aggregate.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,8 @@ class StructDeclaration : public AggregateDeclaration
bool hasNoFields(bool v);
bool hasCopyCtor() const; // copy constructor
bool hasCopyCtor(bool v);
bool hasMoveCtor() const; // move 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
1 change: 1 addition & 0 deletions compiler/src/dmd/astbase.d
Original file line number Diff line number Diff line change
Expand Up @@ -4548,6 +4548,7 @@ struct ASTBase
EXP op;
ubyte size;
ubyte parens;
ubyte rvalue; // consider this an rvalue, even if it is an lvalue
Type type;
Loc loc;

Expand Down
16 changes: 11 additions & 5 deletions compiler/src/dmd/clone.d
Original file line number Diff line number Diff line change
Expand Up @@ -1610,13 +1610,15 @@ 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
* hasMoveCtor = set to true if a move constructor is already present
*
* Returns:
* `true` if one needs to be generated
* `false` otherwise
*/
bool needCopyCtor(StructDeclaration sd, out bool hasCpCtor)
bool needCopyCtor(StructDeclaration sd, out bool hasCpCtor, out bool hasMoveCtor)
{
//printf("needCopyCtor() %s\n", sd.toChars());
if (global.errors)
return false;

Expand Down Expand Up @@ -1648,14 +1650,17 @@ bool needCopyCtor(StructDeclaration sd, out bool hasCpCtor)
return 0;
}

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

if (rvalueCtor)
hasMoveCtor = true;

if (cpCtor)
{
if (rvalueCtor)
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");
Expand Down Expand Up @@ -1710,17 +1715,18 @@ 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.
* References:
* https://dlang.org/spec/struct.html#struct-copy-constructor
*/
bool buildCopyCtor(StructDeclaration sd, Scope* sc)
bool buildCopyCtor(StructDeclaration sd, Scope* sc, out bool hasMoveCtor)
{
bool hasCpCtor;
if (!needCopyCtor(sd, hasCpCtor))
if (!needCopyCtor(sd, hasCpCtor, hasMoveCtor))
return hasCpCtor;

//printf("generating copy constructor for %s\n", sd.toChars());
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dmd/declaration.h
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,7 @@ class CtorDeclaration final : public FuncDeclaration
{
public:
d_bool isCpCtor;
d_bool isMoveCtor;
CtorDeclaration *syntaxCopy(Dsymbol *) override;
const char *kind() const override;
const char *toChars() const override;
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dmd/dscope.d
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import dmd.dclass;
import dmd.declaration;
import dmd.dmodule;
import dmd.doc;
import dmd.dstruct;
import dmd.dsymbol;
import dmd.dsymbolsem;
import dmd.dtemplate;
Expand Down Expand Up @@ -146,6 +147,7 @@ extern (C++) struct Scope

AliasDeclaration aliasAsg; /// if set, then aliasAsg is being assigned a new value,
/// do not set wasRead for it
StructDeclaration argStruct; /// elimiate recursion when looking for rvalue construction

extern (D) __gshared Scope* freelist;

Expand Down
5 changes: 4 additions & 1 deletion compiler/src/dmd/dstruct.d
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ extern (C++) class StructDeclaration : AggregateDeclaration
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 @@ -320,11 +321,13 @@ extern (C++) class StructDeclaration : AggregateDeclaration
import dmd.clone;

bool hasCpCtorLocal;
needCopyCtor(this, hasCpCtorLocal);
bool hasMoveCtorLocal;
needCopyCtor(this, hasCpCtorLocal, hasMoveCtorLocal);

if (enclosing || // is nested
search(this, loc, Id.postblit) || // has postblit
search(this, loc, Id.dtor) || // has destructor
// hasMoveCtorLocal || // has move constructor
hasCpCtorLocal) // has copy constructor
{
ispod = ThreeState.no;
Expand Down
22 changes: 16 additions & 6 deletions compiler/src/dmd/dsymbolsem.d
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,9 @@
*/
bool checkHasBothRvalueAndCpCtor(StructDeclaration sd, CtorDeclaration ctor, TemplateInstance ti)
{
/* cannot use ctor.isMoveCtor because semantic pass may not have been run yet,
* so use isRvalueConstructor()
*/
if (sd && sd.hasCopyCtor && isRvalueConstructor(sd, ctor))
{
.error(ctor.loc, "cannot define both an rvalue constructor and a copy constructor for `struct %s`", sd.toChars());
Expand All @@ -280,6 +283,7 @@
*/
bool isRvalueConstructor(StructDeclaration sd, CtorDeclaration ctor)
{
// note commonality with setting isMoveCtor in the semantic code for CtorDeclaration
auto tf = ctor.type.isTypeFunction();
const dim = tf.parameterList.length;
if (dim == 1 || (dim > 1 && tf.parameterList[1].defaultArg))
Expand Down Expand Up @@ -2399,7 +2403,7 @@

override void visit(CtorDeclaration ctd)
{
//printf("CtorDeclaration::semantic() %s\n", toChars());
//printf("CtorDeclaration::semantic() %p %s\n", ctd, ctd.toChars());
if (ctd.semanticRun >= PASS.semanticdone)
return;
if (ctd._scope)
Expand Down Expand Up @@ -2482,12 +2486,15 @@
}
else if ((dim == 1 || (dim > 1 && tf.parameterList[1].defaultArg)))
{
//printf("tf: %s\n", tf.toChars());
//printf("tf: %s\n", toChars(tf));
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;
//printf("copy constructor %p\n", ctd);
if (param.storageClass & STC.ref_)
ctd.isCpCtor = true; // copy constructor

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

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/dsymbolsem.d#L2495

Added line #L2495 was not covered by tests
else
ctd.isMoveCtor = true; // move constructor

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

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/dsymbolsem.d#L2497

Added line #L2497 was not covered by tests
}
}
}
Expand Down Expand Up @@ -2978,7 +2985,10 @@

buildDtors(sd, sc2);

sd.hasCopyCtor = buildCopyCtor(sd, sc2);
bool hasMoveCtor;
sd.hasCopyCtor = buildCopyCtor(sd, sc2, hasMoveCtor);
sd.hasMoveCtor = hasMoveCtor;

sd.postblit = buildPostBlit(sd, sc2);

buildOpAssign(sd, sc2);
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dmd/e2ir.d
Original file line number Diff line number Diff line change
Expand Up @@ -5532,6 +5532,8 @@ elem *callfunc(const ref Loc loc,
v = ve.var.isVarDeclaration();
bool copy = !(v && (v.isArgDtorVar || v.storage_class & STC.rvalue)); // 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 (arg.rvalue) // marked with __rvalue
copy = false;

elems[i] = addressElem(ea, arg.type, copy);
continue;
Expand Down
39 changes: 24 additions & 15 deletions compiler/src/dmd/expression.d
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@
Loc loc; // file location
const EXP op; // to minimize use of dynamic_cast
bool parens; // if this is a parenthesized expression
bool rvalue; // true if this is considered to be an rvalue, even if it is an lvalue

extern (D) this(const ref Loc loc, EXP op) scope @safe
{
Expand Down Expand Up @@ -1307,7 +1308,7 @@

override final bool isLvalue()
{
return true;
return !this.rvalue;
}

override void accept(Visitor v)
Expand Down Expand Up @@ -1351,7 +1352,7 @@

override bool isLvalue()
{
return true;
return !rvalue;

Check warning on line 1355 in compiler/src/dmd/expression.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/expression.d#L1355

Added line #L1355 was not covered by tests
}

override void accept(Visitor v)
Expand Down Expand Up @@ -1397,7 +1398,7 @@
override final bool isLvalue()
{
// Class `this` should be an rvalue; struct `this` should be an lvalue.
return type.toBasetype().ty != Tclass;
return !rvalue && type.toBasetype().ty != Tclass;
}

override void accept(Visitor v)
Expand Down Expand Up @@ -1782,7 +1783,7 @@
/* string literal is rvalue in default, but
* conversion to reference of static array is only allowed.
*/
return (type && type.toBasetype().ty == Tsarray);
return !rvalue && (type && type.toBasetype().ty == Tsarray);
}

/********************************
Expand Down Expand Up @@ -2719,7 +2720,7 @@

override bool isLvalue()
{
if (var.storage_class & (STC.lazy_ | STC.rvalue | STC.manifest))
if (rvalue || var.storage_class & (STC.lazy_ | STC.rvalue | STC.manifest))
return false;
return true;
}
Expand Down Expand Up @@ -3098,7 +3099,7 @@

override final bool isLvalue()
{
return true;
return !rvalue;
}

override void accept(Visitor v)
Expand Down Expand Up @@ -3303,6 +3304,8 @@

override bool isLvalue()
{
if (rvalue)
return false;

Check warning on line 3308 in compiler/src/dmd/expression.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/expression.d#L3308

Added line #L3308 was not covered by tests
if (e1.op != EXP.structLiteral)
return true;
auto vd = var.isVarDeclaration();
Expand Down Expand Up @@ -3530,6 +3533,8 @@

override bool isLvalue()
{
if (rvalue)
return false;

Check warning on line 3537 in compiler/src/dmd/expression.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/expression.d#L3537

Added line #L3537 was not covered by tests
Type tb = e1.type.toBasetype();
if (tb.ty == Tdelegate || tb.ty == Tpointer)
tb = tb.nextOf();
Expand Down Expand Up @@ -3648,7 +3653,7 @@

override bool isLvalue()
{
return true;
return !rvalue;
}

override void accept(Visitor v)
Expand Down Expand Up @@ -3777,7 +3782,7 @@
override bool isLvalue()
{
//printf("e1.type = %s, to.type = %s\n", e1.type.toChars(), to.toChars());
if (!e1.isLvalue())
if (rvalue || !e1.isLvalue())
return false;
return (to.ty == Tsarray && (e1.type.ty == Tvector || e1.type.ty == Tsarray)) ||
e1.type.mutableOf.unSharedOf().equals(to.mutableOf().unSharedOf());
Expand Down Expand Up @@ -3834,7 +3839,7 @@

override bool isLvalue()
{
return e1.isLvalue();
return !rvalue && e1.isLvalue();
}

override void accept(Visitor v)
Expand Down Expand Up @@ -3891,7 +3896,7 @@
/* slice expression is rvalue in default, but
* conversion to reference of static array is only allowed.
*/
return (type && type.toBasetype().ty == Tsarray);
return !rvalue && (type && type.toBasetype().ty == Tsarray);
}

override Optional!bool toBool()
Expand Down Expand Up @@ -3956,6 +3961,8 @@

override bool isLvalue()
{
if (rvalue)
return false;

Check warning on line 3965 in compiler/src/dmd/expression.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/expression.d#L3964-L3965

Added lines #L3964 - L3965 were not covered by tests
if (type && type.toBasetype().ty == Tvoid)
return false;
return true;
Expand Down Expand Up @@ -4005,7 +4012,7 @@

override bool isLvalue()
{
return e2.isLvalue();
return !rvalue && e2.isLvalue();
}

override Optional!bool toBool()
Expand Down Expand Up @@ -4080,7 +4087,7 @@

override bool isLvalue()
{
return e1.isLvalue();
return !rvalue && e1.isLvalue();
}

override void accept(Visitor v)
Expand All @@ -4103,7 +4110,7 @@

override bool isLvalue()
{
return e1.isLvalue();
return !rvalue && e1.isLvalue();

Check warning on line 4113 in compiler/src/dmd/expression.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/expression.d#L4113

Added line #L4113 was not covered by tests
}

override void accept(Visitor v)
Expand Down Expand Up @@ -4143,6 +4150,8 @@

override bool isLvalue()
{
if (rvalue)
return false;

Check warning on line 4154 in compiler/src/dmd/expression.d

View check run for this annotation

Codecov / codecov/patch

compiler/src/dmd/expression.d#L4154

Added line #L4154 was not covered by tests
auto t1b = e1.type.toBasetype();
if (t1b.isTypeAArray() || t1b.isTypeSArray() ||
(e1.isIndexExp() && t1b != t1b.isTypeDArray()))
Expand Down Expand Up @@ -4251,7 +4260,7 @@
{
return false;
}
return true;
return !rvalue;
}

override void accept(Visitor v)
Expand Down Expand Up @@ -4982,7 +4991,7 @@

override bool isLvalue()
{
return e1.isLvalue() && e2.isLvalue();
return !rvalue && e1.isLvalue() && e2.isLvalue();
}

override void accept(Visitor v)
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dmd/expression.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class Expression : public ASTNode
Loc loc; // file location
EXP op; // to minimize use of dynamic_cast
d_bool parens; // if this is a parenthesized expression
d_bool rvalue; // consider this an rvalue, even if it is an lvalue

size_t size() const;
static void _init();
Expand Down
Loading
Loading