Skip to content

Commit

Permalink
2nd try at __rvalue(expression)
Browse files Browse the repository at this point in the history
  • Loading branch information
WalterBright committed Dec 10, 2024
1 parent 2674d22 commit 3c180ae
Show file tree
Hide file tree
Showing 28 changed files with 318 additions and 149 deletions.
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 @@ Return:
*/
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 checkHasBothRvalueAndCpCtor(StructDeclaration sd, CtorDeclaration ctor, Tem
*/
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 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor

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 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor
}
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 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor

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 @@ extern (C++) abstract class Expression : ASTNode
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 @@ extern (C++) class IdentifierExp : Expression

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

override void accept(Visitor v)
Expand Down Expand Up @@ -1351,7 +1352,7 @@ extern (C++) final class DsymbolExp : Expression

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 @@ extern (C++) class ThisExp : Expression
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 @@ extern (C++) final class StringExp : Expression
/* 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 @@ extern (C++) final class VarExp : SymbolExp

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 @@ extern (C++) class BinAssignExp : BinExp

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

override void accept(Visitor v)
Expand Down Expand Up @@ -3303,6 +3304,8 @@ extern (C++) final class DotVarExp : UnaExp

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 @@ extern (C++) final class CallExp : UnaExp

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 @@ extern (C++) final class PtrExp : UnaExp

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

override void accept(Visitor v)
Expand Down Expand Up @@ -3777,7 +3782,7 @@ extern (C++) final class CastExp : UnaExp
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 @@ extern (C++) final class VectorArrayExp : UnaExp

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

override void accept(Visitor v)
Expand Down Expand Up @@ -3891,7 +3896,7 @@ extern (C++) final class SliceExp : UnaExp
/* 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 @@ extern (C++) final class ArrayExp : UnaExp

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 @@ extern (C++) final class CommaExp : BinExp

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

override Optional!bool toBool()
Expand Down Expand Up @@ -4080,7 +4087,7 @@ extern (C++) final class DelegatePtrExp : UnaExp

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

override void accept(Visitor v)
Expand All @@ -4103,7 +4110,7 @@ extern (C++) final class DelegateFuncptrExp : UnaExp

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 @@ extern (C++) final class IndexExp : BinExp

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 @@ extern (C++) class AssignExp : BinExp
{
return false;
}
return true;
return !rvalue;
}

override void accept(Visitor v)
Expand Down Expand Up @@ -4982,7 +4991,7 @@ extern (C++) final class CondExp : BinExp

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

0 comments on commit 3c180ae

Please sign in to comment.