-
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
Destructuring Drop
ADTs via ManuallyDrop
#3466
base: master
Are you sure you want to change the base?
Destructuring Drop
ADTs via ManuallyDrop
#3466
Conversation
edf3e52
to
c86637f
Compare
c86637f
to
1888553
Compare
`_` doesn't trigger a move.
text/3466-manuallydrop-deref-move.md
Outdated
} | ||
``` | ||
|
||
If code like the above example actually exists in the ecosystem, this proposal would make it unsound. |
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 actually do have an unpublished crate that relies on fields not being able to be moved out if I implement Drop
. In my crate a type SomeStruct
has a field with type FieldReader
. The latter type is a ZST that implements Deref
based on the address of &FieldReader
(in this case, it reads other fields from SomeStruct
), that relies on the fact that if I make SomeStruct
implement Drop
, and construction of FieldReader
unsafe, then all pointers to FieldReader
are guaranteed to exist only within SomeStruct
, since moving out of SomeStruct
isn't allowed.
It would be nice if there is a way to spell "can't move out of any fields of this struct" that does not involve a no-op drop implementation, or maybe there is a way already and I just don't know about it.
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.
What I would expect/was expecting is that in practice, this guarantee would be provided via privacy. Your FieldReader
would be a private field with a public get()
/get_mut()
accessor.
(Also, your use-case might be impacted by subobject provenance discussions, you might want to share details at rust-lang/unsafe-code-guidelines#256)
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.
What I'm hoping to achieve with FieldReader
is that I want it to be a public field, while emulating getters through Deref
, so using accessor methods defeats its point entirely.
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 conversation reminds me of the debate over the soundness of partial-borrow
vs replace_with
. In partial-borrow
I also (ab)use a language inflexibility to allow the use of fields rather than accessors.
(Providing fields rather than accessors is more than just a simple ergonomic improvement to elide some ()
. There are important borrowck features that are available only via borrowck's special handling of fields.)
So it seems both @fee1-dead's crate and partial-borrow
would benefit from some way to get access to those "field-specific" borrowck behaviours. In the meantime this is another case where not only (i) adding a new language feature can make existing code unsound (ii) that existing code is not theoretical and cannot be readily expressed a different way.
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.
(Providing fields rather than accessors is more than just a simple ergonomic improvement to elide some
()
. There are important borrowck features that are available only via borrowck's special handling of fields.)
Yes. I forgot to mention this in my comment, but this is one big reason I'm using a FieldReader
design, to allow having mutable references for two fields.
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.
@fee1-dead would your FieldReader
also be unsound in the presence of replace_with
alone? (Take &mut
to a FieldReader
, use replace_with
to move out of it, then take a shared reference to the moved-out value)
I've expanded the "Rationale and Alternatives" section in light of new discussion on Internals. |
ManuallyDrop
Drop
ADTs via ManuallyDrop
Rendered
Pre-RFC discussion on Internals
@rustbot label A-drop
Rendered