about summary refs log tree commit diff
path: root/src/libsyntax/parse/parser.rs
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2014-01-21 15:56:16 -0800
committerbors <bors@rust-lang.org>2014-01-21 15:56:16 -0800
commit918a7314a8d49d870ff95f8ed6c7bdc5895138c9 (patch)
treefcd89f5349850b34b6ed3c7fd7b04ad421705db1 /src/libsyntax/parse/parser.rs
parent505572b3f830c8f5140efaaf2adf8293e29b0db9 (diff)
parentec422d70c3b10974b81eac6988fc5b3fa6ce60bc (diff)
downloadrust-918a7314a8d49d870ff95f8ed6c7bdc5895138c9.tar.gz
rust-918a7314a8d49d870ff95f8ed6c7bdc5895138c9.zip
auto merge of #11129 : SimonSapin/rust/foo-vs-foo_opt, r=alexcrichton
[On 2013-12-06, I wrote to the rust-dev mailing list](https://mail.mozilla.org/pipermail/rust-dev/2013-December/007263.html):

> Subject: Let’s avoid having both foo() and foo_opt()
>
> We have some functions and methods such as [std::str::from_utf8](http://static.rust-lang.org/doc/master/std/str/fn.from_utf8.html) 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](https://mail.mozilla.org/pipermail/rust-dev/2013-December/007270.html).

Feel free to pick only some of these commits.
Diffstat (limited to 'src/libsyntax/parse/parser.rs')
-rw-r--r--src/libsyntax/parse/parser.rs19
1 files changed, 10 insertions, 9 deletions
diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
index 3a5e737e026..4e1703fe6b0 100644
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -1753,19 +1753,19 @@ impl Parser {
                 return self.mk_expr(lo, hi, ExprLit(lit));
             }
             let mut es = ~[self.parse_expr()];
-            self.commit_expr(*es.last(), &[], &[token::COMMA, token::RPAREN]);
+            self.commit_expr(*es.last().unwrap(), &[], &[token::COMMA, token::RPAREN]);
             while self.token == token::COMMA {
                 self.bump();
                 if self.token != token::RPAREN {
                     es.push(self.parse_expr());
-                    self.commit_expr(*es.last(), &[], &[token::COMMA, token::RPAREN]);
+                    self.commit_expr(*es.last().unwrap(), &[], &[token::COMMA, token::RPAREN]);
                 }
                 else {
                     trailing_comma = true;
                 }
             }
             hi = self.span.hi;
-            self.commit_expr_expecting(*es.last(), token::RPAREN);
+            self.commit_expr_expecting(*es.last().unwrap(), token::RPAREN);
 
             return if es.len() == 1 && !trailing_comma {
                 self.mk_expr(lo, self.span.hi, ExprParen(es[0]))
@@ -1924,7 +1924,8 @@ impl Parser {
 
                     fields.push(self.parse_field());
                     while self.token != token::RBRACE {
-                        self.commit_expr(fields.last().expr, &[token::COMMA], &[token::RBRACE]);
+                        self.commit_expr(fields.last().unwrap().expr,
+                                         &[token::COMMA], &[token::RBRACE]);
 
                         if self.eat(&token::DOTDOT) {
                             base = Some(self.parse_expr());
@@ -1939,7 +1940,7 @@ impl Parser {
                     }
 
                     hi = pth.span.hi;
-                    self.commit_expr_expecting(fields.last().expr, token::RBRACE);
+                    self.commit_expr_expecting(fields.last().unwrap().expr, token::RBRACE);
                     ex = ExprStruct(pth, fields, base);
                     return self.mk_expr(lo, hi, ex);
                 }
@@ -2092,7 +2093,7 @@ impl Parser {
                   // This is a conservative error: only report the last unclosed delimiter. The
                   // previous unclosed delimiters could actually be closed! The parser just hasn't
                   // gotten to them yet.
-                  match p.open_braces.last_opt() {
+                  match p.open_braces.last() {
                       None => {}
                       Some(&sp) => p.span_note(sp, "unclosed delimiter"),
                   };
@@ -2157,7 +2158,7 @@ impl Parser {
 
                 // Parse the close delimiter.
                 result.push(parse_any_tt_tok(self));
-                self.open_braces.pop();
+                self.open_braces.pop().unwrap();
 
                 TTDelim(@result)
             }
@@ -3592,7 +3593,7 @@ impl Parser {
                 }
             );
 
-        let variadic = match args.pop_opt() {
+        let variadic = match args.pop() {
             Some(None) => true,
             Some(x) => {
                 // Need to put back that last arg
@@ -4217,7 +4218,7 @@ impl Parser {
     }
 
     fn pop_mod_path(&mut self) {
-        self.mod_path_stack.pop();
+        self.mod_path_stack.pop().unwrap();
     }
 
     // read a module from a source file.