nail down the final validity rules: references and unions - #2337
nail down the final validity rules: references and unions#2337RalfJung wants to merge 2 commits into
Conversation
|
@rustbot label +I-lang-nominated |
75ee1eb to
caf7145
Compare
caf7145 to
93d25cc
Compare
|
A proposal came up in rust-lang/unsafe-code-guidelines#620 to slightly strengthen the requirements for references / @scottmcm do you have suggestions for how that should be worded? |
| For unsized types, this check considers dynamic information from the metadata: | ||
| If `T` has an unsized tail of slice type (`[U]`), it is inhabited of the length encoded in the metadata is 0 or if `U` is inhabited. |
There was a problem hiding this comment.
These two lines are for rust-lang/unsafe-code-guidelines#620. We didn't explicitly discuss this case in the opsem FCP.
a094ac6 to
7aec9f3
Compare
7aec9f3 to
c8ce01e
Compare
I don't have anything concrete. At least as a mental model -- whether it can be phrased well for a reference I'm less sure -- I wonder if there's a general rule here like saying that the type in the reference must, including looking at the metadata, represent a well-formed and inhabited type. (Part of me wants to say it comes from an unsizing, but there's nothing to unsize to a Basically I wish the "length has to be zero for uninhabited elements" and "length bound for non-ZST elements is |
|
So what do you think about the wording I put in this PR? |
|
@traviscross has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
|
I reviewed the proposed rules both for unions and for references, and this looks reasonable to me. @rfcbot reviewed |
|
This makes me want a separate language construct for "no, I'm not doing partial overwrite silliness" because this rule has meant that unions are not usable for the cases I wanted, but if this is the right answer for FFI unions and such then so be it. @rfcbot reviewed |
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
FWIW the unsized type part hasn't been FCP'd by us yet, as mentioned above. We can do that in a separate issue or a joint FCP here. |
|
@rfcbot fcp cancel |
|
@traviscross proposal cancelled. |
|
Thanks @RalfJung. @rfcbot fcp merge lang,opsem (In addition to what was previously FCPed by opsem, opsem is signing off here on what RalfJ mentions in #2337 (comment).) |
|
@traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:
No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
|
@rfcbot reviewed |
|
FWIW @saethlin has ticked their box in rust-lang/unsafe-code-guidelines#620, that should be carried over. |
|
@rfcbot reviewed |
| For unsized types, this check considers dynamic information from the metadata: | ||
| In particular, if `T` has an unsized tail of slice type `[U]`, and if `U` is uninhabited, and if the length encoded in the metadata is non-zero, then the pointee is uninhabited. |
There was a problem hiding this comment.
I feel like making inhabitedness depend on metadata values is not right / clean.
I would rather make this a separate restriction on the value of the metadata. That is, I would say something along the lines of "A reference or Box<T> must ... have valid metadata" and "slice metadata for a slice type with uninhabited elements, it must be 0".
This allows adding other restrictions in the future, such as the one mentioned by @scottmcm that slice metadata can at most be usize::MAX / size_of::<T>() (for non-ZSTs).
There was a problem hiding this comment.
We generally pretty consistently treat the metadata as telling us the "actual type" that we use to inspect the pointee. I think it would be odd do do something else here. It would look like an arbitrary set of rules, rather than just something that falls out of a more general principle.
This allows adding other restrictions in the future, such as the one mentioned by @scottmcm that slice metadata can at most be usize::MAX / size_of::() (for non-ZSTs).
As I mentioned in reply to Scott, that restriction is already present. It follows from the fact that the reference has to be dereferenceable for its actual dynamic size.
This translates the following t-opsem FCPs into Reference text:
Cc @rust-lang/opsem
@rust-lang/lang you have not FCP'd this yet; please let us know if you want this to go through your own decision process as well.
Fixes rust-lang/unsafe-code-guidelines#413
Fixes rust-lang/unsafe-code-guidelines#414
Fixes rust-lang/unsafe-code-guidelines#438