fix spurious unreachable_code lints for try{} block ok-wrapping

rust-lang · Centril · Sep 16, 2019 · Sep 18, 2019 · Sep 18, 2019 · Sep 18, 2019
commit 3f85ff456230b413e63735e3fc7ca7b26e9e6cf5
diff --git a/src/librustc/hir/lowering/expr.rs b/src/librustc/hir/lowering/expr.rs
@@ -394,30 +394,49 @@ 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(
+            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(
                 DesugaringKind::TryBlock,
-                body.span,
+                tail_expr.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)),
-                |x: P<hir::Expr>| x.into_inner(),
+
+            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, tail, unstable_span));
+
+            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)
         })
     }
 
     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<hir::Expr> {
         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 {
@@ -1258,7 +1277,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
@@ -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
@@ -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;
@@ -147,8 +148,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();
@@ -159,13 +174,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.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"),
+        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"),
+            }
         }
 
         // Any expression that produces a value of type `!` must have diverged