stabilize size_of_val_raw, align_of_val_raw, Layout::for_value_raw#157572
stabilize size_of_val_raw, align_of_val_raw, Layout::for_value_raw#157572RalfJung wants to merge 1 commit into
Conversation
|
The Miri subtree was changed cc @rust-lang/miri |
|
r? @clarfonthey rustbot has assigned @clarfonthey. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| /// - a [trait object] `dyn Trait`, then the vtable part of the pointer must point | ||
| /// to a valid vtable for `Trait`, and the size | ||
| /// of the *entire value* (dynamic tail length + statically sized prefix) | ||
| /// must fit in `isize`. |
There was a problem hiding this comment.
We document that the first part here ("vtable part of the pointer must point to a valid vtable") is a validity invariant for raw pointers (https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html#r-undefined.validity.wide).
For obtaining a Layout, it makes sense to me that we can't exceed isize. However, I'm wondering if we want / will want a safe version of this function, that checks that you haven't exceeded isize. It seems like that's eminently possible since essentially the UB here is from doing an unchecked_add on the two involved sizes. I guess users will probably be able to do that themselves in the future via std::ptr::metadata (safe) and then checked_add of any sized prefix and the DynMetadata::size_of (safe). Maybe we know enough to stabilize at least some of the metadata surface area too...
There was a problem hiding this comment.
We document that the first part here ("vtable part of the pointer must point to a valid vtable") is a validity invariant for raw pointers
Yes. So it is technically redundant. It still seems worth repeating? It is dangerous to rely on validity invariants are a precondition since validity invariants may become weaker in future versions of Rust.
I'm wondering if we want / will want a safe version of this function, that checks that you haven't exceeded isize.
Maybe we do, but I don't think that has to be part of this stabilization.
In terms of how that would impact this stabilization, I guess the point is that maybe the functions we are stabilizing here should be called size_of_val_unchecked or size_of_val_raw_unchecked then?
There was a problem hiding this comment.
Agreed on all points -- I think the impact on naming is the only thing for this stabilization.
There was a problem hiding this comment.
I agree that we want (to at least hold the space for) a safe option with checked overflow for MetaSized types. But since a requirement for being safe is specifically that the data pointer part doesn't matter, a part of me feels like the API for such shouldn't even be given the data part of the address, but only the metadata.
e.g. the API name in my head takes the shape Layout::with_metadata::<T>(<T as Pointee>::Metadata) -> Self.
There was a problem hiding this comment.
The function name discussion should probably happen on the tracking issue as that's where libs-api will do their FCP.
This comment has been minimized.
This comment has been minimized.
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
Stabilizes versions of
size_of_val_rawand friends that can be invoked on raw pointers, which means they can be used even if one does not have a pointer to a "valid value". This was held up for a while because figuring out the exact safety requirements is tricky, but I think the ones we now have had for a while (plus the minor clarifications in this PR) are "good enough": we basically do case distinction on the unsized tail of the type, and spell out the appropriate requirement for each case. This avoids making blanket statements about future kinds of DST that we may introduce eventually.These are the requirements I should we should stabilize:
TisSized, this function is always safe to call.Tis:[U]orstr, then the length of the slice tail must be an initializedinteger, and the size of the entire value
(dynamic tail length + statically sized prefix) must fit in
isize.For the special case where the dynamic tail length is 0, this function
is safe to call.
NOTE: the reason this is safe is that if an overflow were to occur already with size 0,
then we would stop compilation as even the "statically known" part of the type would
already be too big (or the call may be in dead code and optimized away, but then it
doesn't matter).
dyn Trait, then the vtable part of the pointer must pointto a valid vtable for
Trait, and the sizeof the entire value (dynamic tail length + statically sized prefix)
must fit in
isize.call, but may panic or otherwise return the wrong value, as the
extern type's layout is not known. This is the same behavior as
[
align_of_val] on a reference to a type with an extern type tail.introduced in the future, the documentation of this function will have to be extended
before it can be used for such types.
Going over the open questions from the tracking issue:
Cc @rust-lang/opsem @rust-lang/lang
(FCP should probably include both teams, unless lang is okay with fully delegating this to t-opsem)
I'll nominate the tracking issue for libs-api so we can get their approval as well for how the operation is exposed.
Fixes #69835