From 683bcc02955f988ab2726f1b7708baa6c1e64b7b Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Tue, 3 May 2016 13:01:54 +1200 Subject: [PATCH 1/3] Use a Carrier trait with the `?` operator Allows use with `Option` and custom `Result`-like types. --- src/libcore/ops.rs | 125 ++++++++++++++++++++++ src/librustc/hir/lowering.rs | 130 ++++++++++++++--------- src/test/run-pass/try-operator-custom.rs | 73 +++++++++++++ src/test/run-pass/try-operator.rs | 17 +++ 4 files changed, 296 insertions(+), 49 deletions(-) create mode 100644 src/test/run-pass/try-operator-custom.rs diff --git a/src/libcore/ops.rs b/src/libcore/ops.rs index 4ac1b8394f450..a869b3c4f98c5 100644 --- a/src/libcore/ops.rs +++ b/src/libcore/ops.rs @@ -75,6 +75,8 @@ use cmp::PartialOrd; use fmt; use marker::{Sized, Unsize}; +use result::Result::{self, Ok, Err}; +use option::Option::{self, Some, None}; /// The `Drop` trait is used to run some code when a value goes out of scope. /// This is sometimes called a 'destructor'. @@ -2150,3 +2152,126 @@ pub trait BoxPlace : Place { /// Creates a globally fresh place. fn make_place() -> Self; } + +/// A trait for types which have success and error states and are meant to work +/// with the question mark operator. +/// When the `?` operator is used with a value, whether the value is in the +/// success or error state is determined by calling `translate`. +/// +/// This trait is **very** experimental, it will probably be iterated on heavily +/// before it is stabilised. Implementors should expect change. Users of `?` +/// should not rely on any implementations of `Carrier` other than `Result`, +/// i.e., you should not expect `?` to continue to work with `Option`, etc. +#[unstable(feature = "question_mark_carrier", issue = "31436")] +pub trait Carrier { + /// The type of the value when computation succeeds. + type Success; + /// The type of the value when computation errors out. + type Error; + + /// Create a `Carrier` from a success value. + fn from_success(Self::Success) -> Self; + + /// Create a `Carrier` from an error value. + fn from_error(Self::Error) -> Self; + + /// Translate this `Carrier` to another implementation of `Carrier` with the + /// same associated types. + fn translate(self) -> T where T: Carrier; +} + +#[unstable(feature = "question_mark_carrier", issue = "31436")] +impl Carrier for Result { + type Success = U; + type Error = V; + + fn from_success(u: U) -> Result { + Ok(u) + } + + fn from_error(e: V) -> Result { + Err(e) + } + + fn translate(self) -> T + where T: Carrier + { + match self { + Ok(u) => T::from_success(u), + Err(e) => T::from_error(e), + } + } +} + +#[unstable(feature = "question_mark_carrier", issue = "31436")] +impl Carrier for Option { + type Success = U; + type Error = (); + + fn from_success(u: U) -> Option { + Some(u) + } + + fn from_error(_: ()) -> Option { + None + } + + fn translate(self) -> T + where T: Carrier + { + match self { + Some(u) => T::from_success(u), + None => T::from_error(()), + } + } +} + +// Implementing Carrier for bools means it's easy to write short-circuiting +// functions. E.g., +// ``` +// fn foo() -> bool { +// if !(f() || g()) { +// return false; +// } +// +// some_computation(); +// if h() { +// return false; +// } +// +// more_computation(); +// i() +// } +// ``` +// becomes +// ``` +// fn foo() -> bool { +// (f() || g())?; +// some_computation(); +// (!h())?; +// more_computation(); +// i() +// } +// ``` +#[unstable(feature = "question_mark_carrier", issue = "31436")] +impl Carrier for bool { + type Success = (); + type Error = (); + + fn from_success(_: ()) -> bool { + true + } + + fn from_error(_: ()) -> bool { + false + } + + fn translate(self) -> T + where T: Carrier + { + match self { + true => T::from_success(()), + false => T::from_error(()), + } + } +} diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index c2b211238b2f1..b45610c3fe820 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -966,7 +966,7 @@ impl<'a> LoweringContext<'a> { let inplace_finalize = ["ops", "InPlace", "finalize"]; let make_call = |this: &mut LoweringContext, p, args| { - let path = this.core_path(e.span, p); + let path = this.std_path(e.span, p); let path = this.expr_path(path, ThinVec::new()); this.expr_call(e.span, path, args) }; @@ -1159,15 +1159,13 @@ impl<'a> LoweringContext<'a> { ast_expr: &Expr, path: &[&str], fields: &[(&str, &P)]) -> P { - let strs = this.std_path(&iter::once(&"ops") - .chain(path) - .map(|s| *s) - .collect::>()); - - let structpath = this.path_global(ast_expr.span, strs); + let struct_path = this.std_path(ast_expr.span, + &iter::once(&"ops").chain(path) + .map(|s| *s) + .collect::>()); let hir_expr = if fields.len() == 0 { - this.expr_path(structpath, ast_expr.attrs.clone()) + this.expr_path(struct_path, ast_expr.attrs.clone()) } else { let fields = fields.into_iter().map(|&(s, e)| { let expr = this.lower_expr(&e); @@ -1180,7 +1178,7 @@ impl<'a> LoweringContext<'a> { }).collect(); let attrs = ast_expr.attrs.clone(); - this.expr_struct(ast_expr.span, structpath, fields, None, attrs) + this.expr_struct(ast_expr.span, struct_path, fields, None, attrs) }; this.signal_block_expr(hir_vec![], @@ -1463,11 +1461,7 @@ impl<'a> LoweringContext<'a> { // `match ::std::iter::Iterator::next(&mut iter) { ... }` let match_expr = { - let next_path = { - let strs = self.std_path(&["iter", "Iterator", "next"]); - - self.path_global(e.span, strs) - }; + let next_path = self.std_path(e.span, &["iter", "Iterator", "next"]); let iter = self.expr_ident(e.span, iter, iter_pat.id); let ref_mut_iter = self.expr_mut_addr_of(e.span, iter); let next_path = self.expr_path(next_path, ThinVec::new()); @@ -1494,11 +1488,8 @@ impl<'a> LoweringContext<'a> { // `match ::std::iter::IntoIterator::into_iter() { ... }` let into_iter_expr = { - let into_iter_path = { - let strs = self.std_path(&["iter", "IntoIterator", "into_iter"]); - - self.path_global(e.span, strs) - }; + let into_iter_path = self.std_path(e.span, + &["iter", "IntoIterator", "into_iter"]); let into_iter = self.expr_path(into_iter_path, ThinVec::new()); self.expr_call(e.span, into_iter, hir_vec![head]) @@ -1527,16 +1518,32 @@ impl<'a> LoweringContext<'a> { // to: // // { - // match { + // match { Carrier::translate( { } ) } { // Ok(val) => val, - // Err(err) => { - // return Err(From::from(err)) - // } + // Err(err) => { return Carrier::from_error(From::from(err)); } // } // } - // expand - let sub_expr = self.lower_expr(sub_expr); + // { Carrier::translate( { } ) } + let discr = { + // expand + let sub_expr = self.lower_expr(sub_expr); + let sub_expr = self.signal_block_expr(hir_vec![], + sub_expr, + e.span, + hir::PopUnstableBlock, + ThinVec::new()); + + let path = self.std_path(e.span, &["ops", "Carrier", "translate"]); + let path = self.expr_path(path, ThinVec::new()); + let call = self.expr_call(e.span, path, hir_vec![sub_expr]); + + self.signal_block_expr(hir_vec![], + call, + e.span, + hir::PushUnstableBlock, + ThinVec::new()) + }; // Ok(val) => val let ok_arm = { @@ -1548,32 +1555,35 @@ impl<'a> LoweringContext<'a> { self.arm(hir_vec![ok_pat], val_expr) }; - // Err(err) => return Err(From::from(err)) + // Err(err) => { return Carrier::from_error(From::from(err)); } let err_arm = { let err_ident = self.str_to_ident("err"); let err_local = self.pat_ident(e.span, err_ident); let from_expr = { - let path = self.std_path(&["convert", "From", "from"]); - let path = self.path_global(e.span, path); + let path = self.std_path(e.span, &["convert", "From", "from"]); let from = self.expr_path(path, ThinVec::new()); let err_expr = self.expr_ident(e.span, err_ident, err_local.id); self.expr_call(e.span, from, hir_vec![err_expr]) }; - let err_expr = { - let path = self.std_path(&["result", "Result", "Err"]); - let path = self.path_global(e.span, path); - let err_ctor = self.expr_path(path, ThinVec::new()); - self.expr_call(e.span, err_ctor, hir_vec![from_expr]) + let from_err_expr = { + let path = self.std_path(e.span, &["ops", "Carrier", "from_error"]); + let from_err = self.expr_path(path, ThinVec::new()); + self.expr_call(e.span, from_err, hir_vec![from_expr]) }; - let err_pat = self.pat_err(e.span, err_local); + let ret_expr = self.expr(e.span, - hir::Expr_::ExprRet(Some(err_expr)), - ThinVec::new()); - self.arm(hir_vec![err_pat], ret_expr) + hir::Expr_::ExprRet(Some(from_err_expr)), + ThinVec::new()); + let ret_stmt = self.stmt_expr(ret_expr); + let block = self.signal_block_stmt(ret_stmt, e.span, + hir::PushUnstableBlock, ThinVec::new()); + + let err_pat = self.pat_err(e.span, err_local); + self.arm(hir_vec![err_pat], block) }; - return self.expr_match(e.span, sub_expr, hir_vec![err_arm, ok_arm], + return self.expr_match(e.span, discr, hir_vec![err_arm, ok_arm], hir::MatchSource::TryDesugar); } @@ -1787,6 +1797,15 @@ impl<'a> LoweringContext<'a> { (respan(sp, hir::StmtDecl(P(decl), self.next_id())), pat_id) } + // Turns `` into `;`, note that this produces a StmtSemi, not a + // StmtExpr. + fn stmt_expr(&self, expr: P) -> hir::Stmt { + hir::Stmt { + span: expr.span, + node: hir::StmtSemi(expr, self.next_id()), + } + } + fn block_expr(&mut self, expr: P) -> P { self.block_all(expr.span, hir::HirVec::new(), Some(expr)) } @@ -1803,26 +1822,22 @@ impl<'a> LoweringContext<'a> { } fn pat_ok(&mut self, span: Span, pat: P) -> P { - let ok = self.std_path(&["result", "Result", "Ok"]); - let path = self.path_global(span, ok); + let path = self.std_path(span, &["result", "Result", "Ok"]); self.pat_enum(span, path, hir_vec![pat]) } fn pat_err(&mut self, span: Span, pat: P) -> P { - let err = self.std_path(&["result", "Result", "Err"]); - let path = self.path_global(span, err); + let path = self.std_path(span, &["result", "Result", "Err"]); self.pat_enum(span, path, hir_vec![pat]) } fn pat_some(&mut self, span: Span, pat: P) -> P { - let some = self.std_path(&["option", "Option", "Some"]); - let path = self.path_global(span, some); + let path = self.std_path(span, &["option", "Option", "Some"]); self.pat_enum(span, path, hir_vec![pat]) } fn pat_none(&mut self, span: Span) -> P { - let none = self.std_path(&["option", "Option", "None"]); - let path = self.path_global(span, none); + let path = self.std_path(span, &["option", "Option", "None"]); self.pat_enum(span, path, hir_vec![]) } @@ -1920,7 +1935,7 @@ impl<'a> LoweringContext<'a> { } } - fn std_path(&mut self, components: &[&str]) -> Vec { + fn std_path_components(&mut self, components: &[&str]) -> Vec { let mut v = Vec::new(); if let Some(s) = self.crate_root { v.push(token::intern(s)); @@ -1931,8 +1946,8 @@ impl<'a> LoweringContext<'a> { // Given suffix ["b","c","d"], returns path `::std::b::c::d` when // `fld.cx.use_std`, and `::core::b::c::d` otherwise. - fn core_path(&mut self, span: Span, components: &[&str]) -> hir::Path { - let idents = self.std_path(components); + fn std_path(&mut self, span: Span, components: &[&str]) -> hir::Path { + let idents = self.std_path_components(components); self.path_global(span, idents) } @@ -1953,4 +1968,21 @@ impl<'a> LoweringContext<'a> { }); self.expr_block(block, attrs) } + + fn signal_block_stmt(&mut self, + stmt: hir::Stmt, + span: Span, + rule: hir::BlockCheckMode, + attrs: ThinVec) + -> P { + let id = self.next_id(); + let block = P(hir::Block { + rules: rule, + span: span, + id: id, + stmts: hir_vec![stmt], + expr: None, + }); + self.expr_block(block, attrs) + } } diff --git a/src/test/run-pass/try-operator-custom.rs b/src/test/run-pass/try-operator-custom.rs new file mode 100644 index 0000000000000..577d19a58960d --- /dev/null +++ b/src/test/run-pass/try-operator-custom.rs @@ -0,0 +1,73 @@ +// Copyright 2016 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(question_mark, question_mark_carrier)] + +use std::ops::Carrier; + +enum MyResult { + Awesome(T), + Terrible(U) +} + +impl Carrier for MyResult { + type Success = U; + type Error = V; + + fn from_success(u: U) -> MyResult { + MyResult::Awesome(u) + } + + fn from_error(e: V) -> MyResult { + MyResult::Terrible(e) + } + + fn translate(self) -> T + where T: Carrier + { + match self { + MyResult::Awesome(u) => T::from_success(u), + MyResult::Terrible(e) => T::from_error(e), + } + } +} + +fn f(x: i32) -> Result { + if x == 0 { + Ok(42) + } else { + let y = g(x)?; + Ok(y) + } +} + +fn g(x: i32) -> MyResult { + let _y = f(x - 1)?; + MyResult::Terrible("Hello".to_owned()) +} + +fn h() -> MyResult { + let a: Result = Err("Hello"); + let b = a?; + MyResult::Awesome(b) +} + +fn i() -> MyResult { + let a: MyResult = MyResult::Terrible("Hello"); + let b = a?; + MyResult::Awesome(b) +} + +fn main() { + assert!(f(0) == Ok(42)); + assert!(f(10) == Err("Hello".to_owned())); + let _ = h(); + let _ = i(); +} diff --git a/src/test/run-pass/try-operator.rs b/src/test/run-pass/try-operator.rs index de5ccf09c5923..8076e00fd08ac 100644 --- a/src/test/run-pass/try-operator.rs +++ b/src/test/run-pass/try-operator.rs @@ -144,6 +144,23 @@ fn merge_error() -> Result { Ok(s.parse::()? + 1) } +fn option() -> Option { + let x = Some(42); + let y = x?; + Some(y + 2) +} + +fn bool() -> bool { + let x = true; + let y = false; + let z = true; + + (x || y)?; + let a: () = z?; + x?; + true +} + fn main() { assert_eq!(Ok(3), on_method()); From 5aa89d8bf6dcf3f7c08f3db8f48b54676b6203b8 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Tue, 9 Aug 2016 17:54:24 +1200 Subject: [PATCH 2/3] Remove the Option and bool impls for carrier and add a dummy impl The dummy impl should ensure the same type checking behaviour as having other (real) Carrier impls. --- src/libcore/ops.rs | 67 ++----------------- .../compile-fail/question-mark-type-infer.rs | 27 ++++++++ src/test/run-pass/try-operator.rs | 17 ----- 3 files changed, 34 insertions(+), 77 deletions(-) create mode 100644 src/test/compile-fail/question-mark-type-infer.rs diff --git a/src/libcore/ops.rs b/src/libcore/ops.rs index a869b3c4f98c5..0e32d71172b04 100644 --- a/src/libcore/ops.rs +++ b/src/libcore/ops.rs @@ -76,7 +76,6 @@ use cmp::PartialOrd; use fmt; use marker::{Sized, Unsize}; use result::Result::{self, Ok, Err}; -use option::Option::{self, Some, None}; /// The `Drop` trait is used to run some code when a value goes out of scope. /// This is sometimes called a 'destructor'. @@ -2203,75 +2202,23 @@ impl Carrier for Result { } } -#[unstable(feature = "question_mark_carrier", issue = "31436")] -impl Carrier for Option { - type Success = U; - type Error = (); - - fn from_success(u: U) -> Option { - Some(u) - } +struct _DummyErrorType; - fn from_error(_: ()) -> Option { - None - } - - fn translate(self) -> T - where T: Carrier - { - match self { - Some(u) => T::from_success(u), - None => T::from_error(()), - } - } -} - -// Implementing Carrier for bools means it's easy to write short-circuiting -// functions. E.g., -// ``` -// fn foo() -> bool { -// if !(f() || g()) { -// return false; -// } -// -// some_computation(); -// if h() { -// return false; -// } -// -// more_computation(); -// i() -// } -// ``` -// becomes -// ``` -// fn foo() -> bool { -// (f() || g())?; -// some_computation(); -// (!h())?; -// more_computation(); -// i() -// } -// ``` -#[unstable(feature = "question_mark_carrier", issue = "31436")] -impl Carrier for bool { +impl Carrier for _DummyErrorType { type Success = (); type Error = (); - fn from_success(_: ()) -> bool { - true + fn from_success(_: ()) -> _DummyErrorType { + _DummyErrorType } - fn from_error(_: ()) -> bool { - false + fn from_error(_: ()) -> _DummyErrorType { + _DummyErrorType } fn translate(self) -> T where T: Carrier { - match self { - true => T::from_success(()), - false => T::from_error(()), - } + T::from_success(()) } } diff --git a/src/test/compile-fail/question-mark-type-infer.rs b/src/test/compile-fail/question-mark-type-infer.rs new file mode 100644 index 0000000000000..e15c9af41e082 --- /dev/null +++ b/src/test/compile-fail/question-mark-type-infer.rs @@ -0,0 +1,27 @@ +// Copyright 2016 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(question_mark, question_mark_carrier)] + +// Test that type inference fails where there are multiple possible return types +// for the `?` operator. + +fn f(x: &i32) -> Result { + Ok(*x) +} + +fn g() -> Result, ()> { + let l = [1, 2, 3, 4]; + l.iter().map(f).collect()? //~ ERROR type annotations required: cannot resolve +} + +fn main() { + g(); +} diff --git a/src/test/run-pass/try-operator.rs b/src/test/run-pass/try-operator.rs index 8076e00fd08ac..de5ccf09c5923 100644 --- a/src/test/run-pass/try-operator.rs +++ b/src/test/run-pass/try-operator.rs @@ -144,23 +144,6 @@ fn merge_error() -> Result { Ok(s.parse::()? + 1) } -fn option() -> Option { - let x = Some(42); - let y = x?; - Some(y + 2) -} - -fn bool() -> bool { - let x = true; - let y = false; - let z = true; - - (x || y)?; - let a: () = z?; - x?; - true -} - fn main() { assert_eq!(Ok(3), on_method()); From c32456da8f14a7c4206278c55b90a535c4d98e20 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Mon, 22 Aug 2016 09:31:26 +1200 Subject: [PATCH 3/3] Fix type error with `?` in existing code. --- src/librustc/ty/relate.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc/ty/relate.rs b/src/librustc/ty/relate.rs index abf863f953664..8975a799be143 100644 --- a/src/librustc/ty/relate.rs +++ b/src/librustc/ty/relate.rs @@ -151,13 +151,13 @@ pub fn relate_substs<'a, 'gcx, 'tcx, R>(relation: &mut R, let b_ty = &b_subst.types[i]; let variance = variances.map_or(ty::Invariant, |v| v.types[i]); relation.relate_with_variance(variance, a_ty, b_ty) - }).collect()?; + }).collect::>()?; let regions = a_subst.regions.iter().enumerate().map(|(i, a_r)| { let b_r = &b_subst.regions[i]; let variance = variances.map_or(ty::Invariant, |v| v.regions[i]); relation.relate_with_variance(variance, a_r, b_r) - }).collect()?; + }).collect::>()?; Ok(Substs::new(tcx, types, regions)) }