Skip to content

Commit 057569e

Browse files
committed
fix spurious unreachable_code lints for try{} block ok-wrapping
1 parent 4808106 commit 057569e

File tree

3 files changed

+57
-21
lines changed

3 files changed

+57
-21
lines changed

src/librustc/hir/lowering/expr.rs

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -394,30 +394,49 @@ impl LoweringContext<'_> {
394394

395395
fn lower_expr_try_block(&mut self, body: &Block) -> hir::ExprKind {
396396
self.with_catch_scope(body.id, |this| {
397-
let unstable_span = this.mark_span_with_reason(
397+
let mut block = this.lower_block(body, true).into_inner();
398+
399+
let tail_expr = block.expr.take().map_or_else(
400+
|| {
401+
let unit_span = this.mark_span_with_reason(
402+
DesugaringKind::TryBlock,
403+
this.sess.source_map().end_point(body.span),
404+
None
405+
);
406+
this.expr_unit(unit_span)
407+
},
408+
|x: P<hir::Expr>| x.into_inner(),
409+
);
410+
411+
let from_ok_span = this.mark_span_with_reason(
398412
DesugaringKind::TryBlock,
399-
body.span,
413+
tail_expr.span,
400414
this.allow_try_trait.clone(),
401415
);
402-
let mut block = this.lower_block(body, true).into_inner();
403-
let tail = block.expr.take().map_or_else(
404-
|| this.expr_unit(this.sess.source_map().end_point(unstable_span)),
405-
|x: P<hir::Expr>| x.into_inner(),
416+
417+
let ok_wrapped_span = this.mark_span_with_reason(
418+
DesugaringKind::TryBlock,
419+
tail_expr.span,
420+
None
406421
);
407-
block.expr = Some(this.wrap_in_try_constructor(sym::from_ok, tail, unstable_span));
422+
423+
block.expr = Some(this.wrap_in_try_constructor(
424+
sym::from_ok, from_ok_span, tail_expr, ok_wrapped_span));
425+
408426
hir::ExprKind::Block(P(block), None)
409427
})
410428
}
411429

412430
fn wrap_in_try_constructor(
413431
&mut self,
414432
method: Symbol,
415-
e: hir::Expr,
416-
unstable_span: Span,
433+
method_span: Span,
434+
expr: hir::Expr,
435+
overall_span: Span,
417436
) -> P<hir::Expr> {
418437
let path = &[sym::ops, sym::Try, method];
419-
let from_err = P(self.expr_std_path(unstable_span, path, None, ThinVec::new()));
420-
P(self.expr_call(e.span, from_err, hir_vec![e]))
438+
let constructor = P(self.expr_std_path(method_span, path, None, ThinVec::new()));
439+
P(self.expr_call(overall_span, constructor, hir_vec![expr]))
421440
}
422441

423442
fn lower_arm(&mut self, arm: &Arm) -> hir::Arm {
@@ -1244,7 +1263,7 @@ impl LoweringContext<'_> {
12441263
self.expr_call_std_path(try_span, from_path, hir_vec![err_expr])
12451264
};
12461265
let from_err_expr =
1247-
self.wrap_in_try_constructor(sym::from_error, from_expr, unstable_span);
1266+
self.wrap_in_try_constructor(sym::from_error, unstable_span, from_expr, try_span);
12481267
let thin_attrs = ThinVec::from(attrs);
12491268
let catch_scope = self.catch_scopes.last().map(|x| *x);
12501269
let ret_expr = if let Some(catch_node) = catch_scope {

src/librustc/hir/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -861,7 +861,7 @@ pub struct Block {
861861
pub span: Span,
862862
/// If true, then there may exist `break 'a` values that aim to
863863
/// break out of this block early.
864-
/// Used by `'label: {}` blocks and by `catch` statements.
864+
/// Used by `'label: {}` blocks and by `try {}` blocks.
865865
pub targeted_by_break: bool,
866866
}
867867

src/librustc_typeck/check/expr.rs

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use crate::util::nodemap::FxHashMap;
1818
use crate::astconv::AstConv as _;
1919

2020
use errors::{Applicability, DiagnosticBuilder, pluralise};
21+
use syntax_pos::hygiene::DesugaringKind;
2122
use syntax::ast;
2223
use syntax::symbol::{Symbol, kw, sym};
2324
use syntax::source_map::Span;
@@ -150,8 +151,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
150151
debug!(">> type-checking: expr={:?} expected={:?}",
151152
expr, expected);
152153

154+
// If when desugaring the try block we ok-wrapped an expression that diverges
155+
// (e.g. `try { return }`) then technically the ok-wrapping expression is unreachable.
156+
// But since it is autogenerated code the resulting warning is confusing for the user
157+
// so we want avoid generating it.
158+
// Ditto for the autogenerated `Try::from_ok(())` at the end of e.g. `try { return; }`.
159+
let (is_try_block_ok_wrapped_expr, is_try_block_generated_expr) = match expr.node {
160+
ExprKind::Call(_, ref args) if expr.span.is_desugaring(DesugaringKind::TryBlock) => {
161+
(true, args.len() == 1 && args[0].span.is_desugaring(DesugaringKind::TryBlock))
162+
}
163+
_ => (false, false),
164+
};
165+
153166
// Warn for expressions after diverging siblings.
154-
self.warn_if_unreachable(expr.hir_id, expr.span, "expression");
167+
if !is_try_block_generated_expr {
168+
self.warn_if_unreachable(expr.hir_id, expr.span, "expression");
169+
}
155170

156171
// Hide the outer diverging and has_errors flags.
157172
let old_diverges = self.diverges.get();
@@ -162,13 +177,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
162177
let ty = self.check_expr_kind(expr, expected, needs);
163178

164179
// Warn for non-block expressions with diverging children.
165-
match expr.kind {
166-
ExprKind::Block(..) | ExprKind::Loop(..) | ExprKind::Match(..) => {},
167-
ExprKind::Call(ref callee, _) =>
168-
self.warn_if_unreachable(expr.hir_id, callee.span, "call"),
169-
ExprKind::MethodCall(_, ref span, _) =>
170-
self.warn_if_unreachable(expr.hir_id, *span, "call"),
171-
_ => self.warn_if_unreachable(expr.hir_id, expr.span, "expression"),
180+
if !is_try_block_ok_wrapped_expr {
181+
match expr.kind {
182+
ExprKind::Block(..) | ExprKind::Loop(..) | ExprKind::Match(..) => {},
183+
ExprKind::Call(ref callee, _) =>
184+
self.warn_if_unreachable(expr.hir_id, callee.span, "call"),
185+
ExprKind::MethodCall(_, ref span, _) =>
186+
self.warn_if_unreachable(expr.hir_id, *span, "call"),
187+
_ => self.warn_if_unreachable(expr.hir_id, expr.span, "expression"),
188+
}
172189
}
173190

174191
// Any expression that produces a value of type `!` must have diverged

0 commit comments

Comments
 (0)