Skip to content
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

MaybeDangling #3336

Merged
merged 20 commits into from
May 20, 2024
Merged

MaybeDangling #3336

merged 20 commits into from
May 20, 2024

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Oct 29, 2022

Declare that references and Box inside a new MaybeDangling type do not need to satisfy any memory-dependent validity properties (such as dereferenceable and noalias).

Rendered

Thanks to @rust-lang/wg-unsafe-code-guidelines for a lot of helpful feedback :)

Tracking:

@RalfJung RalfJung changed the title add maybe-dangling RFC MaybeDangling Oct 29, 2022
@CAD97
Copy link

CAD97 commented Oct 30, 2022

Does MaybeDangling have any impact on lifetimes? I assume not, but it's unfortunately close to #[may_dangle] (the dropck eyepatch) which does.

Some naming alternatives

Only slightly tongue-in-cheek:

  • Dormant, Inactive, Suspended, etc — the point is to turn off enforcement of the type's "special" memory related properties. Validity is still required, but I think this communicates the intent of only enforcing the "special" properties lazily when used.

    These names might much more strongly want access to the wrapped type to be explicit (but still safe) rather than through Deref, though, as they're much more "active" in what they're saying.

  • Launder — like C++'s std::launder, it's about hiding properties from the compiler, and like std::launder, it's a very necessary tool that nonetheless is only needed in a small set of low level building blocks.

These names also have the advantage of not implying they only impact dangling/dereferenceable but not uniqueness/noalias.


I also find it somewhat amusing that this kind of ends up introducing a new pointer type. MaybeDangling<&'a [mut] T> offers some interesting opportunities to replace some NonNull<T> usage.

@ehuss ehuss added the T-lang Relevant to the language team, which will review and decide on the RFC. label Oct 30, 2022
@RalfJung
Copy link
Member Author

Does MaybeDangling have any impact on lifetimes? I assume not, but it's unfortunately close to #[may_dangle] (the dropck eyepatch) which does.

Oh... yeah that is not great. :/

I also find it somewhat amusing that this kind of ends up introducing a new pointer type. MaybeDangling<&'a [mut] T> offers some interesting opportunities to replace some NonNull usage.

This is still quite different from NonNull. MaybeDangling<&mut T> is not Copy and each time you use it, that will create a mutable reference (via the DerefMut implementation) which comes with all its usual aliasing gurantees!

NonNull is suited for actually aliasing data. MaybeDangling<&mut T> is suited for cases where the reference is "on hold" and aliases exist but the reference will only be used again once the aliases are gone.

Dormant, Inactive, Suspended, etc — the point is to turn off enforcement of the type's "special" memory related properties. Validity is still required, but I think this communicates the intent of only enforcing the "special" properties lazily when used.

I like those, they match the previous sentence about being "on hold".

These names might much more strongly want access to the wrapped type to be explicit (but still safe) rather than through Deref, though, as they're much more "active" in what they're saying.

Hm... yes maybe? No strong opinion here; I assumed it'd be Deref because ManuallyDrop is.

@Pointerbender
Copy link

These names might much more strongly want access to the wrapped type to be explicit (but still safe) rather than through Deref, though, as they're much more "active" in what they're saying.

Hm... yes maybe? No strong opinion here; I assumed it'd be Deref because ManuallyDrop is.

This is a good observation, because the documentation of Deref says the following:

Implementing Deref for smart pointers makes accessing the data behind them convenient, which is why they implement Deref. On the other hand, the rules regarding Deref and DerefMut were designed specifically to accommodate smart pointers. Because of this, Deref should only be implemented for smart pointers to avoid confusion.

Note that ManuallyDrop is not always wrapped around a smart pointer! Looking back at its RFC this was not brought up then. MaybeDangling<..> does have a use case for wrapping Box<T> and &mut T, so here it less in conflict with the documentation of Deref, but still it could wrap a non-pointer type (this is not of any practical use, though).

Should MaybeDangling implement Deref and DerefMut like ManuallyDrop does, or should accessing the inner data be more explicit since that is when the aliasing and validity requirements do come back in full force?

As mentioned in the RFC, the difference between ManuallyDrop and MaybeDangling is that the latter is intended for maybe-aliasing capabilities, in addition to both supporting maybe-dangling properties. This maybe-aliasing property being a potential mild footgun for safe Rust code implicitly dereferencing an aliasing MaybeDangling, combined with the documentation of Deref, seem to suggest that MaybeDangling might be better off not implementing Deref and expose explicit methods for that instead. Or as an alternative, it could only implement Deref<Target=T::Target> when T also implements Deref to stay as close to the documentation of Deref as possible, but this could also hurt composability of MaybeDangling<T> and does not remove the footgun, so that is not ideal either. Finally, whether MaybeDangling implements Deref or not would also have consequences for whether its methods could be implemented as regular methods or as associated functions, so there is a UX aspect to this, as well.

I also don't have a strong opinion here, but it seemed worthwhile to at least consider this for the decision for whether to implement Deref or not :)

@comex
Copy link

comex commented Nov 2, 2022

MaybeDangling<&mut T> is not Copy

To clarify, is MaybeDangling<T> Copy if T is?

@RalfJung
Copy link
Member Author

RalfJung commented Nov 2, 2022

To clarify, is MaybeDangling Copy if T is?

Yes, it propagates all the auto traits and Copy. It should probably derive(Copy, Clone, Debug) and maybe more.

@CAD97
Copy link

CAD97 commented Nov 2, 2022

(@Pointerbender FWIW the "for smart pointers" wording has been considered for removal a few times before, and still is. The general consensus is more along the lines of "when &Self should be implicitly usable as &Target" plus guidelines around avoiding method conflicts. In addition to ManuallyDrop, std also has the unstable LazyLock/LazyCell which aren't pointers persay but do implement Deref.)

Co-authored-by: teor <teor@riseup.net>
@RalfJung
Copy link
Member Author

RalfJung commented Nov 5, 2022

A potential future possibility with this RFC is to use it to resolve the self-referential-generator-aliasing situation: these generators need to be pinned anyway, and if we define Pin<T> as a newtype around Pin<MaybeDangling<T>>, then Pin<&mut T> no longer makes any aliasing assumptions so we wouldn't need anything else (such as the current special treatment of Unpin) to make self-referential generators sound.

@CAD97
Copy link

CAD97 commented Nov 5, 2022

That's not fully sufficient by itself; you still need some way to do pin projection. Currently that's done with the _unchecked methods which require using the actual &mut.

@RalfJung
Copy link
Member Author

RalfJung commented Nov 5, 2022

Hm... yeah you are right, those &mut are short-lived but they do add intermediate noalias assumptions.

@Pointerbender
Copy link

Do the effects of MaybeDangling also recurse into the pointee and/or pointee's fields? E.g. will MaybeDangling<Box<Box<&mut T>>> also strip the &mut T from its memory-dependent validity obligations, or would that require a MaybeDangling<Box<Box<MaybeDangling<&mut T>>>> to that effect?

@RalfJung
Copy link
Member Author

RalfJung commented Nov 6, 2022

A Box<Box<&mut T>> doesn't have aliasing requirements on the inner pointers to begin with, so there's nothing there for MaybeDangling to strip.

text/0000-maybe-dangling.md Outdated Show resolved Hide resolved
Co-authored-by: bl-ue <54780737+bl-ue@users.noreply.github.com>
@kornelski
Copy link
Contributor

kornelski commented Nov 7, 2022

Perhaps making ManuallyDrop more magical would suffice?

It makes sense to me that an object with dangling references is dropped manually, because user of that type already has to be careful about when it is dangling (e.g. drop order of fields in self-referential structs is important).

In the worst case it should be possible to make a DIY MaybeDangling with AutomaticallyDrop<T>(ManuallyDrop<T>) with a Drop.

@RalfJung
Copy link
Member Author

RalfJung commented Nov 7, 2022

The alternative is mentioned in the RFC. The RFC also has 2 use-cases for MaybeDangling that want automatic drop, so it seems better to have ManuallyDrop use a MaybeDangling type rather than awkwardly implementing a dropping MaybeDangling on top of ManuallyDrop.

@tmandry
Copy link
Member

tmandry commented Nov 15, 2023

Separately, I think we should also just move forward with this RFC, given the argument made in the RFC (and by @scottmcm in the aforementioned lang team meeting) that we will need a wrapper type for this even if we also have an attribute for it.

@rfcbot reviewed

@RalfJung
Copy link
Member Author

I have created an issue to track the experiment: rust-lang/rust#118166. Looking for someone to implement the type and its magic behavior now. :)

@danielhenrymantilla
Copy link

Btw, the RFC could nowadays mention https://docs.rs/maybe-dangling as a third-party implementation thereof

Notice that `UnsafeCell` acts "behind references" while `MaybeDangling`, like `MaybeUninit`, acts "around references": `MaybeDangling<&T>` vs `&UnsafeCell<T>`.

There is a [crate](https://docs.rs/maybe-dangling) offering these semantics on stable Rust via `MaybeUninit`.
(This is not "prior" art, it was published after this RFC came out. "Related work" would be more apt. Alas, the RFC template forces this structure on us.)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extreme pedantry: arguably, "prior" in "prior art" as used by the RFC template means "prior to when this RFC is accepted." The history of past revisions of the RFC document is not part of the final product that gets FCPed and published in the RFC Book. So "prior art" is an accurate description of https://docs.rs/maybe-dangling, and the RFC template is perfectly innocent here.

Copy link
Member Author

@RalfJung RalfJung Dec 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not how the term "prior art" is usually used in scientific context. When there is a causal dependency where A caused B, then B cannot be prior art of A, even if the publication of paper A happens after B becomes public. (There is often a significant delay between writing and submitting a paper, and having it published.) Here, as far as I can see, the RFC inspired that crate, so if anything the RFC is prior art for the crate, not vice versa.

Alas, I have several gripes with the strict form of the template, but not the patience to try and fix it.^^

Copy link
Contributor

@Jules-Bertholet Jules-Bertholet Dec 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way I see it, the initial draft of the RFC inspired the crate, which in turn inspired a subsequent draft. But that subsequent draft should be considered a distinct document from the initial draft. Published RFCs, unlike scientific papers, are considered as emanating from the team that approved them, and represent the perspective of that team at the time of FCP approval (the original author is not even listed in the RFC Book).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree, but it doesn't seem useful to discuss this here.

# Reference-level explanation
[reference-level-explanation]: #reference-level-explanation

The standard library contains a type `MaybeDangling<P>` that is safely convertible with `P` (i.e., the safety invariant is the same), and that has all the same niches as `P`, but that does allow passing around dangling boxes and references within unsafe code.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is it possible to have both (1) MaybeDangling<P> and P are safely convertible and (2) P holds more guarantees than MaybeDangling<P>? If they are convertible, one could safely pass say MaybeDangling<&'a mut T> to a fn(&'a mut T, ...) which itself is compiled with noalias assumption, and result in undefined behaviors. If P -> MaybeDangling<P> is safe and MaybeDangling<P> -> P is unsafe, then what's the difference between MaybeDangling and MaybeUninit?

I also disagree with the disagreement by @RalfJung 's #3336 (comment):

MaybeDangling as currently specified by the RFC does not change safety invariants, as it implements safe >> Deref conversion like ManuallyDrop does. I argue that this is likely undesirable, and MaybeDangling should instead make it unsafe to reactivate the wrapped type and its invariants.

I think that would make MaybeDangling mostly useless -- one can already use MaybeUninit for something like that. I consider not having to use unsafe code to get the inner data back out of the no-dangling wrapper to be a feature. For example, this would have avoided adding unsafe code in rust-lang/rust#102589.

No. It's intrinsically unsafe to have fn(SomeType<'some_life>) where 'some_life may ends in the middle of the function invocation depending on its logic. In that case (example 2 of this RFC), the validity is controlled by the runtime logic and is impossible to check at compile time (eg. cannot prevent you from "accidentally" swapping closure(); and control.signal_done();), thus guarantees should be made by programmer, unsafe is required, with or without MaybeDangling. Then I don't really see the advantage of introducing another MaybeDangling than MaybeUninit.

The dereferenceability of ManuallyDrop in example 1, I think, is a separate issue. It's a choice between UB on state observed and UB on state exists.

Copy link
Member Author

@RalfJung RalfJung Dec 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is it possible to have both (1) MaybeDangling<P> and P are safely convertible and (2) P holds more guarantees than MaybeDangling<P>?

There are validity invariants and safety invariants. The safety invariant of P and MaybeDangling<P> are identical, but the validity invariants are not.

Then I don't really see the advantage of introducing another MaybeDangling than MaybeUninit.

MaybeUninit is a terrible solution for example 2 since one has to call drop manually. That's fragile and error-prone. I think it is important that we have a way to not lose automatic drop but still pass around "opaque arbitrary data" without making extra claims about its memory validity.

Once we have implicit drop, there's no reason to make into_inner unsafe, since drop will already implicitly convert to the inner type without unsafe code.

@RalfJung
Copy link
Member Author

Around half a year ago, this was proposed to be merged so we can do experimentation. @joshtriplett @nikomatsakis @pnkfelix any chance you could tick your box? :)

@nikomatsakis
Copy link
Contributor

@rfcbot reviewed

Thanks for the ping, @RalfJung.

@rfcbot rfcbot added the final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. label Apr 29, 2024
@rfcbot
Copy link
Collaborator

rfcbot commented Apr 29, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. label Apr 29, 2024
To handle situations like this, Rust has a special type called `MaybeDangling<P>`:
references and boxes in `P` do *not* have to be dereferenceable or follow aliasing guarantees.
This applies inside nested references/boxes inside `P` as well.
They still have to be non-null and aligned, and it has to at least be *possible* that there exists valid data behind that reference (i.e., `MaybeDangling<&!>` is still invalid).
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MaybeDangling<&!> is still invalid

Based on my current understanding of ref-to-never, this can change to "MaybeDangling<&!> has an always-violated safety invariant"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

&! has an impossible validity invariant according to the Reference, Miri, and MiniRust. So I don't think the text should change.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this RFC. and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels May 9, 2024
@rfcbot
Copy link
Collaborator

rfcbot commented May 9, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

The FCP for RFC 3336 has completed.  Let's prepare it to be merged.
@traviscross traviscross merged commit e4bff82 into rust-lang:master May 20, 2024
@traviscross
Copy link
Contributor

The lang team has accepted this RFC and we've now merged it.

Thanks to @RalfJung for pushing forward this important work, and thanks to all those who reviewed this and offered helpful feedback.

For further updates, follow the tracking issue:

@@ -3,7 +3,7 @@
- Feature Name: `maybe_dangling`
- Start Date: 2022-09-30
- RFC PR: [rust-lang/rfcs#3336](https://github.com/rust-lang/rfcs/pull/3336)
- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000)
- Tracking Issue: [rust-lang/rust#118166](https://github.com/rust-lang/rust/issues/118166)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about also fixing the template which currently still reads "Rust Issue:" here

@RalfJung RalfJung deleted the maybe-dangling branch May 29, 2024 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this RFC. T-lang Relevant to the language team, which will review and decide on the RFC. to-announce
Projects
None yet
Development

Successfully merging this pull request may close these issues.