Auto merge of #23857 - phildawes:libsyntax_nopanic, r=nikomatsakis

Hello! 

I've been working towards a libsyntax without panics. See:
http://internals.rust-lang.org/t/changing-libsyntax-to-use-result-instead-of-panic/1670

This patch changes the internals of parser.rs to use Result<> rather than panicing. It keeps the following old-style panicing functions as a facade:
parse_expr, parse_item, parse_pat, parse_arm, parse_ty, parse_stmt

I left these functions because I wasn't sure what to do about the quote_* macros or how many syntax-extensions would break if these and quoting macros returned Result.

The gyst of the rest of the patch is:

 - Functions in parse/parser.rs return PResult<> rather than panicing
 - Other functions in libsyntax call panic! explicitly if they rely on panicing behaviour.
 - I added a macro 'panictry!()' to act as scaffolding for callers while converting panicing functions. (This does the same as 'unwrap()' but is easier to grep for and turn into try!()).

Am I on the right track?  I'd quite like to get something merged soon as keeping this rebased in the face of libsyntax changes is a lot of work. Please let me know what changes you'd like to see to make this happen.

Thanks!, Phil

Author: bors

src/librustc/metadata/creader.rs

@@ -528,7 +528,10 @@ impl<'a> CrateReader<'a> {
                                                               source_name.clone(),
                                                               body);
                 let lo = p.span.lo;
-                let body = p.parse_all_token_trees();
+                let body = match p.parse_all_token_trees() {
+                    Ok(body) => body,
+                    Err(err) => panic!(err),
+                };
                 let span = mk_sp(lo, p.last_span.hi);
                 p.abort_if_errors();
                 macros.push(ast::MacroDef {

src/librustc/session/mod.rs

@@ -68,13 +68,13 @@ impl Session {
         if self.opts.treat_err_as_bug {
             self.span_bug(sp, msg);
         }
-        self.diagnostic().span_fatal(sp, msg)
+        panic!(self.diagnostic().span_fatal(sp, msg))
     }
     pub fn span_fatal_with_code(&self, sp: Span, msg: &str, code: &str) -> ! {
         if self.opts.treat_err_as_bug {
             self.span_bug(sp, msg);
         }
-        self.diagnostic().span_fatal_with_code(sp, msg, code)
+        panic!(self.diagnostic().span_fatal_with_code(sp, msg, code))
     }
     pub fn fatal(&self, msg: &str) -> ! {
         if self.opts.treat_err_as_bug {

src/libsyntax/attr.rs

@@ -503,8 +503,8 @@ pub fn require_unique_names(diagnostic: &SpanHandler, metas: &[P<MetaItem>]) {
         let name = meta.name();
 
         if !set.insert(name.clone()) {
-            diagnostic.span_fatal(meta.span,
-                                  &format!("duplicate meta item `{}`", name));
+            panic!(diagnostic.span_fatal(meta.span,
+                                  &format!("duplicate meta item `{}`", name)));
         }
     }
 }

src/libsyntax/diagnostic.rs

@@ -68,10 +68,11 @@ pub trait Emitter {
                    sp: RenderSpan, msg: &str, lvl: Level);
 }
 
-/// This structure is used to signify that a task has panicked with a fatal error
-/// from the diagnostics. You can use this with the `Any` trait to figure out
-/// how a rustc task died (if so desired).
+/// Used as a return value to signify a fatal error occurred. (It is also
+/// used as the argument to panic at the moment, but that will eventually
+/// not be true.)
 #[derive(Copy, Clone)]
+#[must_use]
 pub struct FatalError;
 
 /// Signifies that the compiler died with an explicit call to `.bug`
@@ -88,13 +89,13 @@ pub struct SpanHandler {
 }
 
 impl SpanHandler {
-    pub fn span_fatal(&self, sp: Span, msg: &str) -> ! {
+    pub fn span_fatal(&self, sp: Span, msg: &str) -> FatalError {
         self.handler.emit(Some((&self.cm, sp)), msg, Fatal);
-        panic!(FatalError);
+        return FatalError;
     }
-    pub fn span_fatal_with_code(&self, sp: Span, msg: &str, code: &str) -> ! {
+    pub fn span_fatal_with_code(&self, sp: Span, msg: &str, code: &str) -> FatalError {
         self.handler.emit_with_code(Some((&self.cm, sp)), msg, code, Fatal);
-        panic!(FatalError);
+        return FatalError;
     }
     pub fn span_err(&self, sp: Span, msg: &str) {
         self.handler.emit(Some((&self.cm, sp)), msg, Error);

src/libsyntax/ext/asm.rs

@@ -91,16 +91,16 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt, sp: Span, tts: &[ast::TokenTree])
                       p.token != token::ModSep {
 
                     if outputs.len() != 0 {
-                        p.eat(&token::Comma);
+                        panictry!(p.eat(&token::Comma));
                     }
 
-                    let (constraint, _str_style) = p.parse_str();
+                    let (constraint, _str_style) = panictry!(p.parse_str());
 
                     let span = p.last_span;
 
-                    p.expect(&token::OpenDelim(token::Paren));
+                    panictry!(p.expect(&token::OpenDelim(token::Paren)));
                     let out = p.parse_expr();
-                    p.expect(&token::CloseDelim(token::Paren));
+                    panictry!(p.expect(&token::CloseDelim(token::Paren)));
 
                     // Expands a read+write operand into two operands.
                     //
@@ -131,20 +131,20 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt, sp: Span, tts: &[ast::TokenTree])
                       p.token != token::ModSep {
 
                     if inputs.len() != 0 {
-                        p.eat(&token::Comma);
+                        panictry!(p.eat(&token::Comma));
                     }
 
-                    let (constraint, _str_style) = p.parse_str();
+                    let (constraint, _str_style) = panictry!(p.parse_str());
 
                     if constraint.starts_with("=") {
                         cx.span_err(p.last_span, "input operand constraint contains '='");
                     } else if constraint.starts_with("+") {
                         cx.span_err(p.last_span, "input operand constraint contains '+'");
                     }
 
-                    p.expect(&token::OpenDelim(token::Paren));
+                    panictry!(p.expect(&token::OpenDelim(token::Paren)));
                     let input = p.parse_expr();
-                    p.expect(&token::CloseDelim(token::Paren));
+                    panictry!(p.expect(&token::CloseDelim(token::Paren)));
 
                     inputs.push((constraint, input));
                 }
@@ -155,10 +155,10 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt, sp: Span, tts: &[ast::TokenTree])
                       p.token != token::ModSep {
 
                     if clobs.len() != 0 {
-                        p.eat(&token::Comma);
+                        panictry!(p.eat(&token::Comma));
                     }
 
-                    let (s, _str_style) = p.parse_str();
+                    let (s, _str_style) = panictry!(p.parse_str());
 
                     if OPTIONS.iter().any(|&opt| s == opt) {
                         cx.span_warn(p.last_span, "expected a clobber, found an option");
@@ -167,7 +167,7 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt, sp: Span, tts: &[ast::TokenTree])
                 }
             }
             Options => {
-                let (option, _str_style) = p.parse_str();
+                let (option, _str_style) = panictry!(p.parse_str());
 
                 if option == "volatile" {
                     // Indicates that the inline assembly has side effects
@@ -182,7 +182,7 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt, sp: Span, tts: &[ast::TokenTree])
                 }
 
                 if p.token == token::Comma {
-                    p.eat(&token::Comma);
+                    panictry!(p.eat(&token::Comma));
                 }
             }
             StateNone => ()
@@ -194,12 +194,12 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt, sp: Span, tts: &[ast::TokenTree])
             match (&p.token, state.next(), state.next().next()) {
                 (&token::Colon, StateNone, _)   |
                 (&token::ModSep, _, StateNone) => {
-                    p.bump();
+                    panictry!(p.bump());
                     break 'statement;
                 }
                 (&token::Colon, st, _)   |
                 (&token::ModSep, _, st) => {
-                    p.bump();
+                    panictry!(p.bump());
                     state = st;
                 }
                 (&token::Eof, _, _) => break 'statement,

src/libsyntax/ext/base.rs

@@ -652,9 +652,9 @@ impl<'a> ExtCtxt<'a> {
     pub fn bt_push(&mut self, ei: ExpnInfo) {
         self.recursion_count += 1;
         if self.recursion_count > self.ecfg.recursion_limit {
-            self.span_fatal(ei.call_site,
+            panic!(self.span_fatal(ei.call_site,
                             &format!("recursion limit reached while expanding the macro `{}`",
-                                    ei.callee.name));
+                                    ei.callee.name)));
         }
 
         let mut call_site = ei.call_site;
@@ -699,7 +699,7 @@ impl<'a> ExtCtxt<'a> {
     ///   value doesn't have to match anything)
     pub fn span_fatal(&self, sp: Span, msg: &str) -> ! {
         self.print_backtrace();
-        self.parse_sess.span_diagnostic.span_fatal(sp, msg);
+        panic!(self.parse_sess.span_diagnostic.span_fatal(sp, msg));
     }
 
     /// Emit `msg` attached to `sp`, without immediately stopping
@@ -817,7 +817,7 @@ pub fn get_exprs_from_tts(cx: &mut ExtCtxt,
     let mut es = Vec::new();
     while p.token != token::Eof {
         es.push(cx.expander().fold_expr(p.parse_expr()));
-        if p.eat(&token::Comma) {
+        if panictry!(p.eat(&token::Comma)){
             continue;
         }
         if p.token != token::Eof {

src/libsyntax/ext/cfg.rs

@@ -29,7 +29,7 @@ pub fn expand_cfg<'cx>(cx: &mut ExtCtxt,
     let mut p = cx.new_parser_from_tts(tts);
     let cfg = p.parse_meta_item();
 
-    if !p.eat(&token::Eof) {
+    if !panictry!(p.eat(&token::Eof)){
         cx.span_err(sp, "expected 1 cfg-pattern");
         return DummyResult::expr(sp);
     }

src/libsyntax/ext/expand.rs

@@ -1684,7 +1684,7 @@ mod test {
 
     fn expand_crate_str(crate_str: String) -> ast::Crate {
         let ps = parse::new_parse_sess();
-        let crate_ast = string_to_parser(&ps, crate_str).parse_crate_mod();
+        let crate_ast = panictry!(string_to_parser(&ps, crate_str).parse_crate_mod());
         // the cfg argument actually does matter, here...
         expand_crate(&ps,test_ecfg(),vec!(),vec!(),crate_ast)
     }

src/libsyntax/ext/format.rs

@@ -92,7 +92,7 @@ fn parse_args(ecx: &mut ExtCtxt, sp: Span, tts: &[ast::TokenTree])
     let fmtstr = p.parse_expr();
     let mut named = false;
     while p.token != token::Eof {
-        if !p.eat(&token::Comma) {
+        if !panictry!(p.eat(&token::Comma)) {
             ecx.span_err(sp, "expected token: `,`");
             return None;
         }
@@ -101,7 +101,7 @@ fn parse_args(ecx: &mut ExtCtxt, sp: Span, tts: &[ast::TokenTree])
             named = true;
             let ident = match p.token {
                 token::Ident(i, _) => {
-                    p.bump();
+                    panictry!(p.bump());
                     i
                 }
                 _ if named => {
@@ -120,7 +120,7 @@ fn parse_args(ecx: &mut ExtCtxt, sp: Span, tts: &[ast::TokenTree])
             let interned_name = token::get_ident(ident);
             let name = &interned_name[..];
 
-            p.expect(&token::Eq);
+            panictry!(p.expect(&token::Eq));
             let e = p.parse_expr();
             match names.get(name) {
                 None => {}

src/libsyntax/ext/quote.rs

@@ -781,11 +781,11 @@ fn parse_arguments_to_quote(cx: &ExtCtxt, tts: &[ast::TokenTree])
     p.quote_depth += 1;
 
     let cx_expr = p.parse_expr();
-    if !p.eat(&token::Comma) {
-        p.fatal("expected token `,`");
+    if !panictry!(p.eat(&token::Comma)) {
+        panic!(p.fatal("expected token `,`"));
     }
 
-    let tts = p.parse_all_token_trees();
+    let tts = panictry!(p.parse_all_token_trees());
     p.abort_if_errors();
 
     (cx_expr, tts)

src/libsyntax/ext/source_util.rs

@@ -117,11 +117,11 @@ pub fn expand_include<'cx>(cx: &'cx mut ExtCtxt, sp: Span, tts: &[ast::TokenTree
             while self.p.token != token::Eof {
                 match self.p.parse_item() {
                     Some(item) => ret.push(item),
-                    None => self.p.span_fatal(
+                    None => panic!(self.p.span_fatal(
                         self.p.span,
                         &format!("expected item, found `{}`",
                                  self.p.this_token_to_string())
-                    )
+                    ))
                 }
             }
             Some(ret)

src/libsyntax/ext/tt/macro_parser.rs

@@ -226,10 +226,10 @@ pub fn nameize(p_s: &ParseSess, ms: &[TokenTree], res: &[Rc<NamedMatch>])
                     }
                     Occupied(..) => {
                         let string = token::get_ident(bind_name);
-                        p_s.span_diagnostic
+                        panic!(p_s.span_diagnostic
                            .span_fatal(sp,
                                        &format!("duplicated bind name: {}",
-                                               &string))
+                                               &string)))
                     }
                 }
             }
@@ -260,10 +260,10 @@ pub fn parse_or_else(sess: &ParseSess,
     match parse(sess, cfg, rdr, &ms[..]) {
         Success(m) => m,
         Failure(sp, str) => {
-            sess.span_diagnostic.span_fatal(sp, &str[..])
+            panic!(sess.span_diagnostic.span_fatal(sp, &str[..]))
         }
         Error(sp, str) => {
-            sess.span_diagnostic.span_fatal(sp, &str[..])
+            panic!(sess.span_diagnostic.span_fatal(sp, &str[..]))
         }
     }
 }
@@ -512,46 +512,46 @@ pub fn parse_nt(p: &mut Parser, sp: Span, name: &str) -> Nonterminal {
     match name {
         "tt" => {
             p.quote_depth += 1; //but in theory, non-quoted tts might be useful
-            let res = token::NtTT(P(p.parse_token_tree()));
+            let res = token::NtTT(P(panictry!(p.parse_token_tree())));
             p.quote_depth -= 1;
             return res;
         }
         _ => {}
     }
     // check at the beginning and the parser checks after each bump
-    p.check_unknown_macro_variable();
+    panictry!(p.check_unknown_macro_variable());
     match name {
       "item" => match p.parse_item() {
         Some(i) => token::NtItem(i),
-        None => p.fatal("expected an item keyword")
+        None => panic!(p.fatal("expected an item keyword"))
       },
-      "block" => token::NtBlock(p.parse_block()),
+      "block" => token::NtBlock(panictry!(p.parse_block())),
       "stmt" => match p.parse_stmt() {
         Some(s) => token::NtStmt(s),
-        None => p.fatal("expected a statement")
+        None => panic!(p.fatal("expected a statement"))
       },
       "pat" => token::NtPat(p.parse_pat()),
       "expr" => token::NtExpr(p.parse_expr()),
       "ty" => token::NtTy(p.parse_ty()),
       // this could be handled like a token, since it is one
       "ident" => match p.token {
-        token::Ident(sn,b) => { p.bump(); token::NtIdent(box sn,b) }
+        token::Ident(sn,b) => { panictry!(p.bump()); token::NtIdent(box sn,b) }
         _ => {
             let token_str = pprust::token_to_string(&p.token);
-            p.fatal(&format!("expected ident, found {}",
-                             &token_str[..]))
+            panic!(p.fatal(&format!("expected ident, found {}",
+                             &token_str[..])))
         }
       },
       "path" => {
-        token::NtPath(box p.parse_path(LifetimeAndTypesWithoutColons))
+        token::NtPath(box panictry!(p.parse_path(LifetimeAndTypesWithoutColons)))
       }
       "meta" => token::NtMeta(p.parse_meta_item()),
       _ => {
-          p.span_fatal_help(sp,
+          panic!(p.span_fatal_help(sp,
                             &format!("invalid fragment specifier `{}`", name),
                             "valid fragment specifiers are `ident`, `block`, \
                              `stmt`, `expr`, `pat`, `ty`, `path`, `meta`, `tt` \
-                             and `item`")
+                             and `item`"))
       }
     }
 }