From 84d9400ce3d02351bf646b26e455e9a4212b37ab Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Fri, 23 Jan 2026 11:58:32 +0000 Subject: [PATCH 1/5] Disallow discarding an assignment to a struct rvalue Error applies when struct has no pointer fields. (This could be tightened to apply when a pointer field is not tail mutable). My plan is to enable this for the 2024 edition, as it can break a user-defined op overload which has a (non-pointer) field but mutates a global. That should be a rare case and the compiler suggests workarounds. Note that requiring weak purity (for e.g. `opAssign`) would mean the error wouldn't detect bug-prone cases that aren't marked/inferred pure. --- compiler/src/dmd/expression.d | 1 + compiler/src/dmd/expressionsem.d | 40 +++++------- compiler/src/dmd/sideeffect.d | 17 +++++ .../fail_compilation/struct_rvalue_assign.d | 62 +++++++++++++++++++ 4 files changed, 95 insertions(+), 25 deletions(-) create mode 100644 compiler/test/fail_compilation/struct_rvalue_assign.d diff --git a/compiler/src/dmd/expression.d b/compiler/src/dmd/expression.d index 097769bb601c..c1b213a4b005 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 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..65c0b8c576f6 100644 --- a/compiler/src/dmd/expressionsem.d +++ b/compiler/src/dmd/expressionsem.d @@ -5301,6 +5301,15 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor setError(); } + void setOpOverloadAssign(Expression e) + { + result = e; + auto ce = e.isCallExp(); + if (!ce) + return; + ce.fromOpAssignment = true; + } + /************************** * Semantically analyze Expression. * Determine types, fold constants, etc. @@ -9171,10 +9180,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 +11817,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 +12557,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 +12574,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 +13239,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 +13313,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/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 +} From 32adb314234ac85596f6d4ea39ef640ed985f53b Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Fri, 9 Jan 2026 16:20:19 +0000 Subject: [PATCH 2/5] Restart tests From ab016915231812334cea551a6198d48822283d7e Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Fri, 9 Jan 2026 17:51:37 +0000 Subject: [PATCH 3/5] Update frontend.h --- compiler/src/dmd/frontend.h | 1 + 1 file changed, 1 insertion(+) 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); From edcae9542ab0275416b2123ea03943e88c57eb7c Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Fri, 9 Jan 2026 17:55:02 +0000 Subject: [PATCH 4/5] Require 2024 edition for error --- compiler/src/dmd/expression.d | 2 +- compiler/src/dmd/expressionsem.d | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/src/dmd/expression.d b/compiler/src/dmd/expression.d index c1b213a4b005..f5fbf805de17 100644 --- a/compiler/src/dmd/expression.d +++ b/compiler/src/dmd/expression.d @@ -2363,7 +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 + 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 65c0b8c576f6..e5dcfb0e069b 100644 --- a/compiler/src/dmd/expressionsem.d +++ b/compiler/src/dmd/expressionsem.d @@ -5305,7 +5305,8 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor { result = e; auto ce = e.isCallExp(); - if (!ce) + // rvalue error in discardValue from 2024 edition + if (!ce || !sc.hasEdition(Edition.v2024)) return; ce.fromOpAssignment = true; } From 5d824f240fed950c93ad8d7261e7d7b5a7fd3e29 Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Mon, 12 Jan 2026 10:21:44 +0000 Subject: [PATCH 5/5] Make `setOpOverloadAssign` private, add comment --- compiler/src/dmd/expressionsem.d | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/src/dmd/expressionsem.d b/compiler/src/dmd/expressionsem.d index e5dcfb0e069b..3dec8c5e7490 100644 --- a/compiler/src/dmd/expressionsem.d +++ b/compiler/src/dmd/expressionsem.d @@ -5301,7 +5301,8 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor setError(); } - void setOpOverloadAssign(Expression e) + // `e` calls opAssign, opOpAssign, opUnary!++, opUnary!-- when lowered from an assignment + private void setOpOverloadAssign(Expression e) { result = e; auto ce = e.isCallExp();