Description
Summary
It would be helpful if Iterator's .sum() method would explicitly document that it can sum an iterator of Result<T,E> and produce a Result<T,E> as output, and that it stops consuming the iterator when it hits the first error. To compound things, my mental model was a little off, and I didn't understand the compiler's diagnostics well enough to get nudged in the right direction.
Likewise, it would be helpful to also document summing an Option<T>. Similarly, for .product() and Result<T,E> or Option<T>. I'm not sure if there are other Iterator methods that also handle Result and Option similarly.
Steps to Reproduce
If I go to the standard library documentation at https://doc.rust-lang.org/std/index.html, and then search for sum, then click on std::iter::Iterator::sum it gives an example of summing some integers. It would be helpful if this part of the documentation mentioned that you can also sum an iterator of Result<T,E>, which produces a Result<T,E> where the Ok() variant contains the sum (if there were no errors), or the Err() variant contains the first error. It would also be helpful to mention that it stops consuming the iterator once it encounters the first error.
If I click on Sum in the declaration, it takes me to documentation of the Sum trait. In the Implementors section, there is a long list of implementations for various numeric types, their Wrapping variants, their references, Wrapping variants of references, SIMD stuff. At that point, my eyes glazed over, and I missed this one buried near the end:
impl<T, U, E> Sum<Result<U, E>> for Result<T, E>
where
T: Sum<U>If I click the "source" link to the right of the above impl, the doc comments describe the behavior, and there is even an example (yay!). As far as I can tell, those doc comments aren't rendered on the documentation page for either the Sum trait, or the .sum() method. It would probably be helpful to render them on the page for the Sum trait, and add a sentence or two to the documentation for .sum() (perhaps link to the Sum documentation if you don't want to repeat it all).
Background
This came about from the Advent of Code 2022, Day 1. See https://adventofcode.com/2022/day/1. The input is some text with groups of integers. The groups are separated by blank lines. Start by reading the input:
let input = std::fs::read_to_string("src/bin/day01/input.txt")
.expect("Can't open input.txt");Now build a Vec where each element is the sum of the numbers in one group:
let mut elf_totals: Vec<u32> = input.split("\n\n").map(|s|
// s is the numbers for one elf
s.split_terminator('\n').map(|n|
// n is one number for the current elf
n.parse::<u32>().expect("invalid number")
).sum()
).collect();That works, but I'd like to do a better job of handling errors. First step is to change the function (main in this case) to return a Result<(), Box<dyn Error>>, and add an OK(()) at the end of the function.
Now, let's replace the .expect(...) with a question mark. My intent (for now) is to return from the function, not just the closure. I'm using VSCode with rust-analyzer. In the Problems tab, it says:
the `?` operator can only be used in a closure that returns `Result` or `Option`...
the trait `FromResidual<Result<Infallible, ParseIntError>>` is not implemented for `u32`
main.rs[Ln 8, Col 38]: this function should return `Result` or `Option` to accept `?`
My first thought is that my function (main) does return Result. Oh, that last message is talking about the closure I'm passing to .map(). OK, let's try return instead:
let x = n.parse::<u32>();
if let Err(e) = x {
return Err(e);
} else {
x.unwrap()
}Lots of errors. First one says "closure bodies that contain statements must be surrounded by braces." I can do that (there's even a Quick Fix that adds them for me!). Now there are only 3 errors:
the trait bound `u32: Sum<Result<_, ParseIntError>>` is not satisfied
the following other types implement trait `Sum<A>`:
...
expected Result<{unknown}, ParseIntError>, found u32
mismatched types
expected enum `Result<_, ParseIntError>`
found type `u32`
For that last error, it suggests wrapping the expression in Ok(). Let the Quick Fix do that. It wrapped the entire if-else inside Ok(). That still leaves the first error message:
the trait bound `u32: Sum<Result<_, ParseIntError>>` is not satisfied
the following other types implement trait `Sum<A>`:
...
It never occurred to me that the closure could return a Result. After all, I'm trying to sum integers, so I assume my closure has to return an integer. That return statement I just added is returning from the closure, not the function. I go off to Google ways to return from a function from within a closure. I didn't find anything that looked promising.
I take a different approach. Instead of using nested closures, I use nested for loops. Since it's not inside a closure any more, I can use the question mark to return from the function:
let mut elf_totals: Vec<u32> = Vec::new();
for group in input.split("\n\n") {
let mut total: u32 = 0;
for line in group.lines() {
total += line.parse::<u32>()?;
}
elf_totals.push(total);
}That works, but there's got to be a better way. I figured out how to write a function that sums an iterator of Result<T,E> and produces a Result<T,E>, and short circuits on the first error. I didn't know that .sum() could already do this.
fn try_sum<I,T,E>(iter: I) -> Result<T,E>
where T: Default + AddAssign, I: Iterator<Item=Result<T,E>>
{
let mut result = T::default();
for x in iter {
result += x?;
}
Ok(result)
}I figured out how to turn that into a generic trait so I could use it as a method (like the methods in the Itertools trait). That took a bunch of fiddling with generics and bounds, but I was able to work through it (in large part because the diagnostics nudged me in the right direction). The final piece was figuring out how to write the impl block so I could replace .sum() with my new .try_sum() method.
trait TrySum: Iterator {
fn try_sum<T,E>(&mut self) -> Result<T,E>
where Self: Iterator<Item=Result<T,E>>,
T: Default + AddAssign,
{
let mut answer = T::default();
for x in self {
answer += x?;
}
Ok(answer)
}
}
impl<T> TrySum for T where T: Iterator {}I knew that .collect() could transform an iterator over Result<T,E> into Result<Vec<T>, E>, and therefore I can put a question mark or .expect() after .collect().
let mut elf_totals = input.split("\n\n").map(|s| {
// s is the numbers for one elf
s.split_terminator('\n').map(|n|
// n is one number for the current elf
n.parse::<u32>()
).try_sum()
}).collect::<Result<Vec<u32>,_>>()?;My implementation required the type I'm summing to implement Default and AddAssign. Can I get rid of the need for Default? Should I depend on Add instead of AddAssign? I start digging through the implementation of .sum() in the standard library to see if it does anything special that my implementation should. I ended up finding that sum (and other methods, like product) share a general purpose mechanism to handle iteration over Result<T,E>. That means that .sum() already does what my .try_sum() does. I replace .try_sum() with .sum() and ... it works!