From d14a323e74d0770960553fe79d0bcd578c393cf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 24 Jan 2020 10:35:13 -0800 Subject: [PATCH] Use more accurate return path spans No longer suggest `Box::new(if foo { Type1 } else { Type2 })`, instead suggesting `if foo { Box::new(Type1) } else { Box::new(Type2) }`. --- .../traits/error_reporting/suggestions.rs | 53 +++++++++++++---- src/test/ui/error-codes/E0746.fixed | 4 +- src/test/ui/error-codes/E0746.rs | 4 +- src/test/ui/error-codes/E0746.stderr | 2 +- .../dyn-trait-return-should-be-impl-trait.rs | 11 +++- ...n-trait-return-should-be-impl-trait.stderr | 57 +++++++++++-------- .../feature-gate-never_type_fallback.stderr | 2 +- 7 files changed, 91 insertions(+), 42 deletions(-) diff --git a/src/librustc/traits/error_reporting/suggestions.rs b/src/librustc/traits/error_reporting/suggestions.rs index 12c8de17d10..84f12603855 100644 --- a/src/librustc/traits/error_reporting/suggestions.rs +++ b/src/librustc/traits/error_reporting/suggestions.rs @@ -600,12 +600,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { // Visit to make sure there's a single `return` type to suggest `impl Trait`, // otherwise suggest using `Box` or an enum. - let mut visitor = ReturnsVisitor(vec![]); + let mut visitor = ReturnsVisitor::new(); visitor.visit_body(&body); let tables = self.in_progress_tables.map(|t| t.borrow()).unwrap(); - let mut ret_types = visitor.0.iter().filter_map(|expr| tables.node_type_opt(expr.hir_id)); + let mut ret_types = + visitor.returns.iter().filter_map(|expr| tables.node_type_opt(expr.hir_id)); let (last_ty, all_returns_have_same_type) = ret_types.clone().fold( (None, true), |(last_ty, mut same): (std::option::Option>, bool), ty| { @@ -677,7 +678,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { // Suggest `-> Box` and `Box::new(returned_value)`. // Get all the return values and collect their span and suggestion. let mut suggestions = visitor - .0 + .returns .iter() .map(|expr| { ( @@ -737,10 +738,10 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { { let body = hir.body(*body_id); // Point at all the `return`s in the function as they have failed trait bounds. - let mut visitor = ReturnsVisitor(vec![]); + let mut visitor = ReturnsVisitor::new(); visitor.visit_body(&body); let tables = self.in_progress_tables.map(|t| t.borrow()).unwrap(); - for expr in &visitor.0 { + for expr in &visitor.returns { if let Some(returned_ty) = tables.node_type_opt(expr.hir_id) { let ty = self.resolve_vars_if_possible(&returned_ty); err.span_label(expr.span, &format!("this returned value is of type `{}`", ty)); @@ -1691,7 +1692,16 @@ pub fn suggest_constraining_type_param( /// Collect all the returned expressions within the input expression. /// Used to point at the return spans when we want to suggest some change to them. -struct ReturnsVisitor<'v>(Vec<&'v hir::Expr<'v>>); +struct ReturnsVisitor<'v> { + returns: Vec<&'v hir::Expr<'v>>, + in_block_tail: bool, +} + +impl ReturnsVisitor<'_> { + fn new() -> Self { + ReturnsVisitor { returns: vec![], in_block_tail: false } + } +} impl<'v> Visitor<'v> for ReturnsVisitor<'v> { type Map = rustc::hir::map::Map<'v>; @@ -1701,20 +1711,41 @@ impl<'v> Visitor<'v> for ReturnsVisitor<'v> { } fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) { - if let hir::ExprKind::Ret(Some(ex)) = ex.kind { - self.0.push(ex); + match ex.kind { + hir::ExprKind::Ret(Some(ex)) => { + self.returns.push(ex); + } + hir::ExprKind::Block(block, _) if self.in_block_tail => { + self.in_block_tail = false; + for stmt in block.stmts { + hir::intravisit::walk_stmt(self, stmt); + } + self.in_block_tail = true; + if let Some(expr) = block.expr { + self.visit_expr(expr); + } + } + hir::ExprKind::Match(_, arms, _) if self.in_block_tail => { + for arm in arms { + self.visit_expr(arm.body); + } + } + // We need to walk to find `return`s in the entire body. + _ if !self.in_block_tail => hir::intravisit::walk_expr(self, ex), + _ => self.returns.push(ex), } - hir::intravisit::walk_expr(self, ex); } fn visit_body(&mut self, body: &'v hir::Body<'v>) { + let prev = self.in_block_tail; if body.generator_kind().is_none() { if let hir::ExprKind::Block(block, None) = body.value.kind { - if let Some(expr) = block.expr { - self.0.push(expr); + if block.expr.is_some() { + self.in_block_tail = true; } } } hir::intravisit::walk_body(self, body); + self.in_block_tail = prev; } } diff --git a/src/test/ui/error-codes/E0746.fixed b/src/test/ui/error-codes/E0746.fixed index ca8319aa020..70c7f791a2b 100644 --- a/src/test/ui/error-codes/E0746.fixed +++ b/src/test/ui/error-codes/E0746.fixed @@ -10,9 +10,9 @@ fn foo() -> impl Trait { Struct } fn bar() -> impl Trait { //~ ERROR E0746 if true { - return 0; + return 0u32; } - 42 + 42u32 } fn main() {} diff --git a/src/test/ui/error-codes/E0746.rs b/src/test/ui/error-codes/E0746.rs index bf5ba8fff56..fbf18246e16 100644 --- a/src/test/ui/error-codes/E0746.rs +++ b/src/test/ui/error-codes/E0746.rs @@ -10,9 +10,9 @@ fn foo() -> dyn Trait { Struct } fn bar() -> dyn Trait { //~ ERROR E0746 if true { - return 0; + return 0u32; } - 42 + 42u32 } fn main() {} diff --git a/src/test/ui/error-codes/E0746.stderr b/src/test/ui/error-codes/E0746.stderr index e7a8fd304ca..05c61f1149f 100644 --- a/src/test/ui/error-codes/E0746.stderr +++ b/src/test/ui/error-codes/E0746.stderr @@ -17,7 +17,7 @@ LL | fn bar() -> dyn Trait { | ^^^^^^^^^ doesn't have a size known at compile-time | = note: for information on `impl Trait`, see -help: return `impl Trait` instead, as all return paths are of type `{integer}`, which implements `Trait` +help: return `impl Trait` instead, as all return paths are of type `u32`, which implements `Trait` | LL | fn bar() -> impl Trait { | ^^^^^^^^^^ diff --git a/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.rs b/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.rs index 93414379fd4..dfcc22aee34 100644 --- a/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.rs +++ b/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.rs @@ -22,6 +22,13 @@ fn bal() -> dyn Trait { //~ ERROR E0746 } 42 } +fn bax() -> dyn Trait { //~ ERROR E0746 + if true { + Struct + } else { + 42 + } +} fn bam() -> Box { if true { return Struct; //~ ERROR mismatched types @@ -52,9 +59,9 @@ fn baw() -> Box { // Suggest using `impl Trait` fn bat() -> dyn Trait { //~ ERROR E0746 if true { - return 0; + return 0u32; } - 42 + 42u32 } fn bay() -> dyn Trait { //~ ERROR E0746 if true { diff --git a/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr b/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr index 0df6e8f8dc7..fbad7ec124c 100644 --- a/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr +++ b/src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr @@ -95,8 +95,27 @@ LL | } LL | Box::new(42) | +error[E0746]: return type cannot have an unboxed trait object + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:25:13 + | +LL | fn bax() -> dyn Trait { + | ^^^^^^^^^ doesn't have a size known at compile-time + | + = note: for information on trait objects, see + = note: if all the returned values were of the same type you could use `impl Trait` as the return type + = note: for information on `impl Trait`, see + = note: you can create a new `enum` with a variant for each returned type +help: return a boxed trait object instead + | +LL | fn bax() -> Box { +LL | if true { +LL | Box::new(Struct) +LL | } else { +LL | Box::new(42) + | + error[E0308]: mismatched types - --> $DIR/dyn-trait-return-should-be-impl-trait.rs:27:16 + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:34:16 | LL | fn bam() -> Box { | -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type @@ -112,7 +131,7 @@ LL | return Struct; = note: for more on the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html, and https://doc.rust-lang.org/std/boxed/index.html error[E0308]: mismatched types - --> $DIR/dyn-trait-return-should-be-impl-trait.rs:29:5 + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:36:5 | LL | fn bam() -> Box { | -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type @@ -128,7 +147,7 @@ LL | 42 = note: for more on the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html, and https://doc.rust-lang.org/std/boxed/index.html error[E0308]: mismatched types - --> $DIR/dyn-trait-return-should-be-impl-trait.rs:33:16 + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:40:16 | LL | fn baq() -> Box { | -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type @@ -144,7 +163,7 @@ LL | return 0; = note: for more on the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html, and https://doc.rust-lang.org/std/boxed/index.html error[E0308]: mismatched types - --> $DIR/dyn-trait-return-should-be-impl-trait.rs:35:5 + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:42:5 | LL | fn baq() -> Box { | -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type @@ -160,7 +179,7 @@ LL | 42 = note: for more on the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html, and https://doc.rust-lang.org/std/boxed/index.html error[E0308]: mismatched types - --> $DIR/dyn-trait-return-should-be-impl-trait.rs:39:9 + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:46:9 | LL | fn baz() -> Box { | -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type @@ -176,7 +195,7 @@ LL | Struct = note: for more on the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html, and https://doc.rust-lang.org/std/boxed/index.html error[E0308]: mismatched types - --> $DIR/dyn-trait-return-should-be-impl-trait.rs:41:9 + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:48:9 | LL | fn baz() -> Box { | -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type @@ -192,7 +211,7 @@ LL | 42 = note: for more on the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html, and https://doc.rust-lang.org/std/boxed/index.html error[E0308]: mismatched types - --> $DIR/dyn-trait-return-should-be-impl-trait.rs:46:9 + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:53:9 | LL | fn baw() -> Box { | -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type @@ -208,7 +227,7 @@ LL | 0 = note: for more on the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html, and https://doc.rust-lang.org/std/boxed/index.html error[E0308]: mismatched types - --> $DIR/dyn-trait-return-should-be-impl-trait.rs:48:9 + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:55:9 | LL | fn baw() -> Box { | -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type @@ -224,38 +243,30 @@ LL | 42 = note: for more on the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html, and https://doc.rust-lang.org/std/boxed/index.html error[E0746]: return type cannot have an unboxed trait object - --> $DIR/dyn-trait-return-should-be-impl-trait.rs:53:13 + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:60:13 | LL | fn bat() -> dyn Trait { | ^^^^^^^^^ doesn't have a size known at compile-time | = note: for information on `impl Trait`, see -help: return `impl Trait` instead, as all return paths are of type `{integer}`, which implements `Trait` +help: return `impl Trait` instead, as all return paths are of type `u32`, which implements `Trait` | LL | fn bat() -> impl Trait { | ^^^^^^^^^^ error[E0746]: return type cannot have an unboxed trait object - --> $DIR/dyn-trait-return-should-be-impl-trait.rs:59:13 + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:66:13 | LL | fn bay() -> dyn Trait { | ^^^^^^^^^ doesn't have a size known at compile-time | - = note: for information on trait objects, see - = note: if all the returned values were of the same type you could use `impl Trait` as the return type = note: for information on `impl Trait`, see - = note: you can create a new `enum` with a variant for each returned type -help: return a boxed trait object instead - | -LL | fn bay() -> Box { -LL | Box::new(if true { -LL | 0u32 -LL | } else { -LL | 42u32 -LL | }) +help: return `impl Trait` instead, as all return paths are of type `u32`, which implements `Trait` | +LL | fn bay() -> impl Trait { + | ^^^^^^^^^^ -error: aborting due to 18 previous errors +error: aborting due to 19 previous errors Some errors have detailed explanations: E0277, E0308, E0746. For more information about an error, try `rustc --explain E0277`. diff --git a/src/test/ui/never_type/feature-gate-never_type_fallback.stderr b/src/test/ui/never_type/feature-gate-never_type_fallback.stderr index 77288f1bada..08e16f46936 100644 --- a/src/test/ui/never_type/feature-gate-never_type_fallback.stderr +++ b/src/test/ui/never_type/feature-gate-never_type_fallback.stderr @@ -5,7 +5,7 @@ LL | fn should_ret_unit() -> impl T { | ^^^^^^ the trait `T` is not implemented for `()` LL | LL | panic!() - | -------- this returned value is of type `()` + | -------- this returned value is of type `!` | = note: the return type of a function must have a statically known size = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)