diff --git a/changelog/dmd.struct-rvalue-assign.dd b/changelog/dmd.struct-rvalue-assign.dd new file mode 100644 index 000000000000..703cf97a2beb --- /dev/null +++ b/changelog/dmd.struct-rvalue-assign.dd @@ -0,0 +1,31 @@ +Disallow discarding an assignment to a struct rvalue + +It has always been an error for $(DDSUBLINK spec/struct, POD, POD) structs +to assign from an rvalue. It is now an error to discard the result of an +assignment from a struct rvalue when it would call `opAssign`, `opOpAssign`, +`opUnary!"++"`, or `opUnary!"--"` and the struct has no tail-mutable pointer +fields: + +--- +struct S +{ + int i; + + void opAssign(S s); +} + +S foo(); + +void main() +{ + foo() = S(2); // Error, possible no-op +} +--- + +Above, unless `opAssign` mutates global data, the assignment in `main` +will have no effect and indicates a bug. + +If a struct rvalue assignment is needed to mutate global state, either +call the operator overload method directly or use an lvalue. +Note: Calling a non-const method on a struct rvalue +$(DDSUBLINK spec/struct, member-functions, is allowed). diff --git a/compiler/src/dmd/sideeffect.d b/compiler/src/dmd/sideeffect.d index 2e579972d359..3cb9698f554b 100644 --- a/compiler/src/dmd/sideeffect.d +++ b/compiler/src/dmd/sideeffect.d @@ -264,9 +264,10 @@ bool discardValue(Expression e) { if (auto dve = ce.e1.isDotVarExp()) { + import dmd.dcast : implicitConvTo; 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 + if (ts && !lhs.isLvalue() && ts.constOf().implicitConvTo(ts)) // Don't disallow writing to data through a *mutable* pointer field { error(lhs.loc, "assignment to struct rvalue `%s` is discarded", lhs.toChars()); diff --git a/compiler/test/fail_compilation/struct_rvalue_assign.d b/compiler/test/fail_compilation/struct_rvalue_assign.d index 42451768645f..c4c2572526de 100644 --- a/compiler/test/fail_compilation/struct_rvalue_assign.d +++ b/compiler/test/fail_compilation/struct_rvalue_assign.d @@ -29,6 +29,7 @@ S foo() => S.init; struct S { int i; + const(int)* p; // writes through `p` are not allowed, should ignore void opAssign(S s); void opOpAssign(string op : "+")(int); diff --git a/spec/legacy.dd b/spec/legacy.dd index 6e6c8417c1c1..cb5fbf4cc1dc 100644 --- a/spec/legacy.dd +++ b/spec/legacy.dd @@ -25,8 +25,6 @@ $(COMMENT $(TROW Escaping $(DDSUBLINK spec/attribute, scope, `scope`) data, `scope` is $(LINK2 https://github.com/dlang/DIPs/blob/master/DIPs/other/DIP1000.md, enforced) in `@safe` code, 2024) - $(TROW $(RELATIVE_LINK2 struct-assign, Assigning to struct rvalue), - Disallow for structs which overload e.g. `opAssign`, 2024) $(COMMENT Not enforced yet) $(TROW Struct/union postblit, use a $(DDSUBLINK spec/struct, struct-copy-constructor, @@ -102,41 +100,6 @@ struct Foo } --- -$(H2 $(LNAME2 struct-assign, Assigning to struct rvalue)) - - $(P It has always been an error for $(DDSUBLINK spec/struct, POD, POD) structs - to assign from an rvalue.) - - $(P From the 2024 edition, it is an error to discard the result of an - assignment from a struct rvalue when it would call `opAssign`, `opOpAssign`, - `opUnary!"++"`, or `opUnary!"--"` and the struct has no pointer - fields:) - ---- -struct S -{ - int i; - - void opAssign(S s); -} - -S foo(); - -void main() -{ - foo() = S(2); // Error, possible no-op -} ---- - $(P Above, unless `opAssign` mutates global data, the assignment in `main` - will have no effect and indicates a bug.) - -$(H3 Corrective Action) - - $(P If a struct rvalue assignment is needed to mutate global state, either - call the operator overload method directly or use an lvalue. - Note: Calling a non-const method on a struct rvalue - $(DDSUBLINK spec/struct, member-functions, is allowed).) - $(SPEC_SUBNAV_PREV_NEXT glossary, Glossary, editions, Editions) )