From 057569e2c2864ce907d780fc022831064c61e984 Mon Sep 17 00:00:00 2001 From: Alex Zatelepin Date: Wed, 18 Sep 2019 18:08:56 +0300 Subject: [PATCH] fix spurious unreachable_code lints for try{} block ok-wrapping --- src/librustc/hir/lowering/expr.rs | 45 ++++++++++++++++++++++--------- src/librustc/hir/mod.rs | 2 +- src/librustc_typeck/check/expr.rs | 33 +++++++++++++++++------ 3 files changed, 58 insertions(+), 22 deletions(-) diff --git a/src/librustc/hir/lowering/expr.rs b/src/librustc/hir/lowering/expr.rs index 9dcecedd97ca..0d2c7c13a85b 100644 --- a/src/librustc/hir/lowering/expr.rs +++ b/src/librustc/hir/lowering/expr.rs @@ -394,17 +394,35 @@ impl LoweringContext<'_> { fn lower_expr_try_block(&mut self, body: &Block) -> hir::ExprKind { self.with_catch_scope(body.id, |this| { - let unstable_span = this.mark_span_with_reason( - DesugaringKind::TryBlock, - body.span, - this.allow_try_trait.clone(), - ); let mut block = this.lower_block(body, true).into_inner(); - let tail = block.expr.take().map_or_else( - || this.expr_unit(this.sess.source_map().end_point(unstable_span)), + + 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(), ); - block.expr = Some(this.wrap_in_try_constructor(sym::from_ok, tail, unstable_span)); + + let from_ok_span = this.mark_span_with_reason( + DesugaringKind::TryBlock, + tail_expr.span, + this.allow_try_trait.clone(), + ); + + let ok_wrapped_span = this.mark_span_with_reason( + DesugaringKind::TryBlock, + tail_expr.span, + None + ); + + block.expr = Some(this.wrap_in_try_constructor( + sym::from_ok, from_ok_span, tail_expr, ok_wrapped_span)); + hir::ExprKind::Block(P(block), None) }) } @@ -412,12 +430,13 @@ impl LoweringContext<'_> { fn wrap_in_try_constructor( &mut self, method: Symbol, - e: hir::Expr, - unstable_span: Span, + method_span: Span, + expr: hir::Expr, + overall_span: Span, ) -> P { let path = &[sym::ops, sym::Try, method]; - let from_err = P(self.expr_std_path(unstable_span, path, None, ThinVec::new())); - P(self.expr_call(e.span, from_err, hir_vec![e])) + let constructor = P(self.expr_std_path(method_span, path, None, ThinVec::new())); + P(self.expr_call(overall_span, constructor, hir_vec![expr])) } fn lower_arm(&mut self, arm: &Arm) -> hir::Arm { @@ -1244,7 +1263,7 @@ impl LoweringContext<'_> { self.expr_call_std_path(try_span, from_path, hir_vec![err_expr]) }; let from_err_expr = - self.wrap_in_try_constructor(sym::from_error, from_expr, unstable_span); + self.wrap_in_try_constructor(sym::from_error, unstable_span, from_expr, try_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 { diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 9ae661f0952a..9b4d88a5a096 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -861,7 +861,7 @@ pub struct Block { pub span: Span, /// If true, then there may exist `break 'a` values that aim to /// break out of this block early. - /// Used by `'label: {}` blocks and by `catch` statements. + /// Used by `'label: {}` blocks and by `try {}` blocks. pub targeted_by_break: bool, } diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs index 04c8536de8df..a93ea0cc0046 100644 --- a/src/librustc_typeck/check/expr.rs +++ b/src/librustc_typeck/check/expr.rs @@ -18,6 +18,7 @@ use crate::util::nodemap::FxHashMap; use crate::astconv::AstConv as _; use errors::{Applicability, DiagnosticBuilder, pluralise}; +use syntax_pos::hygiene::DesugaringKind; use syntax::ast; use syntax::symbol::{Symbol, kw, sym}; use syntax::source_map::Span; @@ -150,8 +151,22 @@ 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), + }; + // Warn for expressions after diverging siblings. - self.warn_if_unreachable(expr.hir_id, expr.span, "expression"); + if !is_try_block_generated_expr { + self.warn_if_unreachable(expr.hir_id, expr.span, "expression"); + } // Hide the outer diverging and has_errors flags. let old_diverges = self.diverges.get(); @@ -162,13 +177,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let ty = self.check_expr_kind(expr, expected, needs); // Warn for non-block expressions with diverging children. - match expr.kind { - 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"), + if !is_try_block_ok_wrapped_expr { + match expr.kind { + 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"), + } } // Any expression that produces a value of type `!` must have diverged