Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upOk wrapping: Improved support for writing code from an error handling mindset #2107
Conversation
scottmcm
added some commits
Aug 9, 2017
aturon
referenced this pull request
Aug 14, 2017
Open
Language ergonomic/learnability improvements #17
scottmcm
self-assigned this
Aug 14, 2017
scottmcm
added
the
T-lang
label
Aug 14, 2017
withoutboats
added
the
Ergonomics Initiative
label
Aug 14, 2017
sfackler
reviewed
Aug 14, 2017
| Take this example from [RFC 1937]: | ||
|
|
||
| ```rust | ||
| fn main() -> Result<(), Box<Error>> { |
This comment has been minimized.
This comment has been minimized.
sfackler
Aug 14, 2017
Member
Might be worth noting here that C allows you to omit a return from main and it'll insert a return 0 for you for the same reason.
This comment has been minimized.
This comment has been minimized.
ubsan
Aug 14, 2017
Contributor
I don't believe this is true of C, but of C++. I'd have to check the standard to be sure though.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
eddyb
Aug 14, 2017
•
Member
Apparently it does require C99, when compiling in a pre-C99 mode you get the "whatever was left around in the platform return register" behavior.
This comment has been minimized.
This comment has been minimized.
|
While I don't share the view that writing |
This comment has been minimized.
This comment has been minimized.
|
My gut level response to this was
Makes me much, much more okay with it. |
This comment has been minimized.
This comment has been minimized.
|
I have pretty strong feelings about the syntax for this RFC, and I know @nrc has pretty strong feelings about the syntax in an opposite direction. Let me try to lay out my preference while I have ambushed everyone with the module RFC I personally think its very important that this have a function level signifier, and one thing I think is very important about that is that the function not appear to return Instead, I prefer this syntax:
So for example: // Applied to Result
fn read(&mut self, buf: &mut [u8]) -> usize throws io::ErrorEssentially we get something syntactically very similar to Java, but with these extremely significant differences:
In sum, we "have our cake and eat it too" - we have the syntactic nicety of exceptions with the semantic nicety of errors as values. This is also very nice for the "editing case" - you have a function that didn't used to do io, and so it was infallible, and now you introduced some fallible path to it. It needs to start throwing. What do you do?
No other changes necessary! This RFC supports that as well, but its a bit more clunky in that you have to replace the return type with a EDIT: I also think the Err(Error::new(...))?One thing this would mean is that this syntax does not support non-Result types. I pesonally feel okay with that, even prefer it, but others feel its very important that they be symmetric. This syntax was proposed as an option: fn read(&mut self, buf: &mut [u8]) -> usize throws io::Error as Result<usize, io::Error>
fn pop(&mut self) -> T throws as Option<T>In this world, an elided |
This comment has been minimized.
This comment has been minimized.
|
Also, syntactically what I've just proposed is extremely similar to what @glaebhoerl proposed in the original |
This comment has been minimized.
This comment has been minimized.
|
@withoutboats Nit: |
This comment has been minimized.
This comment has been minimized.
stevenblenkinsop
commented
Aug 14, 2017
•
|
The odd thing about this syntax is that it makes it look like you're defining the type of The following syntax is conceptually closer to what's happening while retaining an explicit return type: fn function() -> Result<T, E> catches { ... }
Edit: fn function() -> Result<T, E> catches where ... { ... }` |
This comment has been minimized.
This comment has been minimized.
|
I think Ok-wrapping is great for ergonomics but I'm Similarly, |
This comment has been minimized.
This comment has been minimized.
|
@kennytm Function signatures already contain argument patterns which have no effect outside of the function body. |
This comment has been minimized.
This comment has been minimized.
|
@sfackler That's sunk cost fallacy |
This comment has been minimized.
This comment has been minimized.
|
It's only a sunk cost if you think it's a cost in the first place. |
This comment has been minimized.
This comment has been minimized.
|
In rustdoc at least this syntax could be normalized. I don't agree that its important functions have only externally relevant info, or unimportant that they have info that is only internally relevant. |
This comment has been minimized.
This comment has been minimized.
|
@kennytm do you have alternate proposals except doing this without any syntax (e.g. as coercion, or "whenever there would be an error"), or not doing this at all? |
This comment has been minimized.
This comment has been minimized.
|
I think I actually ended up pretty close to @withoutboats on the syntax. My initial feeling was that from the caller's perspective, the function returns a |
This comment has been minimized.
This comment has been minimized.
|
One thought: I don't really like that |
This comment has been minimized.
This comment has been minimized.
|
@est31: Whenever the function is
(Basically combining "Handle only the No idea about |
This comment has been minimized.
This comment has been minimized.
stevenblenkinsop
commented
Aug 14, 2017
•
|
My main other concern is that, given fn function() -> T throws E { ... }if you write fn function()? -> T throws E { ... }The issue I see with this is that, while it is logical, it might not be terribly usable, since you always have to remember to add a |
This comment has been minimized.
This comment has been minimized.
|
@kennytm that would be a breaking change because right now code like this is possible: fn foo() -> Result<(),()> { ... }
fn bar() -> Result<(),()> { return foo(); }Its a pattern that is present, and you either have to disallow it (doing a breaking change), or you'd have to allow ambiguities which would be horrible for readers of code. |
This comment has been minimized.
This comment has been minimized.
|
@est31 I don't see how that is breaking, I wrote |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@kennytm I see. If only |
This comment has been minimized.
This comment has been minimized.
Thanks for bringing that up, @stevenblenkinsop. That is, after all, how I've added Personally, I'm intrigued by the notion that "ok wrapping" could become essentially "function-level |
This comment has been minimized.
This comment has been minimized.
stevenblenkinsop
commented
Aug 15, 2017
•
Yeah, I like how it describes what's actually happening. Unfortunately, it's since occurred to me that both I see two solutions. The first might be too drastic, but I'm including it because I think it's an elegant solution on its own.
I suppose you could interpret A potential precedent in favour of |
This comment has been minimized.
This comment has been minimized.
burdges
commented
Aug 21, 2017
|
I think |
This comment has been minimized.
This comment has been minimized.
I think it's not just an eyesore. Currently, Rust fails to entirely deliver to the promise of "propagating failure is explicit via fn main() -> Result<(), io::Error> {
let mut stdin = io::stdin();
let mut raw_stdout = io::stdout();
let mut stdout = raw_stdout.lock();
for line in stdin.lock().lines() {
stdout.write(line?.trim().as_bytes())?;
stdout.write(b"\n")?;
}
stdout.flush()
}This makes it look like the I somewhat prefer throws because it makes it so that the type after the Since what I said above is entirely motivated around |
This comment has been minimized.
This comment has been minimized.
Well said.
Here's an example hitting the Today: fn get(self, slice: &str) -> Option<&Self::Output> {
if let Some(end) = self.end.checked_add(1) {
(self.start..end).get(slice)
} else {
None
}
}With fn get(self, slice: &str) -> Option<&Self::Output> {
(self.start..self.end.checked_add(1)?).get(slice)
}But the So, with this RFC: fn get(self, slice: &str)? -> Option<&Self::Output> {
(self.start..self.end.checked_add(1)?).get(slice)?
}Edit: Another example where fn fold1<T, I, F>(mut it: I, f: F) -> Option<T>
where I: Iterator<Item=T>, F: FnMut(T,T)->T
{
if let Some(init) = it.next() {
Some(it.fold(init, f))
} else {
None
}
}In the future with #1859 and trying another syntactic variant of this RFC: fn fold1<T, I, F>(mut it: I, f: F) -> Option<T>
where I: Iterator<Item=T>, F: FnMut(T,T)->T
?{
let init = it.next()?;
it.fold(init, f)
} |
This comment has been minimized.
This comment has been minimized.
True. However, somehow, I am much more comfortable seeing fn get(self, slice: &str) -> Option<&Self::Output> {
(self.start..self.end.checked_add(1)?).get(slice)
}I am not saying we shouldn't have have sugar that does |
This comment has been minimized.
This comment has been minimized.
|
I am strongly against this RFC, in the long term this hurts readability because it makes things nonobvious. Even when seen "from an error handling mindset", when writing fallible code I very much want to convey explicitly that I'm returning a value that represents success. If I see a function ending with a |
This comment has been minimized.
This comment has been minimized.
I guess this is an "agree with the conclusion but not the reasoning" thing for me? It's a basic thing in Rust that a block evaluates to the result of the last expression inside it, and hence its type is also that of its last expression, and here we have a function which returns something of type |
This comment has been minimized.
This comment has been minimized.
|
@glaebhoerl Everything you say makes sense if you think of the
No, I don't think that would be reasonable. But I want a way opt in to the view saying that these are Learning that error handling can be "reduced" to this kind of data passing is eye-opening, and I love Rust for fully buying into this. However, in many cases the other "mode" of thinking about error handling more like exceptions in C++/Java is useful. I actually think learning both views helps people understand error handling in Rust and in general better. It also helps understanding the relationship between Rust's |
This comment has been minimized.
This comment has been minimized.
|
@rfcbot fcp postpone Let me say first that I am really excited about this space of designs, and want to see something along the lines of this RFC in the language someday! However, one thing I think we've discovered over the last few weeks is that there are a lot of different things we can do here, and its going to take some iteration and learning to figure out the path forward. Given that we've got a lot of really high priority & high traffic RFCs going right now, and the impl block coming up in less than a month, I think we can't hope to merge this before the block begins. For that reason I'm proposing we postpone this for the time being, but its something I hope to return to personally in 2018, I think possibly using an attribute to test it out or an eRFC similar to async/await. |
rfcbot
added
the
proposed-final-comment-period
label
Aug 23, 2017
This comment has been minimized.
This comment has been minimized.
rfcbot
commented
Aug 23, 2017
•
|
Team member @withoutboats has proposed to postpone this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
This comment has been minimized.
This comment has been minimized.
|
For the next checkpoint (2018) I strongly suggest we start from "throwing functions" instead of this RFC. |
This comment has been minimized.
This comment has been minimized.
|
Thanks, @withoutboats; I agree we don't seem to be on track to resolve the diametrically-opposed opinions around this area within the schedule target. Two more partially-formed thoughts I wanted to include in the record for later:
fn fold1<T, I, F>(mut it: I, f: F) -> Option<T>
where I: Iterator<Item=T>, F: FnMut(T,T)->T
?{
let init = it.next()?;
it.fold(init, f)
}(This is slightly ambiguous because where clauses can end in
fn write(&mut self, buf: &[u8]) -> usize in io::Result { ... }
fn get(&self, i: usize) -> &T in Option { ... }(There's also been talk before of defaulting |
This comment has been minimized.
This comment has been minimized.
tomwhoiscontrary
commented
Aug 25, 2017
|
I don't see why allowing a general coercion from T to Result::Ok would be a bad thing. The RFC says:
But those both seem rather desirable to me. This example is definitely painful:
But to me, that mostly says that allowing @RalfJung's example is also painful:
And somewhere i saw something like this (although not quite this, which wouldn't compile anyway): fn foo() -> Result<(), u32> {
if everything_is_awesome() {
Ok(())
} else {
Err(911); // the semicolon means this returns Ok
}
}To me, what those examples really show is that the implicit return of a trailing expression is a mistake. Sadly, that ship has long since sailed. Would it be possible to introduce a general coercion, but then not apply it in some particular cases where it's likely to introduce ambiguity? I can imagine that writing the rules for when it would not apply would be a nightmare. And for every case where you avoid a confusion coercion, you introduce a confusing lack of coercion. |
This comment has been minimized.
This comment has been minimized.
For my example, I don't see how trailing expression-return makes any difference. This is just as bad: fn main() -> Result<(), io::Error> {
// snipped
return stdout.flush(); // looks infalliable
} |
This comment has been minimized.
This comment has been minimized.
tomwhoiscontrary
commented
Aug 28, 2017
I think the explicit return draws attention to the fact that this returns something, suggesting it is not infalliable. I realise that at this point we are offering subjective opinions on a well-settled of Rust syntax, though! |
This comment has been minimized.
This comment has been minimized.
rfcbot
commented
Aug 30, 2017
|
|
rfcbot
added
final-comment-period
and removed
proposed-final-comment-period
labels
Aug 30, 2017
This comment has been minimized.
This comment has been minimized.
rfcbot
commented
Aug 30, 2017
|
|
This comment has been minimized.
This comment has been minimized.
phaux
commented
Sep 7, 2017
•
|
How about fn regular_func() -> T {…}
fn throwing_func() -> T ? E {…}
let regular_closure = || {…};
let throwing_closure = ||? {…};Edit: other types of functions that we might eventually have fn function() -> T {…}
// returns T
fn throwing_fn() -> T ? E {…}
// returns impl Try<Success=T, Error=E>
fn generator() -> T * Y {…}
// returns impl Generator<Yield=Y, Return=T>
fn throwing_gen() -> T * Y ? E {…}
// returns impl Generator<Yield=Y, Return=impl Try<Success=T, Error=E>>
fn async_fn() -> ^ T {…}
// returns impl Future<Item=T, Error=()>
fn throw_async_fn() -> ^ T ? E {…}
// returns impl Future<Item=T, Error=E>
fn async_gen() -> ^ T * Y {…}
// returns impl Observable<Item=Y, Error=(), Complete=T>
fn throw_async_gen() -> ^ T * Y ? E {…}
// returns impl Observable<Item=Y, Error=E, Complete=T>
let function = || {…};
let throwing_fn = ||? {…};
let generator = ||* {…};
let throwing_gen = ||*? {…};
let async_fn = ||^ {…};
let throw_async_fn = ||^? {…};
let async_gen = ||^* {…};
let throw_async_gen = ||^*? {…};So basically the symbols mean:
|
This comment has been minimized.
This comment has been minimized.
|
@phaux that's surprisingly nice syntax. I wonder if it could actually work? |
This comment has been minimized.
This comment has been minimized.
rfcbot
commented
Sep 9, 2017
|
The final comment period is now complete. |
This comment has been minimized.
This comment has been minimized.
|
This RFC is being closed as postponed. While the lang team is very interested in pursuing improvements to Result-oriented programming along these lines, more design work and experimentation is going to be needed, so we're punting until after the impl period. Thanks @scottmcm! |
scottmcm commentedAug 14, 2017
Support a
?on function definitions andcatchblocks that automatically wraps the result in anOk,Some,Poll::Ready, etc. In particular, allow function authors to avoid the need forOk(())at the end of functions returningResult<(), E>.Rendered
Posted as part of the error handling ergonomics initiative, rust-lang/rust-roadmap-2017#17
CC originating issue. rust-lang/rust#41414