tamer: iter::trip: Flatten Result

The `*_iter_while_ok` functions now compose like monads, flattening `Result`
at each step and drastically simplifying handling of error types.  This also
removes the bunch of `?`s at the end of the expression, and allows me to use
`?` within the callback itself.

I had originally not used `Result` as the return type of the callback
because I was not entirely sure how I was going to use them, but it's now
clear that I _always_ use `Result` as the return type, and so there's no use
in trying to be too accommodating; it can always change in the future.

This is desirable not just for cleanup, but because trying to refactor
`asg_builder` into a pair of `Parser`s is really messy to chain without
flattening, especially given some state that has to leak temporarily to the
caller.  More on that in a future commit.

DEV-11864
main
Mike Gerwitz 2022-05-20 15:43:49 -04:00
parent 958a707e02
commit f218c452b9
7 changed files with 88 additions and 65 deletions

View File

@ -53,6 +53,7 @@ mod trip {
bench.iter(|| {
into_iter_while_ok(data.clone(), |iter| {
iter.for_each(drop);
Result::<_, &str>::Ok(())
})
});
}

View File

@ -88,7 +88,7 @@ pub fn main() -> Result<(), TamecError> {
|toks| {
toks.write(&mut fout, Default::default(), &escaper)
},
)??;
)?;
Ok(())
})

View File

@ -86,6 +86,8 @@
//! assert_eq!(None, iter.next());
//! assert_eq!(None, iter.next());
//! assert!(iter.is_tripped());
//!
//! Ok(())
//! });
//!
//! // The error that caused the trip is returned.

View File

@ -107,27 +107,13 @@ pub trait TryCollect: Iterator + Sized {
/// This function exists purely to improve readability and reduce
/// boilerplate,
/// but at the expense of a somewhat confusing return type.
///
/// The outer [`Result`] represents the the source iterator,
/// in the sense of [`TrippableIterator::while_ok`].
/// The inner [`Result`] represents the result of the
/// [`try_collect`](TryCollect::try_collect) operation.
/// Since these two errors types are expected to be unrelated to
/// one-another
/// (after all, [`TrippableIterator`] exists precisely to decouple
/// the downstream iterators from upstream failures),
/// there is no obvious and correct conversion,
/// and so it is left up to the caller.
/// Often,
/// this call is simply suffixed with `??`,
/// leaving the containing function's return type to manage the
/// conversion via [`Into`].
#[inline]
fn try_collect_ok<T, E, B: TryFromIterator<T>>(
fn try_collect_ok<T, EI, B: TryFromIterator<T>>(
mut self,
) -> Result<Result<B, B::Error>, E>
) -> Result<B, B::Error>
where
Self: Iterator<Item = Result<T, E>>,
Self: Iterator<Item = Result<T, EI>>,
EI: Into<B::Error>,
{
// At the time of writing,
// `self.while_ok(TryCollect::try_collect)` does not compile,

View File

@ -51,7 +51,7 @@ use std::marker::PhantomData;
///
/// See those two functions and the [parent module](super) for more
/// information and examples.
pub struct TripIter<'a, I: ResultIterator<T, E>, T, E> {
pub struct TripIter<'a, I: ResultIterator<T, EI>, T, EI> {
/// Iterator that will be unwrapped while its item is [`Ok`].
inner: &'a mut I,
@ -61,14 +61,14 @@ pub struct TripIter<'a, I: ResultIterator<T, E>, T, E> {
/// If there is an error,
/// then we have been tripped and will return [`None`] until the error
/// condition has been resolved.
state: Result<(), E>,
state: Result<(), EI>,
/// Our [`Iterator`] implementation will yield `T`,
/// but we don't store it.
_phantom: PhantomData<T>,
}
impl<'a, I: ResultIterator<T, E>, T, E> TripIter<'a, I, T, E> {
impl<'a, I: ResultIterator<T, EI>, T, EI> TripIter<'a, I, T, EI> {
/// Given a mutable reference to a
/// [`ResultIterator<T, E>`](ResultIterator),
/// yield a [`TripIter`] that yields the inner `T` value while the
@ -77,9 +77,10 @@ impl<'a, I: ResultIterator<T, E>, T, E> TripIter<'a, I, T, E> {
/// See the public [`with_iter_while_ok`] function for more
/// information.
#[inline]
fn with_iter_while_ok<F, U>(iter: &'a mut I, f: F) -> Result<U, E>
fn with_iter_while_ok<F, U, E>(iter: &'a mut I, f: F) -> Result<U, E>
where
F: FnOnce(&mut Self) -> U,
F: FnOnce(&mut Self) -> Result<U, E>,
EI: Into<E>,
{
let mut biter = Self {
inner: iter,
@ -95,7 +96,7 @@ impl<'a, I: ResultIterator<T, E>, T, E> TripIter<'a, I, T, E> {
// after the caller is done with it,
// to check to see if we have tripped and force the caller to
// consider the error.
biter.state.and_then(|_| Ok(fret))
biter.state.map_err(EI::into).and_then(|_| fret)
}
/// Whether the iterator has been tripped by an [`Err`] on the
@ -110,9 +111,9 @@ impl<'a, I: ResultIterator<T, E>, T, E> TripIter<'a, I, T, E> {
}
}
impl<'a, I, T, E> Iterator for TripIter<'a, I, T, E>
impl<'a, I, T, EI> Iterator for TripIter<'a, I, T, EI>
where
I: Iterator<Item = Result<T, E>>,
I: Iterator<Item = Result<T, EI>>,
{
type Item = T;
@ -141,9 +142,10 @@ where
}
}
/// Given a mutable reference to a [`ResultIterator<T, E>`](ResultIterator),
/// yield a [`TripIter`] that yields the inner `T` value while the
/// iterator yields an [`Ok`] item.
/// Given a mutable reference to a
/// [`ResultIterator<T, EI>`](ResultIterator),
/// yield a [`TripIter`] that yields the inner `T` value while the
/// iterator yields an [`Ok`] item.
///
/// Once an [`Err`] is encountered,
/// [`TripIter`] trips,
@ -151,12 +153,22 @@ where
/// This allows for a recovery mechanism that resumes computation,
/// provided that the system expects [`TripIter`] to be resumable.
///
/// The [`TripIter`] is provided via a callback `f`,
/// The [`TripIter`] is provided as an argument to a callback `f`
/// and is valid only for its duration.
/// This allows us to return either the most recently encountered [`Err`],
/// otherwise [`Ok`] with the return value of `f`,
/// otherwise the return value of `f`,
/// ensuring that the error causing the trip will not be lost.
///
/// The callback function `f` must return a [`Result`] whose error type is
/// [`Into<E>`](Into);
/// this allows [`Result`]s to be continuously flattened into a
/// consistent type,
/// which is especially useful when nesting [`TripIter`].
/// This puts the error type in control of the caller via the callback.
/// Since nested [`TripIter`] using this function must share the same error
/// type `E`,
/// this operation is monadic.
///
/// This function accepts a mutable reference to the underlying iterator,
/// allowing the caller to retain ownership for further processing after
/// iteration completes.
@ -180,6 +192,8 @@ where
/// assert_eq!(None, iter.next());
/// assert_eq!(None, iter.next());
/// assert!(iter.is_tripped());
///
/// Ok(())
/// });
///
/// // The error that caused the trip is returned.
@ -191,19 +205,19 @@ where
///
/// See the [parent module](super) for more information and examples.
#[inline]
pub fn with_iter_while_ok<'a, I, T, U, E, F>(
pub fn with_iter_while_ok<'a, I, T, U, E, EI>(
from: &'a mut I,
f: F,
f: impl FnOnce(&mut TripIter<'a, I, T, EI>) -> Result<U, E>,
) -> Result<U, E>
where
I: ResultIterator<T, E>,
F: FnOnce(&mut TripIter<'a, I, T, E>) -> U,
I: ResultIterator<T, EI>,
EI: Into<E>,
{
TripIter::with_iter_while_ok(from, f)
}
/// Given an object capable of being converted into a
/// [`ResultIterator<T, E>`](ResultIterator),
/// [`ResultIterator<T, EI>`](ResultIterator),
/// yield a [`TripIter`] that yields the inner `T` value while the
/// iterator yields an [`Ok`] item.
///
@ -226,6 +240,8 @@ where
/// assert_eq!(None, iter.next());
/// assert_eq!(None, iter.next());
/// assert!(iter.is_tripped());
///
/// Ok(())
/// });
///
/// // The error that caused the trip is returned.
@ -237,10 +253,13 @@ where
///
/// See the [parent module](super) for more information and examples.
#[inline]
pub fn into_iter_while_ok<I, T, U, E, F>(from: I, f: F) -> Result<U, E>
pub fn into_iter_while_ok<I, T, U, E, EI>(
from: I,
f: impl FnOnce(&mut TripIter<I::IntoIter, T, EI>) -> Result<U, E>,
) -> Result<U, E>
where
I: IntoIterator<Item = Result<T, E>>,
F: FnOnce(&mut TripIter<I::IntoIter, T, E>) -> U,
I: IntoIterator<Item = Result<T, EI>>,
EI: Into<E>,
{
with_iter_while_ok(&mut from.into_iter(), f)
}
@ -251,9 +270,9 @@ where
///
/// For more information,
/// see the [module-level documentation](self).
pub trait TrippableIterator<T, E>
pub trait TrippableIterator<T, EI>
where
Self: Iterator<Item = Result<T, E>> + Sized,
Self: Iterator<Item = Result<T, EI>> + Sized,
{
/// Given a mutable reference to a
/// [`ResultIterator<T, E>`](ResultIterator),
@ -264,9 +283,10 @@ where
/// see [`with_iter_while_ok`] and the
/// [module-level documentation](super).
#[inline]
fn while_ok<F, U>(&mut self, f: F) -> Result<U, E>
fn while_ok<F, U, E>(&mut self, f: F) -> Result<U, E>
where
F: FnOnce(&mut TripIter<Self, T, E>) -> U,
F: FnOnce(&mut TripIter<Self, T, EI>) -> Result<U, E>,
EI: Into<E>,
{
TripIter::with_iter_while_ok(self, f)
}
@ -279,9 +299,10 @@ where
/// For more information,
/// see [`into_iter_while_ok`] and the [module-level documentation](super).
#[inline]
fn into_while_ok<F, U>(mut self, f: F) -> Result<U, E>
fn into_while_ok<F, U, E>(mut self, f: F) -> Result<U, E>
where
F: FnOnce(&mut TripIter<Self, T, E>) -> U,
F: FnOnce(&mut TripIter<Self, T, EI>) -> Result<U, E>,
EI: Into<E>,
{
self.while_ok(f)
}
@ -300,10 +321,11 @@ mod test {
#[test]
fn inner_none_yields_none() -> TestResult {
let empty = Vec::<Result<(), ()>>::new();
let empty = Vec::<TestResult>::new();
into_iter_while_ok(empty, |sut| {
assert_eq!(None, sut.next());
Ok(())
})
}
@ -312,12 +334,16 @@ mod test {
let value1 = "inner1";
let value2 = "inner2";
into_iter_while_ok([Ok(value1), Ok(value2)], |sut| {
let iter = [Result::<_, ()>::Ok(value1), Ok(value2)];
into_iter_while_ok(iter, |sut| {
assert_eq!(Some(value1), sut.next());
assert_eq!(Some(value2), sut.next());
assert_eq!(None, sut.next());
assert!(!sut.is_tripped());
Ok(())
})
}
@ -341,6 +367,8 @@ mod test {
// Nor should it ever, while tripped.
assert_eq!(None, sut.next());
assert!(sut.is_tripped());
Ok("")
});
assert_eq!(result, Err(err));
@ -356,6 +384,8 @@ mod test {
// Try to consume everything.
assert_eq!(None, sut.next());
assert_eq!(None, sut.next());
Ok(())
}),
);
@ -367,10 +397,10 @@ mod test {
fn open_returns_f_ret_after_none() {
let ret = "retval";
assert_eq!(
Ok(ret),
Result::<_, ()>::Ok(ret),
into_iter_while_ok([Result::<_, ()>::Ok(())], |sut| {
sut.next();
ret
Ok(ret)
})
);
}
@ -382,7 +412,7 @@ mod test {
Err(err),
into_iter_while_ok([Result::<(), _>::Err(err)], |sut| {
sut.next();
"not used"
Ok("not used")
})
);
}
@ -393,10 +423,10 @@ mod test {
fn does_not_trip_before_err_is_encountered() {
let ret = "no next";
assert_eq!(
Ok(ret),
Result::<_, &str>::Ok(ret),
into_iter_while_ok(
[Result::<(), _>::Err("will not be seen")],
|_| { "no next" }
|_| { Ok("no next") }
)
);
}

View File

@ -193,21 +193,23 @@ fn load_xmlo<'a, P: AsRef<Path>, S: Escaper>(
// abstracted away.
let mut state =
into_iter_while_ok(XmlXirReader::new(file, escaper, ctx), |toks| {
flat::State::<64>::parse(toks).lower_while_ok::<XmloReader, _>(
flat::State::<64>::parse(toks).lower_while_ok::<XmloReader, _, _>(
|xirf| {
into_iter_while_ok(xirf, |xmlo_out| {
// TODO: Transitionary---we do not want to filter.
depgraph.import_xmlo(
xmlo_out.filter_map(|parsed| match parsed {
Parsed::Incomplete => None,
Parsed::Object(obj) => Some(Ok(obj)),
}),
state,
)
depgraph
.import_xmlo(
xmlo_out.filter_map(|parsed| match parsed {
Parsed::Incomplete => None,
Parsed::Object(obj) => Some(Ok(obj)),
}),
state,
)
.map_err(TameldError::from)
})
},
)
})????;
})?;
let mut dir: PathBuf = path.clone();
dir.pop();

View File

@ -713,20 +713,22 @@ impl<S: ParseState, I: TokenStream<S::Token>> Parser<S, I> {
/// Consequently,
/// this API (likely the return type) will change.
#[inline]
pub fn lower_while_ok<LS, U>(
pub fn lower_while_ok<LS, U, E>(
&mut self,
f: impl FnOnce(&mut LowerIter<S, I, LS>) -> U,
) -> Result<U, ParseError<S::Token, S::Error>>
f: impl FnOnce(&mut LowerIter<S, I, LS>) -> Result<U, E>,
) -> Result<U, E>
where
LS: ParseState<Token = S::Object>,
<S as ParseState>::Object: Token,
<LS as ParseState>::Context: Default,
ParseError<S::Token, S::Error>: Into<E>,
{
self.while_ok(|toks| {
// TODO: This parser is not accessible after error recovery!
let lower = LS::parse(iter::empty());
f(&mut LowerIter { lower, toks })
})
.map_err(Into::into)
}
}