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.

Sign up for GitHub

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

Carrier trait for testing, DON'T LAND #35056

Closed
wants to merge 4 commits into from
Closed

Carrier trait for testing, DON'T LAND #35056

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c6fa5ee
Use a Carrier trait with the `?` operator
nrc May 3, 2016
3f9f574
Expand try! to ? breaking change - DO NOT LAND!
nrc Jul 26, 2016
5f891c5
Fixes
nrc Aug 9, 2016
6471be5
Remove the Option and bool impls for carrier and add a dummy impl
nrc Aug 9, 2016
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
7 changes: 7 additions & 0 deletions src/libcore/macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ macro_rules! debug_assert_eq {
/// Ok(())
/// }
/// ```
#[cfg(stage0)]
#[macro_export]
#[stable(feature = "rust1", since = "1.0.0")]
macro_rules! try {
Expand All @@ -228,6 +229,12 @@ macro_rules! try {
}
})
}
#[cfg(not(stage0))]
#[macro_export]
#[stable(feature = "rust1", since = "1.0.0")]
macro_rules! try {
($expr:expr) => ($expr?)
Copy link
Contributor

Choose a reason for hiding this comment

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

to be clear, this change (updating try!) was something we wanted to do just to evaluate whether we could stabilize without the carrier trait. Seems clear we cannot -- and so should revert this change, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that is what I meant in my last comment

}

/// Use the `format!` syntax to write data into a buffer.
///
Expand Down
72 changes: 72 additions & 0 deletions src/libcore/ops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
use cmp::PartialOrd;
use fmt;
use marker::{Sized, Unsize};
use result::Result::{self, Ok, Err};

/// The `Drop` trait is used to run some code when a value goes out of scope.
/// This is sometimes called a 'destructor'.
Expand Down Expand Up @@ -2145,3 +2146,74 @@ pub trait BoxPlace<Data: ?Sized> : Place<Data> {
/// 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<T>(self) -> T where T: Carrier<Success=Self::Success, Error=Self::Error>;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should discuss how we could enforce a rule that ensures that the ? operator won't convert (e.g.) a Result to an Option. Lacking HKT, it seems like this might be best done by some special type rules that apply to ?, which probably means we would want to preserve ? into the HIR and have special treatment when lowering to MIR, or something?

Copy link
Member

Choose a reason for hiding this comment

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

If you enforce that rule, then T = Self. There's no From support here AFAICT.

Copy link
Contributor

Choose a reason for hiding this comment

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

@eddyb

If you enforce that rule, then T = Self. There's no From support here AFAICT.

Well, that depends on the rule I guess. =) See my extended comment below for some more precise thoughts.

}

#[unstable(feature = "question_mark_carrier", issue = "31436")]
impl<U, V> Carrier for Result<U, V> {
type Success = U;
type Error = V;

fn from_success(u: U) -> Result<U, V> {
Ok(u)
}

fn from_error(e: V) -> Result<U, V> {
Err(e)
}

fn translate<T>(self) -> T
where T: Carrier<Success=U, Error=V>
{
match self {
Ok(u) => T::from_success(u),
Err(e) => T::from_error(e),
}
}
}

struct _DummyErrorType;

impl Carrier for _DummyErrorType {
type Success = ();
type Error = ();

fn from_success(_: ()) -> _DummyErrorType {
_DummyErrorType
}

fn from_error(_: ()) -> _DummyErrorType {
_DummyErrorType
}

fn translate<T>(self) -> T
where T: Carrier<Success=(), Error=()>
{
T::from_success(())
}
}
130 changes: 81 additions & 49 deletions src/librustc/hir/lowering.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -964,7 +964,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)
};
Expand Down Expand Up @@ -1157,15 +1157,13 @@ impl<'a> LoweringContext<'a> {
ast_expr: &Expr,
path: &[&str],
fields: &[(&str, &P<Expr>)]) -> P<hir::Expr> {
let strs = this.std_path(&iter::once(&"ops")
.chain(path)
.map(|s| *s)
.collect::<Vec<_>>());

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::<Vec<_>>());

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);
Expand All @@ -1178,7 +1176,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![],
Expand Down Expand Up @@ -1461,11 +1459,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());
Expand All @@ -1492,11 +1486,8 @@ impl<'a> LoweringContext<'a> {

// `match ::std::iter::IntoIterator::into_iter(<head>) { ... }`
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])
Expand Down Expand Up @@ -1525,16 +1516,32 @@ impl<'a> LoweringContext<'a> {
// to:
//
// {
// match <expr> {
// match { Carrier::translate( { <expr> } ) } {
// Ok(val) => val,
// Err(err) => {
// return Err(From::from(err))
// }
// Err(err) => { return Carrier::from_error(From::from(err)); }
// }
// }

// expand <expr>
let sub_expr = self.lower_expr(sub_expr);
// { Carrier::translate( { <expr> } ) }
let discr = {
// expand <expr>
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 = {
Expand All @@ -1546,32 +1553,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);
}

Expand Down Expand Up @@ -1785,6 +1795,15 @@ impl<'a> LoweringContext<'a> {
(respan(sp, hir::StmtDecl(P(decl), self.next_id())), pat_id)
}

// Turns `<expr>` into `<expr>;`, note that this produces a StmtSemi, not a
// StmtExpr.
fn stmt_expr(&self, expr: P<hir::Expr>) -> hir::Stmt {
hir::Stmt {
span: expr.span,
node: hir::StmtSemi(expr, self.next_id()),
}
}

fn block_expr(&mut self, expr: P<hir::Expr>) -> P<hir::Block> {
self.block_all(expr.span, hir::HirVec::new(), Some(expr))
}
Expand All @@ -1801,26 +1820,22 @@ impl<'a> LoweringContext<'a> {
}

fn pat_ok(&mut self, span: Span, pat: P<hir::Pat>) -> P<hir::Pat> {
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<hir::Pat>) -> P<hir::Pat> {
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<hir::Pat>) -> P<hir::Pat> {
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<hir::Pat> {
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![])
}

Expand Down Expand Up @@ -1918,7 +1933,7 @@ impl<'a> LoweringContext<'a> {
}
}

fn std_path(&mut self, components: &[&str]) -> Vec<Name> {
fn std_path_components(&mut self, components: &[&str]) -> Vec<Name> {
let mut v = Vec::new();
if let Some(s) = self.crate_root {
v.push(token::intern(s));
Expand All @@ -1929,8 +1944,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)
}

Expand All @@ -1951,4 +1966,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<Attribute>)
-> P<hir::Expr> {
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)
}
}
8 changes: 4 additions & 4 deletions src/librustc_const_eval/eval.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ pub fn const_expr_to_pat<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
hir::ExprTup(ref exprs) =>
PatKind::Tuple(try!(exprs.iter()
.map(|expr| const_expr_to_pat(tcx, &expr, pat_id, span))
.collect()), None),
.collect::<Result<_, _>>()), None),

hir::ExprCall(ref callee, ref args) => {
let def = tcx.expect_def(callee.id);
Expand All @@ -295,7 +295,7 @@ pub fn const_expr_to_pat<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
let pats = try!(args.iter()
.map(|expr| const_expr_to_pat(tcx, &**expr,
pat_id, span))
.collect());
.collect::<Result<_, _>>());
PatKind::TupleStruct(path, pats, None)
}

Expand All @@ -311,15 +311,15 @@ pub fn const_expr_to_pat<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
is_shorthand: false,
},
}))
.collect());
.collect::<Result<_, _>>());
PatKind::Struct(path.clone(), field_pats, false)
}

hir::ExprVec(ref exprs) => {
let pats = try!(exprs.iter()
.map(|expr| const_expr_to_pat(tcx, &expr,
pat_id, span))
.collect());
.collect::<Result<_, _>>());
PatKind::Vec(pats, None, hir::HirVec::new())
}

Expand Down
Loading