Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Further improve impl Trait/dyn Trait suggestions #68522

Merged
merged 6 commits into from
Jan 26, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 42 additions & 11 deletions src/librustc/traits/error_reporting/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<dyn Trait>` 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<Ty<'_>>, bool), ty| {
Expand Down Expand Up @@ -677,7 +678,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
// Suggest `-> Box<dyn Trait>` 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| {
(
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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> {
estebank marked this conversation as resolved.
Show resolved Hide resolved
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>;
Expand All @@ -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 {
estebank marked this conversation as resolved.
Show resolved Hide resolved
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;
estebank marked this conversation as resolved.
Show resolved Hide resolved
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;
estebank marked this conversation as resolved.
Show resolved Hide resolved
}
}
4 changes: 2 additions & 2 deletions src/test/ui/error-codes/E0746.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
4 changes: 2 additions & 2 deletions src/test/ui/error-codes/E0746.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ fn foo() -> dyn Trait { Struct }

fn bar() -> dyn Trait { //~ ERROR E0746
if true {
return 0;
return 0u32;
estebank marked this conversation as resolved.
Show resolved Hide resolved
}
42
42u32
}

fn main() {}
2 changes: 1 addition & 1 deletion src/test/ui/error-codes/E0746.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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 <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
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 {
| ^^^^^^^^^^
Expand Down
11 changes: 9 additions & 2 deletions src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<dyn Trait> {
if true {
return Struct; //~ ERROR mismatched types
Expand Down Expand Up @@ -52,9 +59,9 @@ fn baw() -> Box<dyn Trait> {
// 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 {
Expand Down
57 changes: 34 additions & 23 deletions src/test/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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 <https://doc.rust-lang.org/book/ch17-02-trait-objects.html#using-trait-objects-that-allow-for-values-of-different-types>
= 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 <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
= 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<dyn Trait> {
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<dyn Trait> {
| -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type
Expand All @@ -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<dyn Trait> {
| -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type
Expand All @@ -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<dyn Trait> {
| -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type
Expand All @@ -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<dyn Trait> {
| -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type
Expand All @@ -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<dyn Trait> {
| -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type
Expand All @@ -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<dyn Trait> {
| -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type
Expand All @@ -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<dyn Trait> {
| -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type
Expand All @@ -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<dyn Trait> {
| -------------- expected `std::boxed::Box<(dyn Trait + 'static)>` because of return type
Expand All @@ -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 <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
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 <https://doc.rust-lang.org/book/ch17-02-trait-objects.html#using-trait-objects-that-allow-for-values-of-different-types>
= 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 <https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits>
= 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<dyn Trait> {
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`.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down