Conversation
|
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.
|
e36b3b6 to
61ce191
Compare
This comment was marked as outdated.
This comment was marked as outdated.
aa52efa to
05f17a1
Compare
compiler/src/dmd/expressionsem.d
Outdated
| if (!sc.intypeof && exp.e1.type.ty == Tstruct && !exp.e1.isLvalue()) | ||
| { | ||
| error(exp.e1.loc, "cannot modify struct rvalue `%s`", |
There was a problem hiding this comment.
What other rvalues could appear here? Does it make sense to only restrict structs?
There was a problem hiding this comment.
The check in opOverloadBinaryAssign would get hit by:
int[] a;
a.length += 1;It would also get hit for a class C (new C) += 4;. That should be prevented too, but I don't want to do that in this pull.
|
rvalues cannot be assigned to by definition, correct? |
|
I snipped the buildkite failures below. The libdparse one is fixed upstream. The other projects are failing due to one of these dependency failures. |
Currently dmd allows it for a struct overloading assignment. |
7b84c34 to
e280cb0
Compare
Nested struct may assign context pointer's data, so not a no-op: https://github.com/CyberShadow/ae/blob/v0.0.3672/utils/array.d#LR1072-L1093
e280cb0 to
70f2bdb
Compare
|
|
Ideally this would catch the libdparse case but not the mir ones. I'll have a look next week. |
|
Closing in favour of the other PR. |
When assignment would call an operator overload.
Fixes #21507.
Note: This does not disallow calling e.g.
opAssignon an rvalue, only actual assignment.