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

Unsafe, optimization-oriented traits for precise sizes, orderings, and so forth #1051

Closed
nikomatsakis opened this issue Apr 9, 2015 · 10 comments
Labels
T-lang Relevant to the language team, which will review and decide on the RFC. T-libs-api Relevant to the library API team, which will review and decide on the RFC.

Comments

@nikomatsakis
Copy link
Contributor

We could sometimes write faster routines (e.g. sort, etc) if they can rely on orderings being sane, iterators having correct lengths. But this makes implementing that ordering/iterator code unsafe. Currently the traits do not require unsafe code.

This issue is a request to extend the set of traits with unsafe variants that let users opt in to these faster implementations. These traits might be generated by derive etc.

RFCs and issues along these lines:

@theemathas
Copy link

FYI, my preferred solution is having an unsafe default associated const such as assume_correct_eq, assume_correct_ord, assume_correct_length that defaults to false (but is true for derive comparisons). Changing them to true should require unsafe code.

Current problems:

  • Requires associated constants
  • Requires unsafe const
  • Requires where Self: Sized on constants
  • Requires default associated constants

@Gankra
Copy link
Contributor

Gankra commented Apr 10, 2015

@theemathas an unsafe method seems strictly better. If it's implemented as just true or false then it will trivially be inlined to a constant, and it allows implementors to make the trusted-ness runtime conditional if need be (for instance if I provide an iterator with a fixed length that is potentially too large to be represented).

Also works with trait objects.

Also would completely work today without all of that extra tooling.

@theemathas
Copy link

@gankro Are you intending to use a static method or a &self method? If it's a static method, I cannot think of any useful runtime checks, but if it's a &self method, it won't work well with Ord and sort

@Gankra
Copy link
Contributor

Gankra commented Apr 11, 2015

@theemathas I was mostly thinking about trusted_len; for trusted_cmp static probably makes more sense.

@pythonesque
Copy link
Contributor

Unsafe methods won't work as intended currently (unless something has changed) because you don't need to write unsafe to implement an unsafe fn in a trait. Of course, this wouldn't be too hard to fix.

@Rufflewind
Copy link

I don't think a new language feature is necessary at all. The proof of a trait impl's law-abiding nature can be encoded as an abstract type that can only be conjured through an unsafe method. Here's a demonstration:

mod my_trait {
    use std::marker::PhantomData;

    pub trait MyTrait {
        /// Is there a witness to the trustworthiness of this trait?
        fn trust() -> Option<TrustMyTrait<Self>> {
            None
        }
    }

    /// Proof that the trait impl of `T` is trustworthy.
    pub struct TrustMyTrait<T: MyTrait + ?Sized>(PhantomData<T>);

    /// Conjure a proof out of thin air (hence the unsafety).
    pub unsafe fn trust_my_trait<T: MyTrait + ?Sized>() -> TrustMyTrait<T> {
        TrustMyTrait(PhantomData)
    }
}

use my_trait::{MyTrait, TrustMyTrait, trust_my_trait};

struct MyType;

impl MyTrait for MyType {
    fn trust() -> Option<TrustMyTrait<Self>> {
         // assert the impl of MyTrait for MyType is correct
         unsafe { Some(trust_my_trait()) }
    }
}

fn main() {
    match <MyType as MyTrait>::trust() {
        None => println!("MyTrait of MyType is not trustworthy :("),
        Some(_) => println!("MyTrait of MyType is trustworthy :D"),
    }
}

@comex
Copy link

comex commented Jan 21, 2017

This is an old thread, and by now Rust already has unsafe trait, which requires an unsafe impl.

(Just for the sake of completeness: your workaround would work for 'simulated' specialization using constant-returning functions plus the optimizer, but it wouldn't work particularly well for the upcoming true (type-system based) trait specialization. This is because the existence of an impl is not enough; since fn trust() could be implemented as a panic or infinite loop, code that relies on the unsafe property must actually call the function before doing anything dangerous. Trait specialization is of course based on the existence of an impl; you could still make sure to call the trust function in all affected code, but this would be easy to mess up, and impossible in some cases, e.g. with associated types or consts.)

@Rufflewind
Copy link

… you don't need to write unsafe to implement an unsafe fn in a trait. Of course, this wouldn't be too hard to fix.

@pythonesque I just tried it on 0.15.0 and it seems to be mandatory. I can't find any documentation to confirm whether this is intended behavior or not, though. If it is, then this proposal can be implemented without any new language features.

@Centril
Copy link
Contributor

Centril commented Feb 23, 2018

Closing as implemented.

@Centril Centril closed this as completed Feb 23, 2018
@Centril Centril added T-lang Relevant to the language team, which will review and decide on the RFC. T-libs-api Relevant to the library API team, which will review and decide on the RFC. labels Feb 23, 2018
@GoldsteinE
Copy link

GoldsteinE commented May 4, 2022

Is there an issue/RFC for unsafe versions of Ord, Eq and Hash? There’s TrustedLen for ExactSizeIterator, but I failed to find traits for #956 in the standard library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the RFC. T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests

8 participants