-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Add function Arc/Rc::as_weak(…)
to convert &Arc/Rc
to &Weak
#100472
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
r? @m-ou-se (rust-highfive has picked a reviewer for you, use r? to override) |
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
73143bf
to
95a76c0
Compare
Aside from needing a proper tracking issue (which shouldn't be created before this is given the okay), the diff LGTM. |
…d points to the right address.
9c5fd1c
to
876236c
Compare
struct SomeStruct(PhantomPinned);
let pinned = Rc::pin(SomeStruct(PhantomPinned));
// This is unsound !!!
let weak = Rc::as_weak(pinned.as_ref());
// ... because it would be possible to move the content of pinned:
let mut unpinned_rc = weak.upgrade().unwrap();
std::mem::drop(pinned);
// unpinned_rc is now the only reference so this will work:
let x = std::mem::replace(
Rc::get_mut(&mut unpinned_rc).unwrap(),
SomeStruct(PhantomPinned),
); Edit: false alarm. This doesn't work. |
@ogoffart you may be still editing your example, but |
An interesting phenomenon is that this can let |
@cuviper ah right, my mistake. Then that's a false alarm. Sorry for the noise. |
Hmm, that's a good point. I guess we could limit When I have time I think I'll take a look through crates that are currently using |
…count() result in the documentation
Of course, if you care about the existence of strong pointers, you can use The closest thing I can find is fn is_unique(this: &Self) -> bool {
Rc::weak_count(this) == 0 && Rc::strong_count(this) == 1
} but that kind of check doesn't make sense for |
Looking through this crates.io package search, the uses of
As far as I can tell, none of these crates could be broken by this change. |
☔ The latest upstream changes (presumably #89132) made this pull request unmergeable. Please resolve the merge conflicts. |
For (For that specific algorithm we could still make it work by doing 1-2 atomic operations in For |
@jeremyBanks FYI: when a PR is ready for review, send a message containing |
Closing this as inactive. Feel free to reöpen this pr or create a new pr if you get the time to work on this. Thanks |
I would like to propose the addition of a new
as_weak
associated function onArc
andRc
, taking a shared reference to a strongArc
/Rc
and transmuting it into a shared reference to the correspondingWeak
type.Currently, if you have a strong
Arc
/Rc
and you're calling a function that expects a&Weak
, you'll need to.downgrade()
to create a temporaryWeak
, incurring two additional writes to the backing allocation. This could be avoided if it were possible to convert an&Arc
/&Rc
to a&Weak
directly, with a function like this:In memory,
Arc
/Rc
andsync::Weak
/rc::Weak
are both represented by aNotNull
pointer to the backingArcInner
/RcBox
where the reference counts and inner value are actually stored. Whether a reference is strong or weak exists only at the type level, and the static guarantees provided byWeak
(that the pointed-to allocation will exist, unless the pointer has the special non-alignedWeak::new
value) are strictly weaker than those provided byArc
/Rc
(that the pointed-to allocation will exist full-stop, and that the value inside that allocation will still be valid/will not have been dropped yet). TheArc
/Rc
do have aPhantomData<T>
that theWeak
s do not, but it's zero-size and shouldn't affect layout, only drop behaviour (which isn't relevant since the proposed function is for borrowed values).This requires the addition of the
#[repr(transparent)]
attribute toArc
/Rc
, andWeak
in order to guarantee their memory layouts are identical. (#[repr(C)]
might also work for that purpose, but I'm not sure if that would break theNonZero
niche optimization.) According to the discussions at #72841 (review), adding the#[repr]
does not constitute a public interface change/commitment because the internal fields are still private (and with #90435 such#[repr]
s may be hidden from the docs in the future). So even if the#[repr(transparent)]
were added, this function still couldn't be implemented in an external crate as the crate wouldn't be able to soundly rely on the layout remaining the same; this function can only be implemented in the standard library.This implementation is gated behind a new
#![feature(rc_as_weak)]
.previous discussion: Rust Internals thread #17171, Stack Overflow question #73314967