Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
ztlpn committed Sep 22, 2019
1 parent c60c0cb commit 65251ea
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 39 deletions.
28 changes: 13 additions & 15 deletions src/librustc/hir/lowering/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,36 +392,34 @@ impl LoweringContext<'_> {
)
}

/// Desugar `try { <stmts>; <expr> }` into `{ <stmts>; ::std::ops::Try::from_ok(<expr>) }`,
/// `try { <stmts>; }` into `{ <stmts>; ::std::ops::Try::from_ok(()) }`
/// and save the block id to use it as a break target for desugaring of the `?` operator.
fn lower_expr_try_block(&mut self, body: &Block) -> hir::ExprKind {
self.with_catch_scope(body.id, |this| {
let mut block = this.lower_block(body, true).into_inner();

let tail_expr = block.expr.take().map_or_else(
|| {
let unit_span = this.mark_span_with_reason(
DesugaringKind::TryBlock,
this.sess.source_map().end_point(body.span),
None
);
this.expr_unit(unit_span)
},
|x: P<hir::Expr>| x.into_inner(),
);

let from_ok_span = this.mark_span_with_reason(
let try_span = this.mark_span_with_reason(
DesugaringKind::TryBlock,
tail_expr.span,
body.span,
this.allow_try_trait.clone(),
);

// Final expression of the block (if present) or `()` with span at the end of block
let tail_expr = block.expr.take().map_or_else(
|| this.expr_unit(this.sess.source_map().end_point(try_span)),
|x: P<hir::Expr>| x.into_inner(),
);

let ok_wrapped_span = this.mark_span_with_reason(
DesugaringKind::TryBlock,
tail_expr.span,
None
);

// `::std::ops::Try::from_ok($tail_expr)`
block.expr = Some(this.wrap_in_try_constructor(
sym::from_ok, from_ok_span, tail_expr, ok_wrapped_span));
sym::from_ok, try_span, tail_expr, ok_wrapped_span));

hir::ExprKind::Block(P(block), None)
})
Expand Down
40 changes: 20 additions & 20 deletions src/librustc_typeck/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,20 +148,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
debug!(">> type-checking: expr={:?} expected={:?}",
expr, expected);

// If when desugaring the try block we ok-wrapped an expression that diverges
// (e.g. `try { return }`) then technically the ok-wrapping expression is unreachable.
// But since it is autogenerated code the resulting warning is confusing for the user
// so we want avoid generating it.
// Ditto for the autogenerated `Try::from_ok(())` at the end of e.g. `try { return; }`.
let (is_try_block_ok_wrapped_expr, is_try_block_generated_expr) = match expr.node {
ExprKind::Call(_, ref args) if expr.span.is_desugaring(DesugaringKind::TryBlock) => {
(true, args.len() == 1 && args[0].span.is_desugaring(DesugaringKind::TryBlock))
}
_ => (false, false),
// True if `expr` is a `Try::from_ok(())` that is a result of desugaring a try block
// without the final expr (e.g. `try { return; }`). We don't want to generate an
// unreachable_code lint for it since warnings for autogenerated code are confusing.
let is_try_block_generated_unit_expr = match expr.node {
ExprKind::Call(_, ref args) if expr.span.is_desugaring(DesugaringKind::TryBlock) =>
args.len() == 1 && args[0].span.is_desugaring(DesugaringKind::TryBlock),

_ => false,
};

// Warn for expressions after diverging siblings.
if !is_try_block_generated_expr {
if !is_try_block_generated_unit_expr {
self.warn_if_unreachable(expr.hir_id, expr.span, "expression");
}

Expand All @@ -174,15 +172,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let ty = self.check_expr_kind(expr, expected, needs);

// Warn for non-block expressions with diverging children.
if !is_try_block_ok_wrapped_expr {
match expr.node {
ExprKind::Block(..) | ExprKind::Loop(..) | ExprKind::Match(..) => {},
ExprKind::Call(ref callee, _) =>
self.warn_if_unreachable(expr.hir_id, callee.span, "call"),
ExprKind::MethodCall(_, ref span, _) =>
self.warn_if_unreachable(expr.hir_id, *span, "call"),
_ => self.warn_if_unreachable(expr.hir_id, expr.span, "expression"),
}
match expr.node {
ExprKind::Block(..) | ExprKind::Loop(..) | ExprKind::Match(..) => {},
// If `expr` is a result of desugaring the try block and is an ok-wrapped
// diverging expression (e.g. it arose from desugaring of `try { return }`),
// we skip issuing a warning because it is autogenerated code.
ExprKind::Call(..) if expr.span.is_desugaring(DesugaringKind::TryBlock) => {},
ExprKind::Call(ref callee, _) =>
self.warn_if_unreachable(expr.hir_id, callee.span, "call"),
ExprKind::MethodCall(_, ref span, _) =>
self.warn_if_unreachable(expr.hir_id, *span, "call"),
_ => self.warn_if_unreachable(expr.hir_id, expr.span, "expression"),
}

// Any expression that produces a value of type `!` must have diverged
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/try-block/try-block-bad-type.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,18 @@ LL | let res: Result<i32, i32> = try { };
found type `()`

error[E0277]: the trait bound `(): std::ops::Try` is not satisfied
--> $DIR/try-block-bad-type.rs:17:25
--> $DIR/try-block-bad-type.rs:17:23
|
LL | let res: () = try { };
| ^ the trait `std::ops::Try` is not implemented for `()`
| ^^^ the trait `std::ops::Try` is not implemented for `()`
|
= note: required by `std::ops::Try::from_ok`

error[E0277]: the trait bound `i32: std::ops::Try` is not satisfied
--> $DIR/try-block-bad-type.rs:19:26
--> $DIR/try-block-bad-type.rs:19:24
|
LL | let res: i32 = try { 5 };
| ^ the trait `std::ops::Try` is not implemented for `i32`
| ^^^^^ the trait `std::ops::Try` is not implemented for `i32`
|
= note: required by `std::ops::Try::from_ok`

Expand Down

0 comments on commit 65251ea

Please sign in to comment.