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

Tracking Issue for ops::Residual (feature try_trait_v2_residual) #91285

Open
1 of 3 tasks
Tracked by #1568
scottmcm opened this issue Nov 27, 2021 · 9 comments
Open
1 of 3 tasks
Tracked by #1568

Tracking Issue for ops::Residual (feature try_trait_v2_residual) #91285

scottmcm opened this issue Nov 27, 2021 · 9 comments
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. F-try_trait_v2 Tracking issue for RFC#3058 T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@scottmcm
Copy link
Member

scottmcm commented Nov 27, 2021

Feature gate: #![feature(try_trait_v2_residual)]

This is a tracking issue for the ops::Residual trait.

This is used by try_* APIs that need to change from one member of a family to another, such as

For example, the closure passed to Iterator::try_find returns Foo<bool>, but the method wants to be able to return Foo<Option<<Self as Iterator>::Item>>.

Public API

// ops::Residual

trait Residual<O> {
    type TryTrait: Try<Output = O, Residual = Self>;
}

// with implementations for `Result`, `Option`, and `ControlFlow`.

Steps / History

Unresolved Questions

@scottmcm scottmcm added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. labels Nov 27, 2021
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 3, 2021
Make `array::{try_from_fn, try_map}` and `Iterator::try_find` generic over `Try`

Fixes rust-lang#85115

This only updates unstable functions.

`array::try_map` didn't actually exist before; this adds it under the still-open tracking issue rust-lang#79711 from the old PR rust-lang#79713.

Tracking issue for the new trait: rust-lang#91285

This would also solve the return type question in for the proposed `Iterator::try_reduce` in rust-lang#87054
@BGR360
Copy link
Contributor

BGR360 commented Dec 26, 2021

@rustbot label +F-try_trait_v2

@rustbot rustbot added the F-try_trait_v2 Tracking issue for RFC#3058 label Dec 26, 2021
@ghost
Copy link

ghost commented Nov 6, 2022

Now that GATs are stable, would it be possible to move the generic to the associated type? It would eliminate the need for ChangeOutputType and simplify usage.

trait Residual {
    type TryType<O>: Try<Output = O, Residual = Self>;
}
// core::array

fn try_from_fn<T, const N: usize, R, F>(f: F) -> R::TryType<[T; N]>
where
    R: Residual,
    F: FnMut(usize) -> R::TryType<T>,
{ .. }
// core::array

fn try_map<U, R, F>(self, f: F) -> R::TryType<[U; N]>
where
    R: Residual,
    F: FnMut(T) -> R::TryType<U>,
{ .. }

@danielhenrymantilla
Copy link
Contributor

danielhenrymantilla commented Nov 7, 2022

Yeah, the current Residual trait reads rather awkwardly due to that generic O parameter it must be fed eagerly:

fn try_from_fn<R, const N: usize, F>(cb: F)
  -> ChangeOutputType<R, [R::Output; N]>
where
    F : FnMut(usize) -> R,
    R : Try,
    R::Residual : Residual<[R::Output; N]>,

Notice that : Residual<[R::Output; N]> clause. It can't really be spelled out/read out loud in English: "is a residual of an array / can residual an array" doesn't make that much sense.

In a way, the type alias internally used by the stdlib already provides a way nicer name: ChangeOutputType.

So in this case, taking that name and using it on the trait itself, we would rather imagine:

R::Residual : CanWrapOutputType<[R::Output; N]>,
  • (or {Feed,Apply,Change,Set,With}OutputType)

Now we get something that does read better, although it talks a bit too much about type-level operations, which is something the stdlib doesn't do that often.

So, rather than focusing on the how, if we focus on the what instead, we can stick to the "being a Residual" property, but this time with no generic Output parameter yet, by delegating it to a GAT, which features the proper quantification of "it can be fed any output type":

R::Residual : Residual, // <- Bonus: this wouldn't even be needed, since now we could eagerly add this bound to `Try`!

and then using <R::Residual as Residual>::TryType<[T; N]>.

  • Or even TryTypeWithOutput<[T; N]>. I'll be using this new name for the remainder of the post

Notice that bonus of being able to eagerly require that Try::Residual types always implement Residual, which we can't do with the current design since that would need a for<Output> kind of quantification. EDIT: not all Residuals / Try types may want to be able to wrap any kind of T, as pointed out by @h4x5 in the comment just below.

I think it would be confusing to have some Try types not be usable with array_from_fn just because of the Residual associated type happens not to meet part of the implicitly required API contract. Having it explicitly required seems like a definite win, in that regard.


A technical remark, however, @h4x5: we can't use Res : Residual and then an impl Fn… -> Res::TryTypeWithOutput<T> closure, and then expect Res to be inferrable from context. Indeed, we'd have a "preïmage" situation regarding the type FeedT<Res> = Res::TryType<T>; operation, which is not an injective one, and thus won't be solvable by type inference.

So, while keeping Res seems convenient for the sake of the signature (Res::TryType<…>), we'd have to instead say that the output of the closure implements Try<Residual = R>:

fn try_from_fn<T, const N: usize, F, Ret, Res>(f: F)
  -> Res::TryTypeWithOutput<[T; N]>
where
    F : FnMut(usize) -> Ret,
    Ret : Try<Output = T, Residual = Res>,
    Res : Residual, // EDIT
  • Notice how now we are getting the associated "image"/type from Ret to Ret::Residual in order to figure out Res.

Or we could get rid of that Res altogether:

fn try_from_fn<T, const N: usize, R, F>(f: F)
// -> <R::Residual as Residual>::TryTypeWithOutput<[T; N]>
   -> ChangeOutputType<R, [T; N]>
where
    F : FnMut(usize) -> R,
    R : Try<Output = T>,
    R::Residual : Residual, // EDIT

@ghost
Copy link

ghost commented Nov 7, 2022

Thanks! I'll try and draft a PR today to change it.

Side note:
This trait really only applies to the generic container Try types (Result, Option, ControlFlow, etc), not to the simpler, non-generic ones like the RFC's ResultCode example. Thus, adding Try::Residual: Residual would be overly restrictive and disallow using try with types like ResultCode.

@danielhenrymantilla
Copy link
Contributor

Oh, good catch! (I've edited my post accordingly).

  • 🤔 In that case I very much feel like that trait ought not to be named Residual. Given R : Try, then having R::Residual : Residual not hold by default and have to be explicitly spelled out feels quite weird, tbh. We'd be back to ChangeOutput or things along those lines, etc.

@ghost ghost mentioned this issue Nov 7, 2022
@ghost
Copy link

ghost commented Nov 7, 2022

I've submitted #104128, an attempt at changing this.

@scottmcm
Copy link
Member Author

scottmcm commented Nov 9, 2022

(See some additional conversation on zulip: https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/lib.20after.20GATs/near/308206137)

The thing I'm worried about using a GAT here is that it makes a bunch of things not work that would work without the GAT. The problem is that the GAT is the equivalent of a for<T> R: Residual<T>. But that's a very strong promise. It means, for example, that you wouldn't be able to have a Try type that would work only with Output types that are Copy.

Imagine a Rust type for an HRESULT. That might look something like this:

#![feature(try_trait_v2_residual)]
#![feature(try_trait_v2)]
use std::marker::PhantomData;
use std::ops::{ControlFlow, Try, Residual, FromResidual};

mod hacks {
    pub trait TypeThatWorksInHResult {
        fn into_u32(self) -> u32;
        fn from_u32(x: u32) -> Self;
    }
    impl TypeThatWorksInHResult for () {
        fn into_u32(self) -> u32 { 0 }
        fn from_u32(x: u32) { debug_assert!(x == 0); }
    }
    impl TypeThatWorksInHResult for bool {
        // `S_FALSE` is `1`: <https://referencesource.microsoft.com/#windowsbase/Base/MS/Internal/Interop/ErrorCodes.cs,e08462dc3482f421>
        fn into_u32(self) -> u32 { if self { 0 } else { 1 } }
        fn from_u32(x: u32) -> bool { debug_assert!(x <= 1); x == 0 }
    }
    impl TypeThatWorksInHResult for u8 {
        fn into_u32(self) -> u32 { self as _ }
        fn from_u32(x: u32) -> u8 { debug_assert!(x <= 0xFF); x as _ }
    }
    impl TypeThatWorksInHResult for u16 {
        fn into_u32(self) -> u32 { self as _ }
        fn from_u32(x: u32) -> u16 { debug_assert!(x <= 0xFFFF); x as _ }
    }
}

#[repr(transparent)]
pub struct HResult<T: hacks::TypeThatWorksInHResult>(u32, PhantomData<T>);
pub struct HResultResidual(u32); // TODO: use `NegativeI32`, once possible

impl<T: hacks::TypeThatWorksInHResult> Try for HResult<T> {
    type Output = T;
    type Residual = HResultResidual;
    
    fn from_output(x: T) -> Self {
        Self(x.into_u32(), PhantomData)
    }
    fn branch(self) -> ControlFlow<HResultResidual, T> {
        if (self.0 as i32) < 0 {
            ControlFlow::Break(HResultResidual(self.0))
        } else {
            ControlFlow::Continue(T::from_u32(self.0))
        }
    }
}

impl<T: hacks::TypeThatWorksInHResult> FromResidual for HResult<T> {
    fn from_residual(r: HResultResidual) -> Self {
        Self(r.0, PhantomData)
    }
}

impl<T: hacks::TypeThatWorksInHResult> Residual<T> for HResultResidual {
    type TryType = HResult<T>;
}

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=93e6c13eea6bf3b93683576d6aa79f3a

But that Residual impl can't turn into a GAT, because it's not ∀T, but only for some Ts. And there's no syntax that exists to restrict the GAT in an impl -- the restrictions need to be in the trait, where there wouldn't be any for this.

Thoughts?

@ghost
Copy link

ghost commented Nov 9, 2022

Yeah. I was thinking about this more, and it did seem overly restrictive, but I couldn't think of a use case for Try::Output: Trait.

For now, sticking with the non-GAT design seems like the best option.

@discreaminant2809
Copy link

The Residual type mapping for Poll seems weird for me :

#![feature(iterator_try_collect, array_try_from_fn)]

use std::{array, task::Poll};

fn main() {
    let arr = [
        Poll::Ready(Ok(3)),
        Poll::Pending,
        Poll::Ready(Ok(3)),
        Poll::Ready(Err("NaN")),
        Poll::Ready(Ok(4)),
    ];

    // Why not `Poll<Result<Vec<i32>, &str>>`?
    let _v: Result<Vec<Poll<i32>>, &str> = arr.into_iter().try_collect::<Vec<_>>();
    // Why not `Poll<Result<[i32; 5], &str>>`?
    let _arr: Result<[Poll<i32>; 5], &str> = array::try_from_fn(|i| arr[i]);
}

It is because the Try implementations for Poll<Result<T, E>> and Poll<Option<Result<T, E>>> are weird also. Their Residuals are both Result<Infallible, E>, which causes the weirdness. But since the implementations are exposed for stable, I think we can only do something withResidual trait? Or the two try_ methods/functions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. F-try_trait_v2 Tracking issue for RFC#3058 T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants