Skip to content

[redis] Avoid rvalue struct assign - #2838

Closed
ntrel wants to merge 1 commit into
vibe-d:masterfrom
ntrel:rvalue-assign
Closed

[redis] Avoid rvalue struct assign#2838
ntrel wants to merge 1 commit into
vibe-d:masterfrom
ntrel:rvalue-assign

Conversation

@ntrel

@ntrel ntrel commented Dec 12, 2025

Copy link
Copy Markdown

I'm trying to make assigning to a struct rvalue a compiler error, because often the assignment is silently thrown away:
dlang/dmd#21717 (comment)

That PR breaks some buildkite projects, including this one - so it will probably be a deprecation/error in an edition. This PR makes this project compatible.

Call opAssign directly instead, which is allowed by the spec:

an instance method can still be called on an rvalue struct instance,
even if the method is not const

https://dlang.org/spec/struct.html#member-functions

Disclaimer: I'm not sure if this is a suitable solution for this project, as I'm not familiar with it. From running dub test, it appears to work without triggering the new dmd error.

Call `opAssign` directly instead, which is allowed by the spec:

> an instance method can still be called on an rvalue struct instance,
even if the method is not const

https://dlang.org/spec/struct.html#member-functions

@s-ludwig s-ludwig left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in order to not break other code that uses the same pattern, it is necessary to implement opIndexAssign instead of fixing the place where the assignment is made:

void opIndexAssign(U)(U value, IDS id) { auto entry = this[id]; entry = value; }

@ntrel

ntrel commented Jan 19, 2026

Copy link
Copy Markdown
Author

Thanks, sorry for the late response. I closed the original PR and the replacement one doesn't need to change vibe.d, because it allows assignment to an rvalue if the struct has a field with a pointer.

@ntrel ntrel closed this Jan 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants