Disallow discarding an assignment to a struct rvalue #22369
Conversation
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.
|
Thanks for your pull request and interest in making D better, @ntrel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#22369" |
Needed for dlang/dmd#22369.
Needed for dlang/dmd#22369.
Needed for dlang/dmd#22369.
|
The tests for e36981e showed only 3 projects in buildkite were broken, all due to them depending on older libdparse (which had a real bug). So I've made this require >= 2024 edition to enable the new error as mentioned. I pushed a fix for the C++ header (which CircleCI was failing on). |
compiler/src/dmd/expressionsem.d
Outdated
| setError(); | ||
| } | ||
|
|
||
| void setOpOverloadAssign(Expression e) |
There was a problem hiding this comment.
Need function documentation
There was a problem hiding this comment.
I've made it private and added a comment.
|
Can you please be clearer about what problem this addresses? |
|
@WalterBright It detects assignments which have no effect (aside from writing to globals which is rare - I haven't seen any), indicating a bug. In these cases the programmer believes it has an effect - e.g. the libdparse one. |
3 participants
More permissive version of #21717 (which already found an unintended no-op in libdparse). This allows the assignment if it returns a value which is used, so this doesn't break libmir.
Fixes #21507.
Error only applies when struct has no pointer fields - as a pointer could be written through, making the assignment meaningful. (This could be tightened to also error when there's a pointer field but it's not tail mutable). Note this is required not to break ae.utils.array.list whose
opAssignwrites through its context pointer.My plan is to enable this for the 2024 edition, as it potentially can break existing calls of 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.