Skip to content

Commit

Permalink
Rollup merge of rust-lang#64581 - ztlpn:fix-ok-wrapping-unreachable-c…
Browse files Browse the repository at this point in the history
…ode, r=Centril

Fix unreachable_code warnings for try{} block ok-wrapped expressions

Fixes rust-lang#54165 and fixes rust-lang#63324.
  • Loading branch information
Centril authored Oct 1, 2019
2 parents 702b45e + 58eacaa commit e463a22
Show file tree
Hide file tree
Showing 6 changed files with 161 additions and 13 deletions.
37 changes: 27 additions & 10 deletions src/librustc/hir/lowering/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,32 +392,49 @@ impl LoweringContext<'_> {
)
}

/// Desugar `try { <stmts>; <expr> }` into `{ <stmts>; ::std::ops::Try::from_ok(<expr>) }`,
/// `try { <stmts>; }` into `{ <stmts>; ::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 unstable_span = this.mark_span_with_reason(
let mut block = this.lower_block(body, true).into_inner();

let try_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)),

// 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<hir::Expr>| x.into_inner(),
);
block.expr = Some(this.wrap_in_try_constructor(sym::from_ok, tail, unstable_span));

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, try_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 @@ -1244,7 +1261,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
19 changes: 18 additions & 1 deletion 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 @@ -150,8 +151,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
debug!(">> type-checking: expr={:?} expected={:?}",
expr, expected);

// 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.
self.warn_if_unreachable(expr.hir_id, expr.span, "expression");
if !is_try_block_generated_unit_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 @@ -164,6 +177,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// Warn for non-block expressions with diverging children.
match expr.kind {
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, _) =>
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ pub enum Diverges {
WarnedAlways
}

// Convenience impls for combinig `Diverges`.
// Convenience impls for combining `Diverges`.

impl ops::BitAnd for Diverges {
type Output = Self;
Expand Down
76 changes: 76 additions & 0 deletions src/test/ui/try-block/try-block-unreachable-code-lint.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// Test unreachable_code lint for `try {}` block ok-wrapping. See issues #54165, #63324.

// compile-flags: --edition 2018
// check-pass
#![feature(try_blocks)]
#![warn(unreachable_code)]

fn err() -> Result<u32, ()> {
Err(())
}

// In the following cases unreachable code is autogenerated and should not be reported.

fn test_ok_wrapped_divergent_expr_1() {
let res: Result<u32, ()> = try {
loop {
err()?;
}
};
println!("res: {:?}", res);
}

fn test_ok_wrapped_divergent_expr_2() {
let _: Result<u32, ()> = try {
return
};
}

fn test_autogenerated_unit_after_divergent_expr() {
let _: Result<(), ()> = try {
return;
};
}

// In the following cases unreachable code should be reported.

fn test_try_block_after_divergent_stmt() {
let _: Result<u32, ()> = {
return;

try {
loop {
err()?;
}
}
// ~^^^^^ WARNING unreachable expression
};
}

fn test_wrapped_divergent_expr() {
let _: Result<u32, ()> = {
Err(return)
// ~^ WARNING unreachable call
};
}

fn test_expr_after_divergent_stmt_in_try_block() {
let res: Result<u32, ()> = try {
loop {
err()?;
}

42
// ~^ WARNING unreachable expression
};
println!("res: {:?}", res);
}

fn main() {
test_ok_wrapped_divergent_expr_1();
test_ok_wrapped_divergent_expr_2();
test_autogenerated_unit_after_divergent_expr();
test_try_block_after_divergent_stmt();
test_wrapped_divergent_expr();
test_expr_after_divergent_stmt_in_try_block();
}
38 changes: 38 additions & 0 deletions src/test/ui/try-block/try-block-unreachable-code-lint.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
warning: unreachable expression
--> $DIR/try-block-unreachable-code-lint.rs:41:9
|
LL | return;
| ------ any code following this expression is unreachable
LL |
LL | / try {
LL | | loop {
LL | | err()?;
LL | | }
LL | | }
| |_________^ unreachable expression
|
note: lint level defined here
--> $DIR/try-block-unreachable-code-lint.rs:6:9
|
LL | #![warn(unreachable_code)]
| ^^^^^^^^^^^^^^^^

warning: unreachable call
--> $DIR/try-block-unreachable-code-lint.rs:52:9
|
LL | Err(return)
| ^^^ ------ any code following this expression is unreachable
| |
| unreachable call

warning: unreachable expression
--> $DIR/try-block-unreachable-code-lint.rs:63:9
|
LL | / loop {
LL | | err()?;
LL | | }
| |_________- any code following this expression is unreachable
LL |
LL | 42
| ^^ unreachable expression

0 comments on commit e463a22

Please sign in to comment.