Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The various {str,String}::from_utf{8,16}() functions should all return a Result<> #18888

Closed
jamesluke opened this issue Nov 11, 2014 · 3 comments
Labels
A-collections Area: `std::collection`

Comments

@jamesluke
Copy link
Contributor

The functions (note the return types):

collections::str::from_utf8(v: &'a [u8]) -> Option<&'a str>
collections::String::from_utf8(vec: Vec<u8>) -> Result<String, Vec<u8>>
collections::String::from_utf16(v: &[u16]) -> Option<String>

As you can see, String::from_utf8 is an odd one out, returning a Result rather than an Option. I propose that all these functions return a Result, as this more closely matches the semantics of Option and Result. Option is generally used where Some() or None are both valid return values. Result is used where Ok() is a valid value and Err() means the function could not perform as desired. The latter more closely matches common use of from_utf8 (we expect this buffer to be a utf8 string, but need to confirm it before turning it into a str or String, and something has gone wrong if it's not utf8). Running through the rust compiler source code, it's almost exclusively used in this fashion: virtually all cases are followed immediately by a .unwrap(), panicking the task if the buffer was not utf-8.

Exactly what the Result's Err() variant should hold, I do not know. It seems that it should return a proper FromError-style enum with a variant like UtfDecodeError, but I do now know how many different error enums we want in the standard library. I leave it to someone else to decide what's most useful and elegant.

@barosl
Copy link
Contributor

barosl commented Nov 12, 2014

+1 with this. We should unify the interfaces ASAP. It is a bit surprising that even the methods from the same type have the different return types.

@lifthrasiir
Copy link
Contributor

RFC #236 has a strong opinion supporting for this change. Among other reasons, the must_use attribute on Result alone is enough to justify that.

(Aside: While Result is unanimously better for this issue in my opinion, we still don't have a good convention on functions that optionally consume its argument and Result is not desirable. Can't think of any good example, though, so it may be fine.)

@huonw huonw added the A-libs label Nov 12, 2014
@kmcallister kmcallister added the A-collections Area: `std::collection` label Jan 16, 2015
@steveklabnik
Copy link
Member

This is now true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-collections Area: `std::collection`
Projects
None yet
Development

No branches or pull requests

6 participants