Skip to content

Commit

Permalink
fix spurious unreachable_code lints for try{} block ok-wrapping
Browse files Browse the repository at this point in the history
  • Loading branch information
ztlpn committed Sep 22, 2019
1 parent 1d704e7 commit 3f85ff4
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 21 deletions.
43 changes: 31 additions & 12 deletions src/librustc/hir/lowering/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down
33 changes: 25 additions & 8 deletions src/librustc_typeck/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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
Expand Down

0 comments on commit 3f85ff4

Please sign in to comment.