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

Add ok-wrapping to catch blocks, per RFC #49371

Merged
merged 4 commits into from
Apr 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/doc/unstable-book/src/language-features/catch-expr.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>()?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

huzzah

+ "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());
```
43 changes: 36 additions & 7 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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> {
Expand Down
3 changes: 2 additions & 1 deletion src/librustc/ich/impls_syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_mir/borrow_check/nll/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>(
Expand Down
11 changes: 11 additions & 0 deletions src/librustc_mir/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,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]
Expand All @@ -54,6 +55,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;
Expand Down
10 changes: 4 additions & 6 deletions src/librustc_mir/util/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand All @@ -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(())
};
}};
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/libsyntax_pos/hygiene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ pub enum ExpnFormat {
pub enum CompilerDesugaringKind {
DotFill,
QuestionMark,
Catch,
}

impl CompilerDesugaringKind {
Expand All @@ -440,6 +441,7 @@ impl CompilerDesugaringKind {
let s = match *self {
DotFill => "...",
QuestionMark => "?",
Catch => "do catch",
};
Symbol::intern(s)
}
Expand Down
2 changes: 0 additions & 2 deletions src/test/compile-fail/catch-bad-lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ pub fn main() {
//~^ ERROR `my_string` does not live long enough
Err(my_str) ?;
Err("") ?;
Ok(())
};
}

Expand All @@ -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
Expand Down
13 changes: 10 additions & 3 deletions src/test/compile-fail/catch-bad-type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
4 changes: 1 addition & 3 deletions src/test/compile-fail/catch-maybe-bad-lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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`
}
Expand All @@ -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;
Expand Down
1 change: 0 additions & 1 deletion src/test/compile-fail/catch-opt-init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
25 changes: 12 additions & 13 deletions src/test/run-pass/catch-expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand All @@ -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[..]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting.

Copy link
Member Author

@scottmcm scottmcm Apr 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I filed #49356 about this. (Edit: Which I put in the comment; duh.) It feels to me like it ought to work, since the type needed here is completely determined by the context.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can imagine why it fails, we might be able to improve it.

};
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(()));
}
26 changes: 26 additions & 0 deletions src/test/ui/catch-block-type-error.rs
Original file line number Diff line number Diff line change
@@ -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
}
21 changes: 21 additions & 0 deletions src/test/ui/catch-block-type-error.stderr
Original file line number Diff line number Diff line change
@@ -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`.