From 65251ea73ab5d19e70a6ff4b64fb218a03504b6b Mon Sep 17 00:00:00 2001 From: Alex Zatelepin Date: Thu, 19 Sep 2019 01:20:18 +0300 Subject: [PATCH] address review comments --- src/librustc/hir/lowering/expr.rs | 28 ++++++------- src/librustc_typeck/check/expr.rs | 40 +++++++++---------- .../ui/try-block/try-block-bad-type.stderr | 8 ++-- 3 files changed, 37 insertions(+), 39 deletions(-) diff --git a/src/librustc/hir/lowering/expr.rs b/src/librustc/hir/lowering/expr.rs index 86f706ebe555a..3ba11644f4959 100644 --- a/src/librustc/hir/lowering/expr.rs +++ b/src/librustc/hir/lowering/expr.rs @@ -392,36 +392,34 @@ impl LoweringContext<'_> { ) } + /// Desugar `try { ; }` into `{ ; ::std::ops::Try::from_ok() }`, + /// `try { ; }` into `{ ; ::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| 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| 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) }) diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs index 70f23a834d266..e788e7e58fbaa 100644 --- a/src/librustc_typeck/check/expr.rs +++ b/src/librustc_typeck/check/expr.rs @@ -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"); } @@ -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 diff --git a/src/test/ui/try-block/try-block-bad-type.stderr b/src/test/ui/try-block/try-block-bad-type.stderr index 0e993cb4e5885..e1c2c6b675e9b 100644 --- a/src/test/ui/try-block/try-block-bad-type.stderr +++ b/src/test/ui/try-block/try-block-bad-type.stderr @@ -32,18 +32,18 @@ LL | let res: Result = 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`