Auto merge of #49371 - scottmcm:catch-wrapping, r=nikomatsakis

Add ok-wrapping to catch blocks, per RFC

Updates the `catch{}` lowering to wrap the result in `Try::from_ok`.

r? @nikomatsakis

Fixes #41414
Fixes #43818

Author: bors

src/doc/unstable-book/src/language-features/catch-expr.md

@@ -15,16 +15,16 @@ expression creates a new scope one can use the `?` operator in.
 use std::num::ParseIntError;
 
 let result: Result<i32, ParseIntError> = do catch {
-    Ok("1".parse::<i32>()?
+    "1".parse::<i32>()?
         + "2".parse::<i32>()?
-        + "3".parse::<i32>()?)
+        + "3".parse::<i32>()?
 };
 assert_eq!(result, Ok(6));
 
 let result: Result<i32, ParseIntError> = do catch {
-    Ok("1".parse::<i32>()?
+    "1".parse::<i32>()?
         + "foo".parse::<i32>()?
-        + "3".parse::<i32>()?)
+        + "3".parse::<i32>()?
 };
 assert!(result.is_err());
 ```

src/librustc/hir/lowering.rs

@@ -3010,7 +3010,28 @@ impl<'a> LoweringContext<'a> {
                 )
             }),
             ExprKind::Catch(ref body) => {
-                self.with_catch_scope(body.id, |this| hir::ExprBlock(this.lower_block(body, true)))
+                self.with_catch_scope(body.id, |this| {
+                    let unstable_span =
+                        this.allow_internal_unstable(CompilerDesugaringKind::Catch, body.span);
+                    let mut block = this.lower_block(body, true).into_inner();
+                    let tail = block.expr.take().map_or_else(
+                        || {
+                            let LoweredNodeId { node_id, hir_id } = this.next_id();
+                            let span = this.sess.codemap().end_point(unstable_span);
+                            hir::Expr {
+                                id: node_id,
+                                span,
+                                node: hir::ExprTup(hir_vec![]),
+                                attrs: ThinVec::new(),
+                                hir_id,
+                            }
+                        },
+                        |x: P<hir::Expr>| x.into_inner(),
+                    );
+                    block.expr = Some(this.wrap_in_try_constructor(
+                        "from_ok", tail, unstable_span));
+                    hir::ExprBlock(P(block))
+                })
             }
             ExprKind::Match(ref expr, ref arms) => hir::ExprMatch(
                 P(self.lower_expr(expr)),
@@ -3539,12 +3560,8 @@ impl<'a> LoweringContext<'a> {
 
                         self.expr_call(e.span, from, hir_vec![err_expr])
                     };
-                    let from_err_expr = {
-                        let path = &["ops", "Try", "from_error"];
-                        let from_err = P(self.expr_std_path(unstable_span, path, ThinVec::new()));
-                        P(self.expr_call(e.span, from_err, hir_vec![from_expr]))
-                    };
-
+                    let from_err_expr =
+                        self.wrap_in_try_constructor("from_error", from_expr, unstable_span);
                     let thin_attrs = ThinVec::from(attrs);
                     let catch_scope = self.catch_scopes.last().map(|x| *x);
                     let ret_expr = if let Some(catch_node) = catch_scope {
@@ -4079,6 +4096,18 @@ impl<'a> LoweringContext<'a> {
             )
         }
     }
+
+    fn wrap_in_try_constructor(
+        &mut self,
+        method: &'static str,
+        e: hir::Expr,
+        unstable_span: Span,
+    ) -> P<hir::Expr> {
+        let path = &["ops", "Try", method];
+        let from_err = P(self.expr_std_path(unstable_span, path,
+                                            ThinVec::new()));
+        P(self.expr_call(e.span, from_err, hir_vec![e]))
+    }
 }
 
 fn body_ids(bodies: &BTreeMap<hir::BodyId, hir::Body>) -> Vec<hir::BodyId> {

src/librustc/ich/impls_syntax.rs

@@ -372,7 +372,8 @@ impl_stable_hash_for!(enum ::syntax_pos::hygiene::ExpnFormat {
 
 impl_stable_hash_for!(enum ::syntax_pos::hygiene::CompilerDesugaringKind {
     DotFill,
-    QuestionMark
+    QuestionMark,
+    Catch
 });
 
 impl_stable_hash_for!(enum ::syntax_pos::FileName {

src/librustc_mir/borrow_check/nll/mod.rs

@@ -202,11 +202,11 @@ fn dump_mir_results<'a, 'gcx, 'tcx>(
     });
 
     // Also dump the inference graph constraints as a graphviz file.
-    let _: io::Result<()> = do catch {
+    let _: io::Result<()> = do_catch! {{
         let mut file =
             pretty::create_dump_file(infcx.tcx, "regioncx.dot", None, "nll", &0, source)?;
-        regioncx.dump_graphviz(&mut file)
-    };
+        regioncx.dump_graphviz(&mut file)?;
+    }};
 }
 
 fn dump_annotation<'a, 'gcx, 'tcx>(

src/librustc_mir/lib.rs

@@ -32,6 +32,7 @@ Rust MIR: a lowered representation of Rust. Also: an experiment!
 #![feature(nonzero)]
 #![feature(inclusive_range_fields)]
 #![feature(crate_visibility_modifier)]
+#![cfg_attr(stage0, feature(try_trait))]
 
 extern crate arena;
 #[macro_use]
@@ -53,6 +54,16 @@ extern crate log_settings;
 extern crate rustc_apfloat;
 extern crate byteorder;
 
+#[cfg(stage0)]
+macro_rules! do_catch {
+  ($t:expr) => { (|| ::std::ops::Try::from_ok($t) )() }
+}
+
+#[cfg(not(stage0))]
+macro_rules! do_catch {
+  ($t:expr) => { do catch { $t } }
+}
+
 mod diagnostics;
 
 mod borrow_check;

src/librustc_mir/util/pretty.rs

@@ -137,7 +137,7 @@ fn dump_matched_mir_node<'a, 'gcx, 'tcx, F>(
 ) where
     F: FnMut(PassWhere, &mut dyn Write) -> io::Result<()>,
 {
-    let _: io::Result<()> = do catch {
+    let _: io::Result<()> = do_catch! {{
         let mut file = create_dump_file(tcx, "mir", pass_num, pass_name, disambiguator, source)?;
         writeln!(file, "// MIR for `{}`", node_path)?;
         writeln!(file, "// source = {:?}", source)?;
@@ -150,16 +150,14 @@ fn dump_matched_mir_node<'a, 'gcx, 'tcx, F>(
         extra_data(PassWhere::BeforeCFG, &mut file)?;
         write_mir_fn(tcx, source, mir, &mut extra_data, &mut file)?;
         extra_data(PassWhere::AfterCFG, &mut file)?;
-        Ok(())
-    };
+    }};
 
     if tcx.sess.opts.debugging_opts.dump_mir_graphviz {
-        let _: io::Result<()> = do catch {
+        let _: io::Result<()> = do_catch! {{
             let mut file =
                 create_dump_file(tcx, "dot", pass_num, pass_name, disambiguator, source)?;
             write_mir_fn_graphviz(tcx, source.def_id, mir, &mut file)?;
-            Ok(())
-        };
+        }};
     }
 }
 

src/libsyntax_pos/hygiene.rs

@@ -432,6 +432,7 @@ pub enum ExpnFormat {
 pub enum CompilerDesugaringKind {
     DotFill,
     QuestionMark,
+    Catch,
 }
 
 impl CompilerDesugaringKind {
@@ -440,6 +441,7 @@ impl CompilerDesugaringKind {
         let s = match *self {
             DotFill => "...",
             QuestionMark => "?",
+            Catch => "do catch",
         };
         Symbol::intern(s)
     }

src/test/compile-fail/catch-bad-lifetime.rs

@@ -21,7 +21,6 @@ pub fn main() {
             //~^ ERROR `my_string` does not live long enough
             Err(my_str) ?;
             Err("") ?;
-            Ok(())
         };
     }
 
@@ -32,7 +31,6 @@ pub fn main() {
         let mut j: Result<(), &mut i32> = do catch {
             Err(k) ?;
             i = 10; //~ ERROR cannot assign to `i` because it is borrowed
-            Ok(())
         };
         ::std::mem::drop(k); //~ ERROR use of moved value: `k`
         i = 40; //~ ERROR cannot assign to `i` because it is borrowed

src/test/compile-fail/catch-bad-type.rs

@@ -11,11 +11,18 @@
 #![feature(catch_expr)]
 
 pub fn main() {
-    let res: Result<i32, i32> = do catch {
+    let res: Result<u32, i32> = do catch {
         Err("")?; //~ ERROR the trait bound `i32: std::convert::From<&str>` is not satisfied
-        Ok(5)
+        5
     };
+
     let res: Result<i32, i32> = do catch {
-        Ok("") //~ mismatched types
+        "" //~ ERROR type mismatch
     };
+
+    let res: Result<i32, i32> = do catch { }; //~ ERROR type mismatch
+
+    let res: () = do catch { }; //~ the trait bound `(): std::ops::Try` is not satisfied
+
+    let res: i32 = do catch { 5 }; //~ ERROR the trait bound `i32: std::ops::Try` is not satisfied
 }

src/test/compile-fail/catch-maybe-bad-lifetime.rs

@@ -17,7 +17,7 @@ pub fn main() {
         let mut i = 222;
         let x: Result<&i32, ()> = do catch {
             Err(())?;
-            Ok(&i)
+            &i
         };
         x.ok().cloned();
         i = 0; //~ ERROR cannot assign to `i` because it is borrowed
@@ -29,7 +29,6 @@ pub fn main() {
         let _y: Result<(), ()> = do catch {
             Err(())?;
             ::std::mem::drop(x);
-            Ok(())
         };
         println!("{}", x); //~ ERROR use of moved value: `x`
     }
@@ -42,7 +41,6 @@ pub fn main() {
         let x: Result<(), ()> = do catch {
             Err(())?;
             j = &i;
-            Ok(())
         };
         i = 0; //~ ERROR cannot assign to `i` because it is borrowed
         let _ = i;

src/test/compile-fail/catch-opt-init.rs

@@ -19,7 +19,6 @@ pub fn main() {
         cfg_res = 5;
         Ok::<(), ()>(())?;
         use_val(cfg_res);
-        Ok(())
     };
     assert_eq!(cfg_res, 5); //~ ERROR use of possibly uninitialized variable
 }

src/test/run-pass/catch-expr.rs

@@ -13,11 +13,11 @@
 struct catch {}
 
 pub fn main() {
-    let catch_result = do catch {
+    let catch_result: Option<_> = do catch {
         let x = 5;
         x
     };
-    assert_eq!(catch_result, 5);
+    assert_eq!(catch_result, Some(5));
 
     let mut catch = true;
     while catch { catch = false; }
@@ -30,51 +30,50 @@ pub fn main() {
         _ => {}
     };
 
-    let catch_err = do catch {
+    let catch_err: Result<_, i32> = do catch {
         Err(22)?;
-        Ok(1)
+        1
     };
     assert_eq!(catch_err, Err(22));
 
     let catch_okay: Result<i32, i32> = do catch {
         if false { Err(25)?; }
         Ok::<(), i32>(())?;
-        Ok(28)
+        28
     };
     assert_eq!(catch_okay, Ok(28));
 
     let catch_from_loop: Result<i32, i32> = do catch {
         for i in 0..10 {
             if i < 5 { Ok::<i32, i32>(i)?; } else { Err(i)?; }
         }
-        Ok(22)
+        22
     };
     assert_eq!(catch_from_loop, Err(5));
 
     let cfg_init;
     let _res: Result<(), ()> = do catch {
         cfg_init = 5;
-        Ok(())
     };
     assert_eq!(cfg_init, 5);
 
     let cfg_init_2;
     let _res: Result<(), ()> = do catch {
         cfg_init_2 = 6;
         Err(())?;
-        Ok(())
     };
     assert_eq!(cfg_init_2, 6);
 
     let my_string = "test".to_string();
     let res: Result<&str, ()> = do catch {
-        Ok(&my_string)
+        // Unfortunately, deref doesn't fire here (#49356)
+        &my_string[..]
     };
     assert_eq!(res, Ok("test"));
 
-    do catch {
-        ()
-    }
+    let my_opt: Option<_> = do catch { () };
+    assert_eq!(my_opt, Some(()));
 
-    ();
+    let my_opt: Option<_> = do catch { };
+    assert_eq!(my_opt, Some(()));
 }

src/test/ui/catch-block-type-error.rs

@@ -0,0 +1,26 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![feature(catch_expr)]
+
+fn foo() -> Option<()> { Some(()) }
+
+fn main() {
+    let _: Option<f32> = do catch {
+        foo()?;
+        42
+        //~^ ERROR type mismatch
+    };
+
+    let _: Option<i32> = do catch {
+        foo()?;
+    };
+    //~^ ERROR type mismatch
+}

src/test/ui/catch-block-type-error.stderr

@@ -0,0 +1,21 @@
+error[E0271]: type mismatch resolving `<std::option::Option<f32> as std::ops::Try>::Ok == {integer}`
+  --> $DIR/catch-block-type-error.rs:18:9
+   |
+LL |         42
+   |         ^^ expected f32, found integral variable
+   |
+   = note: expected type `f32`
+              found type `{integer}`
+
+error[E0271]: type mismatch resolving `<std::option::Option<i32> as std::ops::Try>::Ok == ()`
+  --> $DIR/catch-block-type-error.rs:24:5
+   |
+LL |     };
+   |     ^ expected i32, found ()
+   |
+   = note: expected type `i32`
+              found type `()`
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0271`.