Disallow discarding an assignment to a struct rvalue (#22369)

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.

Require 2024 edition for error

Author: ntrel

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
 

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();

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<Expression* >* exps);

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)

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
+}