diff --git a/compiler/src/dmd/expression.d b/compiler/src/dmd/expression.d index 097769bb601c..f5fbf805de17 100644 --- a/compiler/src/dmd/expression.d +++ b/compiler/src/dmd/expression.d @@ -2363,6 +2363,7 @@ extern (C++) final class CallExp : UnaExp bool inDebugStatement; /// true if this was in a debug statement bool ignoreAttributes; /// don't enforce attributes (e.g. call @gc function in @nogc code) bool isUfcsRewrite; /// the first argument was pushed in here by a UFCS rewrite + bool fromOpAssignment; // set when operator overload method call from assignment (2024 edition) VarDeclaration vthis2; // container for multi-context Expression loweredFrom; // set if this is the result of a lowering diff --git a/compiler/src/dmd/expressionsem.d b/compiler/src/dmd/expressionsem.d index fef1a40bdc1a..3dec8c5e7490 100644 --- a/compiler/src/dmd/expressionsem.d +++ b/compiler/src/dmd/expressionsem.d @@ -5301,6 +5301,17 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor setError(); } + // `e` calls opAssign, opOpAssign, opUnary!++, opUnary!-- when lowered from an assignment + private void setOpOverloadAssign(Expression e) + { + result = e; + auto ce = e.isCallExp(); + // rvalue error in discardValue from 2024 edition + if (!ce || !sc.hasEdition(Edition.v2024)) + return; + ce.fromOpAssignment = true; + } + /************************** * Semantically analyze Expression. * Determine types, fold constants, etc. @@ -9171,10 +9182,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor { if (Expression e = exp.opOverloadBinaryAssign(sc, aliasThisStop)) - { - result = e; - return; - } + return setOpOverloadAssign(e); Expression e; if (exp.e1.op == EXP.arrayLength) @@ -11811,10 +11819,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor // printf("PreExp::semantic('%s')\n", toChars()); if (Expression e = exp.opOverloadUnary(sc)) - { - result = e; - return; - } + return setOpOverloadAssign(e); // Rewrite as e1+=1 or e1-=1 Expression e; @@ -12554,10 +12559,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor else if (exp.op == EXP.assign) { if (Expression e = exp.isAssignExp().opOverloadAssign(sc, aliasThisStop)) - { - result = e; - return; - } + return setOpOverloadAssign(e); } else assert(exp.op == EXP.blit); @@ -12574,10 +12576,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor if (exp.op == EXP.assign && !exp.e2.implicitConvTo(exp.e1.type)) { if (Expression e = exp.isAssignExp().opOverloadAssign(sc, aliasThisStop)) - { - result = e; - return; - } + return setOpOverloadAssign(e); } if (exp.e2.checkValue()) return setError(); @@ -13242,10 +13241,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor override void visit(PowAssignExp exp) { if (Expression e = exp.opOverloadBinaryAssign(sc, aliasThisStop)) - { - result = e; - return; - } + return setOpOverloadAssign(e); if (exp.suggestOpOpAssign(sc, parent) || exp.e1.checkReadModifyWrite(exp.op, exp.e2)) @@ -13319,13 +13315,9 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor override void visit(CatAssignExp exp) { - //printf("CatAssignExp::semantic() %s\n", exp.toChars()); if (Expression e = exp.opOverloadBinaryAssign(sc, aliasThisStop)) - { - result = e; - return; - } + return setOpOverloadAssign(e); if (exp.suggestOpOpAssign(sc, parent)) return setError(); diff --git a/compiler/src/dmd/frontend.h b/compiler/src/dmd/frontend.h index 17cd86e1c714..0b32ea3ecddf 100644 --- a/compiler/src/dmd/frontend.h +++ b/compiler/src/dmd/frontend.h @@ -2594,6 +2594,7 @@ class CallExp final : public UnaExp bool inDebugStatement; bool ignoreAttributes; bool isUfcsRewrite; + bool fromOpAssignment; VarDeclaration* vthis2; Expression* loweredFrom; static CallExp* create(Loc loc, Expression* e, Array* exps); diff --git a/compiler/src/dmd/sideeffect.d b/compiler/src/dmd/sideeffect.d index e7d54d917adc..2e579972d359 100644 --- a/compiler/src/dmd/sideeffect.d +++ b/compiler/src/dmd/sideeffect.d @@ -258,6 +258,23 @@ bool discardValue(Expression e) error(e.loc, "discarded assignment to indexed array literal"); } } + // check assignment to struct rvalue + auto ce = e.isCallExp(); + if (ce && ce.fromOpAssignment) + { + if (auto dve = ce.e1.isDotVarExp()) + { + auto lhs = dve.e1; + auto ts = lhs.type.isTypeStruct(); + if (ts && !lhs.isLvalue() && !ts.sym.hasPointerField) // Don't disallow writing to data through a pointer field + { + error(lhs.loc, "assignment to struct rvalue `%s` is discarded", + lhs.toChars()); + errorSupplemental(e.loc, "if the assignment is needed to modify a global, call `%s` directly or use an lvalue", + dve.var.toChars()); + } + } + } return false; } switch (e.op) diff --git a/compiler/test/fail_compilation/struct_rvalue_assign.d b/compiler/test/fail_compilation/struct_rvalue_assign.d new file mode 100644 index 000000000000..42451768645f --- /dev/null +++ b/compiler/test/fail_compilation/struct_rvalue_assign.d @@ -0,0 +1,62 @@ +/* +TEST_OUTPUT: +--- +fail_compilation/struct_rvalue_assign.d(16): Error: assignment to struct rvalue `foo()` is discarded +fail_compilation/struct_rvalue_assign.d(16): if the assignment is needed to modify a global, call `opAssign` directly or use an lvalue +fail_compilation/struct_rvalue_assign.d(17): Error: assignment to struct rvalue `foo()` is discarded +fail_compilation/struct_rvalue_assign.d(17): if the assignment is needed to modify a global, call `opOpAssign` directly or use an lvalue +fail_compilation/struct_rvalue_assign.d(18): Error: assignment to struct rvalue `foo()` is discarded +fail_compilation/struct_rvalue_assign.d(18): if the assignment is needed to modify a global, call `opUnary` directly or use an lvalue +--- +*/ +module sra 2024; + +void main() +{ + foo() = S.init; + foo() += 5; + ++foo(); + *foo(); // other unary ops may be OK + + // allowed + foo().opAssign(S.init); + foo().opOpAssign!"+"(5); + foo().opUnary!"++"(); +} + +S foo() => S.init; + +struct S +{ + int i; + + void opAssign(S s); + void opOpAssign(string op : "+")(int); + void opUnary(string op : "++")(); + void opUnary(string op : "*")(); +} + +void test() +{ + int i; + + static struct Ptr + { + int* p; + void opAssign(int rhs) { *p = rhs; } + } + Ptr(&i) = 1; // allowed + + struct Nested + { + void opAssign(int rhs) { i = rhs; } + } + Nested() = 1; // allowed + + static struct StaticOp + { + static si = 0; + static void opAssign(int rhs) { si = rhs; } + } + StaticOp() = 1; // allowed +}