-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Forbid partial outbound moves from immutable variables #238
Conversation
|
||
Rust's "immutable" variables do *not* provide *strict immutability*. There are three exceptions: | ||
|
||
1. it is legal to have internal mutability even inside "immutable" variables, via `UnsafeSell<T>`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UnsafeSell
should be UnsafeCell
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zwarich Thanks, corrected.
|
||
```rust | ||
#[deriving(Show)] | ||
enum Gender { Male, Female } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong. Even for 'sex' instead of 'gender' it'd be inaccurate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need to be pedantic SJWs about this? Its an example, its not a social statement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need to pedantically defend this choice? It's an example, no need to make a social statement by keeping it precisely like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rust' score values include inclusivity. Please refer to the Code of Conduct.
Beyond that, this is just factually wrong. Intersex people exist.
Furthermore, it is trivial to provide another example that isn't exclusionary.=
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be pedantic: it's not factually wrong if the domain of discussion is e.g. the ends of certain I/O cables. (At least, I'm not aware of a new in vogue term there, though I am out of touch with current A/V terminology).
But otherwise I agree: it's trivial to change the example, and I would rather discussion not be further derailed.
Can we just assume for now that the author will adjust it in the near future?
(Update: of course the context of this example is not A/V cabling but rather a struct Person
, so it is not like @steveklabnik 's comment was unwarranted. I was just idly musing.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I would just change it to any other enumeration. The Django people use a conference talk example of submitted/reviewed/accepted/rejected, but it's really not as important, as long as it's not gender/sex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, will do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really stupid nitpick, I can't believe I'm wasting my time reading this garbage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iopq https://github.com/rust-lang/rust/wiki/Note-development-policy#conduct
Please review the first, third, sixth and eighth bullets. Please also review the "Moderation" section beneath the code of conduct, notably about "avoid flirting with offensive or sensitive issues, particularly if they're off-topic"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I'll note that @CloudiDust had a particularly good response to this issue; it might not be something they've really thought about before, but, once the issue was raised, they were clearly very happy to make the small amount of effort it takes be more inclusive. 👏 )
Why do you call outbound move a mutation? For example, I do not see it as a mutation, because the values themselves are not modified (they are just shallow-copied), and moving immutable values around is a very natural concept to me. |
@netvl, moves are mutations, they at least mutate the bindings between variables and values. Semantically, moves are not shallow copies but, eh, moves. If moves were shallow copies, then the original variables/fields should still contain values after moves, but actually they will contain nothing. Full outbound moves only mutate bindings, so they can be seen as "not mutations" in the sense that the values themselves are not mutated. |
@netvl Well, strictly speaking, with the current move semantics it totally is a real mutation: http://is.gd/3ru22U That said, I find being able to move out of immutable values to be handy, and the only way to "expose" any mutation is with unsafe code. Anything else will just trigger a compiler error for trying to access the partially or totally moved value. I find the overall premise of the RFC confusing, though. It says that removing partial moves from immutable values removes the need for NoisyDrop, but I don't see how that follows. As I understood it, the point of NoisyDrop is that a destructor might be re-prioritized if there exists a branch that moves the value before the end of the function, and a branch that doesn't. This seems to be a problem regardless of partial moves, and this solution doesn't even address the issue of partial moves if I have the value mutably. Am I missing something? |
s/"immutable"/"not mutations", sorry cannot edit on the phone. |
@gankro, The change in the RFC by itself doesn't enable guaranteed lifetimes. But combining this with explicit drops will do. And explicit drops need no RFC as they are already supported by the language, just not used to enforce this guarantee cuurrently. |
@gankro, this RFC only forbids partial outbound moves. And I agree that this move pattern can be handy sometimes. (Please refer to the My full solution to the ergonomic issues caused by this RFC is "fine grained mutation control with the |
Note that non- |
+1 this or Alternative 1, partial moves are a bit confusing and it would be better to use the ref pattern more often. |
@huonw, yes and I listed this as one of the exceptions in the RFC. This exception is justified because Cells are explicitly designed for internal mutability, a exceptional case, and the programmer must opt-in. While partial outbound moves are mutations that the programmer can not opt-out currently. |
You can't move out of types with destructors. |
@sfackler, thanks and I'll add discussions about types with |
@gankro, wow, that's surprising. However, I think that since this can only be observed via unsafe code, and the original binding is prohibited from future use, it is acceptable behavior for me. @CloudiDust, well, modifying bindings is not the same as modifying values, and I'd argue that Anyway, now I see the argument why partial moves are mutations. However, this is very special kind of mutation. It is handled differently by the compiler, it causes different errors, it is natural (for me, at least), the fact that the binding is mutated can't be observed in the safe code (same as in the case of full move) and I'd argue that it is not against intuition. Even if the field is actually mutated, it still can't be used in safe code. How is really this:
different from this:
? Moving from The other argument (related to drop) still holds, I guess, but I'm not that familiar with drop-related proposals to say anything about that. |
@netvl, in the first case, the value in But a programmer can explicitly move the whole value to a mutable variable first, and then do value mutation on it. And this is fine. In the second case, no value is mutated, only bindings are. As you have pointed out, |
@CloudiDust in my opinion, your RFC's description of "Exception 1" is misrepresenting the situation. @huonw is pointing out in his comment that in addition to An insight that I had while preparing my ML workshop talk was that That is, writing I realize that this comment is not providing much direct feedback on this RFC itself (apart from undermining the proposed model of immutability that the RFC is trying to shoehorn onto Rust). I am not yet convinced that the change proposed here would buy us all that much, but I need to think on the matter more. |
As Niko said a while back 'mut' really means unique, 'insight' is exactly how it felt when he pointed that out. But lets not restart the mutpocalypse :-) Sorry for off-topic. |
@nick29581 yes, my "insight" was really just finding a nuanced phrase to make it not sound like we made a grave mistake in our own terminology + UX. :) |
Thanks for the new perspective. Yeah I remember the mutpocalypse as well. But the problem is that we are not using Or we can expose So my preference would be sacrificing a bit of theoretical beauty and do what seems practically more "correct". |
@nick29581 @pnkfelix And I will adjust the RFC accordingly. Guess I have much to change tonight. ;) |
Everyone: As @rkjnsn pointed out in the comments in #210, partial moves themselves forbid full moves later. So explicit drop alone can guarantee lifetimes. Which eliminates half the usefulness of this RFC. Now the problem becomes: Should |
@nick29581 @pnkfelix, I think from the exclusive access/uniqueness/aliasing control point of view, the semantics of a "bare variable" (that is one without |
No, the value is not mutated, since you can't use the parts that are moved out (of course, moving something back in is and must be disallowed). Disallowing this seems fundamentally wrong, since it means that "let (a, b) = (Foo::new(), Foo::new());" and "let p = (Foo::new(), Foo::new());", which intuitively should be fully eqivalent, are no longer equivalent, since you can move out a and b, but not the two parts of p You could forbid all moving of non-mut let variables, but this is also fundamentally wrong, because dropping a value is an implicit move, and if dropping is possible (which it must be), then it must be possible to consume the value otherwise, which requires moving. Or in other words, if you don't use a part of the variable, who cares what happened to it, and if you use it, it either has the initial value or the program doesn't compile, so it's fine. |
Well, I disagree. I don't see why immutability should disallow this. Looks like we have different notions of what is "natural" here :)
Again, while moves, partial or not, do mutate values, it is not observable, because the compiler will never allow you to use or overwrite the moved value through the affected binding. Consequently, the immutable value is semantically immutable even in presence of partial moves. |
@netvl @bill-myers, yeah, we have different notions of what is "natural" here. I have realized, both perspective are internally consistent in their own ways. My perspective is: immutable values should never be mutated in any way, only bindings can, so full outbound moves are fine, while partial outbound moves should be forbidden. While you two's perspective is: immutable variables and fields should follow the same rule: they either have the initial value, or are unusable, so partial outbound moves are fine, just like full outbound moves are. So this RFC has no clear advantage in the semantics aspect, and also no clear advantage in the bug catching aspect. (In practice, the bug in the example snippet would most likely be caught eventually, just not when the move happened, but when we try to use Thus I believe the benefits of this RFC to be negligible, and not worth the cost in ergonomics. I'll withdraw this RFC. Thanks everyone. :) EDIT: Made the descriptions of the two perspectives clearer. |
Also @steveklabnik, I think some material here may be worth incorporating into the Guide or other discussions about |
This RFC proposes that the semantics of immutable variables be refined by forbidding partial outbound moves, so immutable variables in Rust become more immutable.
2. guaranteed lifetimes for values with move semantics ("movable values") can be achieved by combining this with explicitdrop
calls.This makesNoisyDrop
/QuietDrop
from #210 unnecessary.(EDIT: Guaranteed lifetimes can be provided by explicit
drop
calls alone.)Rendered View.