Rename foo_opt to foo and drop the old foo behavior

[SimonSapin]

On 2013-12-06, I wrote to the rust-dev mailing list:

Subject: Let’s avoid having both foo() and foo_opt()

We have some functions and methods such as std::str::from_utf8 that may succeed and give a result, or fail when the input is invalid.

1. Sometimes we assume the input is valid and don’t want to deal with the error case. Task failure works nicely.
2. Sometimes we do want to do something different on invalid input, so returning an Option<T> works best.

And so we end up with both from_utf8 and from_utf8. This particular case is worse because we also have from_utf8_owned and from_utf8_owned_opt, to cover everything.

Multiplying names like this is just not good design. I’d like to reduce this pattern.

Getting behavior 1. when you have 2. is easy: just call .unwrap() on the Option. I think we should rename every foo_opt() function or method to just foo, remove the old foo() behavior, and tell people (through documentation) to use foo().unwrap() if they want it back?

The downsides are that unwrap is more verbose and gives less helpful error messages on task failure. But I think it’s worth it.

The email discussion has gone around long enough. Let’s discuss a concrete proposal. For the following functions or methods, I removed foo (that caused task failure) and renamed foo_opt (that returns Option) to just foo.

Vector methods:

• get_opt (rename only, get did not exist as it would have been just [])
• head_opt
• last_opt
• pop_opt
• shift_opt
• remove_opt

std::path::BytesContainer method:

• container_as_str_opt

std::str functions:

• from_utf8_opt
• from_utf8_owned_opt (also remove the now unused not_utf8 condition)

Is there something else that should recieve the same treatement?

I did not rename recv_opt on channels based on @brson’s feedback.

Feel free to pick only some of these commits.

[alexcrichton]

I'm a little worried about this patch. This introduces a lot of call to unwrap(), which is in theory supposed to be an antipattern. It's possible that it's just too much of a pain to alter all the code that uses these functions right now, but I am afraid of promoting the unwrap() methodology in real programs because of these api changes. It seems marginally better than failure in a library because we can at least say "you were asking for it", but I'm also uncomfortable in that we don't have a solution for problems like this.

In general this seems like an improvement, but I think this warrants discussion to ensure that this it the direction that we want to go in. It would be sad for unwrap() to become rust's "null pointer problem", but that definitely seems to be where it's headed right now.

[alexcrichton]

I have added this to the next meeting agenda, but this can certainly be discussed before then.

[SimonSapin]

Thanks @alexcrichton . Yes, I definitely expect this needs more discussion.

[liigo]

This will make code full of .unwrap() in some situation, which is not a good smell. We should not take a "not a good smell" to replace another "not a good smell". The right direction is: find a better resolution first. -- I will share my solution later.

(Just as the "do" expr, someone want to remove it, and leave us a more ugly syntax. The right direction is, find a better syntax before remove do-expr.)

[kud1ing]

Maybe we don't need full blown Monads/do-expressions.
Could we add a macro to achieve C# monadic null-checking http://adamralph.com/2013/12/06/ndc-diary-day-3/ ?

[liigo]

#11133 [auto-unwrap]
After it is accepted, we may safely remove one of .foo() and .foo_opt().

[alexcrichton]

We decided in today's meeting that we'd like to merge this (all commits).

If you've run out of time, I'm fine to take over and rebase!

[SimonSapin]

@alexcrichton I’ll do the rebase. Is there some other API that should get the same treatement?

[alexcrichton]

This is certainly good for now. We can weed out more cases as we see them. We're going to adopt this as a general design principle from now on, so we'll prevent future things from coming in.

Basically, just rebasing this is fine for now, thanks so much!

[SimonSapin]

I now notice that std::vec has a few more methods that fail on empty vectors/slices:

• shift_ref
• pop_ref
• mut_last
• mut_shift_ref
• mut_pop_ref

Maybe they should return Option, to be consistent with how this PR is changing shift, pop and last.

[SimonSapin]

@alexcrichton Rebased, but still building (again) to run make check

[alexcrichton]

It looks like bors's queue isn't so full right now, so we can just run this through bors.

Thanks again for all this awesome work, and not everything has to change this time around, the remaining methods can be redone in follow-up PRs