rust-lang / rust-lang/libs-team

ACP: add `unsafe trait PureRef<U>` to guarantee purity of the AsRef/Deref/Borrow traits

Open
#827 18 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

api-change-proposal
Dominant language
Rust
Stars
178
Forks
28
Avg merge
15m
Merged PRs (30d)
1

Description

Proposal

Problem statement

Several times I came across a case where some unsafe code wants to take a generic implementing AsRef or its "friends", allowing to call it with different types but to do that soundly it'd have to require that those implementations are pure - that is the pointer they return always points to the same value (maybe moved) unless some other method modified it. This looks like requiring StableDeref would work except that StableDeref requires that the pointer stays the same after move while this is not really always required by unsafe code and is overly restrictive.

Motivating examples or use cases

Let's say you have a struct:

struct Initializer<B: AsRef<[MaybeUninit<u8>]> + AsMut<[MaybeUninit<u8>]>> {
    bytes: B,
    filled_up_to: usize,
    // maybe more stuff
}

One would want to soundly write unsafe code with safe API that writes the bytes and later reads them however AsRef + AsMut alone is unsound because today it's allowed for the safe code to return a different reference on each call. The genericity of the struct is useful because it can serve both the cases when the caller created Initializer on the stack and wants to reuse it and the case when it needs to be owned (in a Box).

Solution sketch

/// Guarantees that the reference casting implementations are pure.
///
/// This trait may be implemented on a type `T` if it implements at least one of these traits:
///
/// * `AsRef<U>`
/// * `AsMut<U>`
/// * `Borrow<U>`
/// * `BorrowMut<U>`
/// * `Deref<Target = U>`
/// * DerefMut<Target = U>`
///
/// and **every** implementation of those has these properties:
/// * for any given reference passed to any of the respective method of these traits the same reference is returned after each call unless the object behind the reference was modified by some method that is not a method of these traits or the object was moved
/// * if a reference returned after the input reference moved is different then it still has to point to the same value that it pointed before, just at different location (also moved)
/// * the methods with different mutability return a reference pointing to the same value (having the same address), just differ in mutability
/// * the methods with the same mutability from different traits are interchangeable and do the same thing
///
/// Note: the methods do not need to be strictly, truly pure in the sense of not having side effects however it's not advisable for them to have side effects unless it's for temporary debugging (e.g. printing to stderr).
///
/// For example, this code:
///
/// ```
/// foo(bar.as_ref());
/// foo(bar.as_ref());
/// ```
///
/// where the type of `bar` implements `PureRef<WhatFooAccepts>` must behave the same (modulo unrelated side effects) as this code:
///
/// ```
/// let bar = bar.as_ref();
/// foo(bar);
/// foo(bar);
/// ```
///
/// And this code:
///
/// ```
/// foo(bar.as_ref());
/// foo(bar.borrow());
/// ```
///
/// And this code:
///
/// ```
/// foo(bar.as_ref());
/// foo(bar.as_mut()); // &mut -> & coercion
/// ```
///
/// This applies for any combination of methods and mutabilities
///
/// Implementing this trait if those methods don't behave this way is not an immediate UB but an API unsoundness that can cause UB in safe code.
///
/// Note: it is generally advisable to follow these rules even for types that don't implement this trait as that is what most people expect and breaking them may lead to confusion or bugs. But only this trait guarantees that `unsafe` code is allowed to depend on it for soundness.
// probably in the core::marker module
pub unsafe trait PureRef<U> {}

Alternatives

  • Have only StableDeref - this prevents structs accepting both T and &T which I think is quite annoying limitation
  • Have it as crate on crates.io - while technically possible, this looks like a very fundamental property that needs to be communicated across crates and I believe it really belongs to core. It'd be like having Send or Sync in external crates - also technically possible (aside from auto trait stability and thread APIs in std) but would be super-annoying. The trait is also fairly trivial so I don't expect it to be a huge maintenance burden.
  • Each crate defines its own - this is annoying as everyone has to reinvent the wheel
  • Other trait design, such as having the method on the trait - I don't see how this could have any advantage and it'd require more boilerplate. Also, I don't think the type parameter can be soundly removed because without the parameter it'd apply to all implementations including from other crates the implementor has no control over.
  • Just wait for the custom projections to be implemented - while custom projections are really awesome (and huuuuge thanks to everyone working on them if you read this!) I think they will take a long time to stabilize and it might be complicated to use them. This trait is stupid-simple, easy to add and even if it got replaced by projections eventually it wouldn't be a huge dead weight on the library. Besides, it might still be useful with projections - such as AsRef without Deref (things like AsRef<[u8]> for str where projections may be undesirable).

Links and related work

The StableDeref trait already exists as a crate: https://docs.rs/stable_deref_trait/latest/stable_deref_trait/ but note that I've seen proposals to put it into the standard library, I can't remember where.
After writing all this I also found that DerefPure already exists and is almost the same just applies to Deref only and perma-unstable: https://doc.rust-lang.org/stable/std/ops/trait.DerefPure.html
I think mine is superior and perhaps could be used where DerefPure is used now. And, obviously, I would want the trait to be stable.

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

  • We think this problem seems worth solving, and the standard library might be the right place to solve it.
  • We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

  • We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
  • We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the ACP proposal and review the existing StableDeref crate and the standard library's DerefPure documentation linked in the issue. Follow the libs-api feature lifecycle and comment discussion before proposing implementation details. Done would require an approved design for PureRef and a subsequent implementation path.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend-api-design
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.