Open scottmcm opened 2 years ago
@rustbot label +F-try_trait_v2
Now that GATs are stable, would it be possible to move the generic to the associated type? It would eliminate the need for and simplify usage.ChangeOutputType
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>,
{ .. }
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]>,
{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]>
.
TryTypeWithOutput<[T; N]>
. I'll be using this new name for the remainder of the postNotice that bonus of being able to eagerly require that EDIT: not all 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.Residual
s / 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
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
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
.
Oh, good catch! (I've edited my post accordingly).
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.I've submitted #104128, an attempt at changing this.
(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>;
}
But that Residual
impl can't turn into a GAT, because it's not ∀T
, but only for some T
s. 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?
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.
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 Residual
s 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?
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 asFor example, the closure passed to
Iterator::try_find
returnsFoo<bool>
, but the method wants to be able to returnFoo<Option<<Self as Iterator>::Item>>
.Public API
Steps / History
Unresolved Questions
Try::Residual
is renamed (an open question in #84277), this this probably should be too.