Add mem::forget_unsized() for forgetting unsized values #55785
Conversation
rust-highfive
added
the
S-waiting-on-review
|
By the way, I hope those |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
I'll admit that I don't really understand the implications of this change. These are two very prominent functions, though, so I'd be wary about exposing functionality which we don't actually want to stabilize. How come these functions are necessary? Is there a way we could expose unstable versions for now? |
Consider how one might allocate an unsized value on the heap: let val: impl ?Sized = ...;
let dest = alloc(Layout::for_value(&val));
ptr::copy_nonoverlapping(&val as *const _ as *const u8, dest, mem::size_of_val(&val));
mem::forget(val); // We need this!As far as I can tell, there is literally no way to forget an unsized local right now.
I'd be okay with having something like |
|
AFAIK calling these with an unsized value is unstable (and we could test that). fn forget_boxed<T: ?Sized>(b: Box<T>) {
std::mem::forget(*b);
}EDIT: this seems to only error on fn drop<T: ?Sized>(_: T) {}
fn drop_boxed<T: ?Sized>(b: Box<T>) {
drop(*b);
}But this does respect the feature-gate, so that seems inconsistent: fn drop_boxed<T: ?Sized>(b: Box<T>) {
{*b};
} |
|
Hm so given @eddyb's discovery, could we perhaps add a separate method for now? The intention can be to remove the unstable separate method before stabilization, but for now it seems best to not tamper too much with the existing functions while the story for unsized locals is still being sorted out |
|
I don't think we should land this (changing #![feature(unsized_locals)]
fn drop<T: ?Sized>(_: T) {} // if you remove this line...
trait A {}
const FOO: fn(dyn A) = drop;
// then you will get: ^^^^ doesn't have a size known at compile-timeI have no objections to doing what @alexcrichton suggests; adding two temporary unstable functions seems fine. |
|
Note that we only need an unstable function for |
|
@eddyb Sounds good! Do I need to open a tracking issue for |
|
I think it's fine to skip it for now. Could the documentation perhaps explicitly say that it's a shim intended to go away once the lang feature is stabilized? I think it's ok to avoid copy/pasting most of the other docs too, the function can just reference the already existing one |
|
@alexcrichton Done! |
ghost
changed the title
|
@bors: r+ |
|
📌 Commit 56d3a82 has been approved by |
bors
added
S-waiting-on-bors
…xcrichton Add mem::forget_unsized() for forgetting unsized values ~~Allows passing values of `T: ?Sized` types to `mem::drop` and `mem::forget`.~~ Adds `mem::forget_unsized()` that accepts `T: ?Sized`. I had to revert the PR that removed the `forget` intrinsic and replaced it with `ManuallyDrop`: rust-lang#40559 We can't use `ManuallyDrop::new()` here because it needs `T: Sized` and we don't have support for unsized return values yet (will we ever?). r? @eddyb
| #[cfg(not(stage0))] | ||
| #[unstable(feature = "forget_unsized", issue = "0")] | ||
| pub fn forget_unsized<T: ?Sized>(t: T) { | ||
| unsafe { intrinsics::forget(t) } |
There was a problem hiding this comment.
Could ManuallyDrop be used in the future?
There was a problem hiding this comment.
Yes, provided we implement support for unsized return values. The reason is that ManuallyDrop::new(t) returns a ManuallyDrop<T>, which is unsized.
There was a problem hiding this comment.
Ah, I see, I was thinking in terms of construction. A pub(crate) ManuallyDrop::forget method that doesn't actually return ManuallyDrop could still work, right?
There was a problem hiding this comment.
Oh wait, you're totally right. I think adding the forget intrinsic was unnecessary since we can do:
pub fn forget_unsized<T: ?Sized>(t: T) {
ManuallyDrop { value: t };
}
There was a problem hiding this comment.
So should we r- then and use that instead? Seems nicer than an intrinsic.
There was a problem hiding this comment.
It seems I was wrong. It's still impossible to initialize structs with unsized fields, so we'll have to live with intrinsics::forget for a while.
| /// Like [`forget`], but also accepts unsized values. | ||
| /// | ||
| /// This function is just a shim intended to be removed when the `unsized_locals` feature gets | ||
| /// stabilized. |
There was a problem hiding this comment.
Removed in favor of making mem::forget support unsized types, I assume?
…xcrichton Add mem::forget_unsized() for forgetting unsized values ~~Allows passing values of `T: ?Sized` types to `mem::drop` and `mem::forget`.~~ Adds `mem::forget_unsized()` that accepts `T: ?Sized`. I had to revert the PR that removed the `forget` intrinsic and replaced it with `ManuallyDrop`: rust-lang#40559 We can't use `ManuallyDrop::new()` here because it needs `T: Sized` and we don't have support for unsized return values yet (will we ever?). r? @eddyb
Rollup of 17 pull requests Successful merges: - #55182 (Redox: Update to new changes) - #55211 (Add BufWriter::buffer method) - #55507 (Add link to std::mem::size_of to size_of intrinsic documentation) - #55530 (Speed up String::from_utf16) - #55556 (Use `Mmap` to open the rmeta file.) - #55622 (NetBSD: link libstd with librt in addition to libpthread) - #55750 (Make `NodeId` and `HirLocalId` `newtype_index`) - #55778 (Wrap some query results in `Lrc`.) - #55781 (More precise spans for temps and their drops) - #55785 (Add mem::forget_unsized() for forgetting unsized values) - #55852 (Rewrite `...` as `..=` as a `MachineApplicable` 2018 idiom lint) - #55865 (Unix RwLock: avoid racy access to write_locked) - #55901 (fix various typos in doc comments) - #55926 (Change sidebar selector to fix compatibility with docs.rs) - #55930 (A handful of hir tweaks) - #55932 (core/char: Speed up `to_digit()` for `radix <= 10`) - #55956 (add tests for some fixed ICEs) Failed merges: r? @ghost
ghost
deleted the
ghost
mentioned this pull request
Allows passing values ofT: ?Sizedtypes tomem::dropandmem::forget.Adds
mem::forget_unsized()that acceptsT: ?Sized.I had to revert the PR that removed the
forgetintrinsic and replaced it withManuallyDrop: #40559We can't use
ManuallyDrop::new()here because it needsT: Sizedand we don't have support for unsized return values yet (will we ever?).r? @eddyb