From 55dce720b24fde5d8489498fb5f7abd8c52c28f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 23 Jan 2020 15:21:15 -0800 Subject: [PATCH 1/6] Account for `ty::Error` when suggesting `impl Trait` or `Box` --- .../traits/error_reporting/suggestions.rs | 14 +- .../dyn-trait-return-should-be-impl-trait.rs | 33 ++++ ...n-trait-return-should-be-impl-trait.stderr | 152 +++++++++++++++++- 3 files changed, 191 insertions(+), 8 deletions(-) diff --git a/src/librustc/traits/error_reporting/suggestions.rs b/src/librustc/traits/error_reporting/suggestions.rs index 4559007ea426a..12c8de17d10f9 100644 --- a/src/librustc/traits/error_reporting/suggestions.rs +++ b/src/librustc/traits/error_reporting/suggestions.rs @@ -606,11 +606,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { 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 (last_ty, all_returns_have_same_type) = - ret_types.clone().fold((None, true), |(last_ty, mut same), returned_ty| { - same &= last_ty.map_or(true, |ty| ty == returned_ty); - (Some(returned_ty), same) - }); + let (last_ty, all_returns_have_same_type) = ret_types.clone().fold( + (None, true), + |(last_ty, mut same): (std::option::Option>, bool), ty| { + same &= last_ty.map_or(true, |last_ty| last_ty == ty) && ty.kind != ty::Error; + (Some(ty), same) + }, + ); let all_returns_conform_to_trait = if let Some(ty_ret_ty) = tables.node_type_opt(ret_ty.hir_id) { match ty_ret_ty.kind { @@ -625,7 +627,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { }) }) } - _ => true, + _ => false, } } else { true 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 b70a51dc82511..93414379fd41a 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,32 @@ fn bal() -> dyn Trait { //~ ERROR E0746 } 42 } +fn bam() -> Box { + if true { + return Struct; //~ ERROR mismatched types + } + 42 //~ ERROR mismatched types +} +fn baq() -> Box { + if true { + return 0; //~ ERROR mismatched types + } + 42 //~ ERROR mismatched types +} +fn baz() -> Box { + if true { + Struct //~ ERROR mismatched types + } else { + 42 //~ ERROR mismatched types + } +} +fn baw() -> Box { + if true { + 0 //~ ERROR mismatched types + } else { + 42 //~ ERROR mismatched types + } +} // Suggest using `impl Trait` fn bat() -> dyn Trait { //~ ERROR E0746 @@ -30,5 +56,12 @@ fn bat() -> dyn Trait { //~ ERROR E0746 } 42 } +fn bay() -> dyn Trait { //~ ERROR E0746 + if true { + 0u32 + } else { + 42u32 + } +} fn main() {} 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 977a7ef0e0244..0df6e8f8dc77c 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,136 @@ LL | } LL | Box::new(42) | +error[E0308]: mismatched types + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:27:16 + | +LL | fn bam() -> Box { + | -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type +LL | if true { +LL | return Struct; + | ^^^^^^ + | | + | expected struct `std::boxed::Box`, found struct `Struct` + | help: store this in the heap by calling `Box::new`: `Box::new(Struct)` + | + = note: expected struct `std::boxed::Box<(dyn Trait + 'static)>` + found struct `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 + | +LL | fn bam() -> Box { + | -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type +... +LL | 42 + | ^^ + | | + | expected struct `std::boxed::Box`, found integer + | help: store this in the heap by calling `Box::new`: `Box::new(42)` + | + = note: expected struct `std::boxed::Box<(dyn Trait + 'static)>` + found type `{integer}` + = 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 + | +LL | fn baq() -> Box { + | -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type +LL | if true { +LL | return 0; + | ^ + | | + | expected struct `std::boxed::Box`, found integer + | help: store this in the heap by calling `Box::new`: `Box::new(0)` + | + = note: expected struct `std::boxed::Box<(dyn Trait + 'static)>` + found type `{integer}` + = 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 + | +LL | fn baq() -> Box { + | -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type +... +LL | 42 + | ^^ + | | + | expected struct `std::boxed::Box`, found integer + | help: store this in the heap by calling `Box::new`: `Box::new(42)` + | + = note: expected struct `std::boxed::Box<(dyn Trait + 'static)>` + found type `{integer}` + = 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 + | +LL | fn baz() -> Box { + | -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type +LL | if true { +LL | Struct + | ^^^^^^ + | | + | expected struct `std::boxed::Box`, found struct `Struct` + | help: store this in the heap by calling `Box::new`: `Box::new(Struct)` + | + = note: expected struct `std::boxed::Box<(dyn Trait + 'static)>` + found struct `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 + | +LL | fn baz() -> Box { + | -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type +... +LL | 42 + | ^^ + | | + | expected struct `std::boxed::Box`, found integer + | help: store this in the heap by calling `Box::new`: `Box::new(42)` + | + = note: expected struct `std::boxed::Box<(dyn Trait + 'static)>` + found type `{integer}` + = 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 + | +LL | fn baw() -> Box { + | -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type +LL | if true { +LL | 0 + | ^ + | | + | expected struct `std::boxed::Box`, found integer + | help: store this in the heap by calling `Box::new`: `Box::new(0)` + | + = note: expected struct `std::boxed::Box<(dyn Trait + 'static)>` + found type `{integer}` + = 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 + | +LL | fn baw() -> Box { + | -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type +... +LL | 42 + | ^^ + | | + | expected struct `std::boxed::Box`, found integer + | help: store this in the heap by calling `Box::new`: `Box::new(42)` + | + = note: expected struct `std::boxed::Box<(dyn Trait + 'static)>` + found type `{integer}` + = 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:27:13 + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:53:13 | LL | fn bat() -> dyn Trait { | ^^^^^^^^^ doesn't have a size known at compile-time @@ -107,7 +235,27 @@ help: return `impl Trait` instead, as all return paths are of type `{integer}`, LL | fn bat() -> impl Trait { | ^^^^^^^^^^ -error: aborting due to 9 previous errors +error[E0746]: return type cannot have an unboxed trait object + --> $DIR/dyn-trait-return-should-be-impl-trait.rs:59: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 | }) + | + +error: aborting due to 18 previous errors Some errors have detailed explanations: E0277, E0308, E0746. For more information about an error, try `rustc --explain E0277`. 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 2/6] 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 12c8de17d10f9..84f1260385553 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 ca8319aa020dc..70c7f791a2b88 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 bf5ba8fff562a..fbf18246e1648 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 e7a8fd304cabe..05c61f1149fb5 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 93414379fd41a..dfcc22aee3485 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 0df6e8f8dc77c..fbad7ec124cb9 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 77288f1badac5..08e16f4693645 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) From 34d51b33787c9b2f59cb3283c8b57a290ab86437 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 24 Jan 2020 11:18:45 -0800 Subject: [PATCH 3/6] Increase suggestion code window from 6 lines to 20 --- src/librustc_errors/emitter.rs | 11 ++++++++--- src/test/ui/issues/issue-22644.stderr | 3 ++- .../unboxed-closure-sugar-lifetime-elision.stderr | 3 ++- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index b0e0cb611afaf..2149e46a1cfbc 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -456,9 +456,14 @@ impl Emitter for SilentEmitter { fn emit_diagnostic(&mut self, _: &Diagnostic) {} } -/// maximum number of lines we will print for each error; arbitrary. +/// Maximum number of lines we will print for each error; arbitrary. pub const MAX_HIGHLIGHT_LINES: usize = 6; -/// maximum number of suggestions to be shown +/// Maximum number of lines we will print for a multiline suggestion; arbitrary. +/// +/// This should be replaced with a more involved mechanism to output multiline suggestions that +/// more closely mimmics the regular diagnostic output, where irrelevant code lines are ellided. +pub const MAX_SUGGESTION_HIGHLIGHT_LINES: usize = 20; +/// Maximum number of suggestions to be shown /// /// Arbitrary, but taken from trait import suggestion limit pub const MAX_SUGGESTIONS: usize = 4; @@ -1521,7 +1526,7 @@ impl EmitterWriter { draw_col_separator_no_space(&mut buffer, 1, max_line_num_len + 1); let mut line_pos = 0; let mut lines = complete.lines(); - for line in lines.by_ref().take(MAX_HIGHLIGHT_LINES) { + for line in lines.by_ref().take(MAX_SUGGESTION_HIGHLIGHT_LINES) { // Print the span column to avoid confusion buffer.puts( row_num, diff --git a/src/test/ui/issues/issue-22644.stderr b/src/test/ui/issues/issue-22644.stderr index 2bddcc2ba56ce..105cf73652586 100644 --- a/src/test/ui/issues/issue-22644.stderr +++ b/src/test/ui/issues/issue-22644.stderr @@ -74,7 +74,8 @@ LL | LL | as LL | LL | - ... +LL | usize) + | error: `<` is interpreted as a start of generic arguments for `usize`, not a shift --> $DIR/issue-22644.rs:32:31 diff --git a/src/test/ui/unboxed-closures/unboxed-closure-sugar-lifetime-elision.stderr b/src/test/ui/unboxed-closures/unboxed-closure-sugar-lifetime-elision.stderr index 0a028e44919a6..469425ea44ded 100644 --- a/src/test/ui/unboxed-closures/unboxed-closure-sugar-lifetime-elision.stderr +++ b/src/test/ui/unboxed-closures/unboxed-closure-sugar-lifetime-elision.stderr @@ -13,7 +13,8 @@ LL | dyn Foo(&isize) -> &isize >(); LL | eq::< dyn for<'a> Foo<(&'a isize,), Output=(&'a isize, &'a isize)>, LL | dyn Foo(&isize) -> (&isize, &isize) >(); LL | - ... +LL | let _: dyn Foo(&isize, &usize) -> &'lifetime usize; + | error: aborting due to previous error From d493dccef7ae1d2ca739fe828bf9556b44dc460a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 24 Jan 2020 11:47:54 -0800 Subject: [PATCH 4/6] Apply `resolve_vars_if_possible` to returned types for more accurate suggestions --- src/librustc/traits/error_reporting/suggestions.rs | 8 ++++++-- .../impl-trait/dyn-trait-return-should-be-impl-trait.rs | 8 ++++---- .../dyn-trait-return-should-be-impl-trait.stderr | 4 ++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/librustc/traits/error_reporting/suggestions.rs b/src/librustc/traits/error_reporting/suggestions.rs index 84f1260385553..cc2c97096e966 100644 --- a/src/librustc/traits/error_reporting/suggestions.rs +++ b/src/librustc/traits/error_reporting/suggestions.rs @@ -605,11 +605,15 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { let tables = self.in_progress_tables.map(|t| t.borrow()).unwrap(); - let mut ret_types = - visitor.returns.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)) + .map(|ty| self.resolve_vars_if_possible(&ty)); let (last_ty, all_returns_have_same_type) = ret_types.clone().fold( (None, true), |(last_ty, mut same): (std::option::Option>, bool), ty| { + let ty = self.resolve_vars_if_possible(&ty); same &= last_ty.map_or(true, |last_ty| last_ty == ty) && ty.kind != ty::Error; (Some(ty), same) }, 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 dfcc22aee3485..08bab5734fd7a 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 @@ -59,15 +59,15 @@ fn baw() -> Box { // Suggest using `impl Trait` fn bat() -> dyn Trait { //~ ERROR E0746 if true { - return 0u32; + return 0; } - 42u32 + 42 } fn bay() -> dyn Trait { //~ ERROR E0746 if true { - 0u32 + 0 } else { - 42u32 + 42 } } 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 fbad7ec124cb9..664a897c593fc 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 @@ -249,7 +249,7 @@ 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 `u32`, which implements `Trait` +help: return `impl Trait` instead, as all return paths are of type `{integer}`, which implements `Trait` | LL | fn bat() -> impl Trait { | ^^^^^^^^^^ @@ -261,7 +261,7 @@ LL | fn bay() -> 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 `u32`, which implements `Trait` +help: return `impl Trait` instead, as all return paths are of type `{integer}`, which implements `Trait` | LL | fn bay() -> impl Trait { | ^^^^^^^^^^ From 600e385c43904eb4a5337427f3f6fb169fe32234 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 24 Jan 2020 14:03:35 -0800 Subject: [PATCH 5/6] review comments --- .../traits/error_reporting/suggestions.rs | 18 ++++++++---------- src/librustc_errors/emitter.rs | 2 +- src/test/ui/error-codes/E0746.fixed | 4 ++-- src/test/ui/error-codes/E0746.rs | 4 ++-- src/test/ui/error-codes/E0746.stderr | 2 +- 5 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/librustc/traits/error_reporting/suggestions.rs b/src/librustc/traits/error_reporting/suggestions.rs index cc2c97096e966..b2aec78c175ad 100644 --- a/src/librustc/traits/error_reporting/suggestions.rs +++ b/src/librustc/traits/error_reporting/suggestions.rs @@ -600,7 +600,7 @@ 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::new(); + let mut visitor = ReturnsVisitor::default(); visitor.visit_body(&body); let tables = self.in_progress_tables.map(|t| t.borrow()).unwrap(); @@ -742,7 +742,7 @@ 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::new(); + let mut visitor = ReturnsVisitor::default(); visitor.visit_body(&body); let tables = self.in_progress_tables.map(|t| t.borrow()).unwrap(); for expr in &visitor.returns { @@ -1696,17 +1696,12 @@ 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. +#[derive(Default)] 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>; @@ -1715,6 +1710,10 @@ impl<'v> Visitor<'v> for ReturnsVisitor<'v> { } fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) { + // Visit every expression to detect `return` paths, either through the function's tail + // expression or `return` statements. We walk all nodes to find `return` statements, but + // we only care about tail expressions when `in_block_tail` is `true`, which means that + // they're in the return path of the function body. match ex.kind { hir::ExprKind::Ret(Some(ex)) => { self.returns.push(ex); @@ -1741,7 +1740,7 @@ impl<'v> Visitor<'v> for ReturnsVisitor<'v> { } fn visit_body(&mut self, body: &'v hir::Body<'v>) { - let prev = self.in_block_tail; + assert!(!self.in_block_tail); if body.generator_kind().is_none() { if let hir::ExprKind::Block(block, None) = body.value.kind { if block.expr.is_some() { @@ -1750,6 +1749,5 @@ impl<'v> Visitor<'v> for ReturnsVisitor<'v> { } } hir::intravisit::walk_body(self, body); - self.in_block_tail = prev; } } diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 2149e46a1cfbc..b62e4223fea10 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -461,7 +461,7 @@ pub const MAX_HIGHLIGHT_LINES: usize = 6; /// Maximum number of lines we will print for a multiline suggestion; arbitrary. /// /// This should be replaced with a more involved mechanism to output multiline suggestions that -/// more closely mimmics the regular diagnostic output, where irrelevant code lines are ellided. +/// more closely mimmics the regular diagnostic output, where irrelevant code lines are elided. pub const MAX_SUGGESTION_HIGHLIGHT_LINES: usize = 20; /// Maximum number of suggestions to be shown /// diff --git a/src/test/ui/error-codes/E0746.fixed b/src/test/ui/error-codes/E0746.fixed index 70c7f791a2b88..ca8319aa020dc 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 0u32; + return 0; } - 42u32 + 42 } fn main() {} diff --git a/src/test/ui/error-codes/E0746.rs b/src/test/ui/error-codes/E0746.rs index fbf18246e1648..bf5ba8fff562a 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 0u32; + return 0; } - 42u32 + 42 } fn main() {} diff --git a/src/test/ui/error-codes/E0746.stderr b/src/test/ui/error-codes/E0746.stderr index 05c61f1149fb5..e7a8fd304cabe 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 `u32`, which implements `Trait` +help: return `impl Trait` instead, as all return paths are of type `{integer}`, which implements `Trait` | LL | fn bar() -> impl Trait { | ^^^^^^^^^^ From 16709f032cfaa0b37a83bc798f0dd4a30d2b2c0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 25 Jan 2020 12:26:33 -0800 Subject: [PATCH 6/6] Revert suggestion window size change --- src/librustc_errors/emitter.rs | 2 +- src/test/ui/issues/issue-22644.stderr | 3 +-- .../unboxed-closure-sugar-lifetime-elision.stderr | 3 +-- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index b62e4223fea10..7218730538a4b 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -462,7 +462,7 @@ pub const MAX_HIGHLIGHT_LINES: usize = 6; /// /// This should be replaced with a more involved mechanism to output multiline suggestions that /// more closely mimmics the regular diagnostic output, where irrelevant code lines are elided. -pub const MAX_SUGGESTION_HIGHLIGHT_LINES: usize = 20; +pub const MAX_SUGGESTION_HIGHLIGHT_LINES: usize = 6; /// Maximum number of suggestions to be shown /// /// Arbitrary, but taken from trait import suggestion limit diff --git a/src/test/ui/issues/issue-22644.stderr b/src/test/ui/issues/issue-22644.stderr index 105cf73652586..2bddcc2ba56ce 100644 --- a/src/test/ui/issues/issue-22644.stderr +++ b/src/test/ui/issues/issue-22644.stderr @@ -74,8 +74,7 @@ LL | LL | as LL | LL | -LL | usize) - | + ... error: `<` is interpreted as a start of generic arguments for `usize`, not a shift --> $DIR/issue-22644.rs:32:31 diff --git a/src/test/ui/unboxed-closures/unboxed-closure-sugar-lifetime-elision.stderr b/src/test/ui/unboxed-closures/unboxed-closure-sugar-lifetime-elision.stderr index 469425ea44ded..0a028e44919a6 100644 --- a/src/test/ui/unboxed-closures/unboxed-closure-sugar-lifetime-elision.stderr +++ b/src/test/ui/unboxed-closures/unboxed-closure-sugar-lifetime-elision.stderr @@ -13,8 +13,7 @@ LL | dyn Foo(&isize) -> &isize >(); LL | eq::< dyn for<'a> Foo<(&'a isize,), Output=(&'a isize, &'a isize)>, LL | dyn Foo(&isize) -> (&isize, &isize) >(); LL | -LL | let _: dyn Foo(&isize, &usize) -> &'lifetime usize; - | + ... error: aborting due to previous error