Add boxing suggestions for impl Trait return type mismatches

[Unique-Usman]

A sort of a follow up pr to this -> #155546

[rustbot]

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 73 candidates
• Random selection from 19 candidates

[mejrs]

r? me

We already have a SuggestBoxingForReturnImplTrait suggestion.

Please use it instead. Or ideally, figure out why it's not being emitted in some cases, like here

// suggestion emitted
fn works() -> impl std::fmt::Display {
    match 13 {
        0 => 0i32,
        _ => 1u32,
    }
}

// suggestion not given
fn oh_no() -> impl std::fmt::Display {
    match 13 {
        0 => return 0i32,
        _ => 1u32,
    }
}

error[E0308]: `match` arms have incompatible types
 --> src/lib.rs:5:14
  |
3 | /     match 13 {
4 | |         0 => 0i32,
  | |              ---- this is found to be of type `i32`
5 | |         _ => 1u32,
  | |              ^^^^ expected `i32`, found `u32`
6 | |     }
  | |_____- `match` arms have incompatible types
  |
help: you could change the return type to be a boxed trait object
  |
2 - fn works() -> impl std::fmt::Display {
2 + fn works() -> Box<dyn std::fmt::Display> {
  |
help: if you change the return type to expect trait objects, box the returned expressions
  |
4 ~         0 => Box::new(0i32),
5 ~         _ => Box::new(1u32),
  |
help: change the type of the numeric literal from `u32` to `i32`
  |
5 -         _ => 1u32,
5 +         _ => 1i32,
  |

error[E0308]: mismatched types
  --> src/lib.rs:13:14
   |
10 | fn oh_no() -> impl std::fmt::Display {
   |               ---------------------- expected a single type implementing `std::fmt::Display` because of return type
11 |     match 13 {
12 |         0 => return 0i32,
   |                     ---- return type resolved to be `i32`
13 |         _ => 1u32,
   |              ^^^^ expected `i32`, found `u32`
   |
help: you can convert a `u32` to an `i32` and panic if the converted value doesn't fit
   |
14 |     }.try_into().unwrap()
   |      ++++++++++++++++++++

For more information about this error, try `rustc --explain E0308`.

View changes since this review

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[Unique-Usman]

r? me

We already have a SuggestBoxingForReturnImplTrait suggestion.

That is was used in the pr.

Please use it instead. Or ideally, figure out why it's not being emitted in some cases, like here

// suggestion emitted
fn works() -> impl std::fmt::Display {
    match 13 {
        0 => 0i32,
        _ => 1u32,
    }
}

// suggestion not given
fn oh_no() -> impl std::fmt::Display {
    match 13 {
        0 => return 0i32,
        _ => 1u32,
    }
}

error[E0308]: `match` arms have incompatible types
 --> src/lib.rs:5:14
  |
3 | /     match 13 {
4 | |         0 => 0i32,
  | |              ---- this is found to be of type `i32`
5 | |         _ => 1u32,
  | |              ^^^^ expected `i32`, found `u32`
6 | |     }
  | |_____- `match` arms have incompatible types
  |
help: you could change the return type to be a boxed trait object
  |
2 - fn works() -> impl std::fmt::Display {
2 + fn works() -> Box<dyn std::fmt::Display> {
  |
help: if you change the return type to expect trait objects, box the returned expressions
  |
4 ~         0 => Box::new(0i32),
5 ~         _ => Box::new(1u32),
  |
help: change the type of the numeric literal from `u32` to `i32`
  |
5 -         _ => 1u32,
5 +         _ => 1i32,
  |

error[E0308]: mismatched types
  --> src/lib.rs:13:14
   |
10 | fn oh_no() -> impl std::fmt::Display {
   |               ---------------------- expected a single type implementing `std::fmt::Display` because of return type
11 |     match 13 {
12 |         0 => return 0i32,
   |                     ---- return type resolved to be `i32`
13 |         _ => 1u32,
   |              ^^^^ expected `i32`, found `u32`
   |
help: you can convert a `u32` to an `i32` and panic if the converted value doesn't fit
   |
14 |     }.try_into().unwrap()
   |      ++++++++++++++++++++

For more information about this error, try `rustc --explain E0308`.

View changes since this review

Those two are different errors and are emitted at different locations. While we could make the second case suggest Box::new() for the return type, it is cleaner to let the compiler guide the user (if the user adds Box<dyn std::fmt::Display>, the compiler will then suggest Box::new() in the return expressions). The location where the error is emitted makes it a bit harder to implement, and it could become messy or overly verbose if there are multiple return points, such as in an if-else chain.

[mejrs]

Right, sorry. I meant, rather than emit that custom note, use SuggestBoxingForReturnImplTrait::BoxReturnExpr to provide the Box::new suggestion.

it is cleaner to let the compiler guide the user (if the user adds Box, the compiler will then suggest Box::new() in the return expressions).

No? Sometimes it's unavoidable but I'm not a fan of suggesting fixes that create new errors suggesting more fixes etc. This case may be obvious to you and me but remember it's sometimes hard to tell if the compiler is actually being helpful or if it's guiding you into a dead end.

[Unique-Usman]

Right, sorry. I meant, rather than emit that custom note, use SuggestBoxingForReturnImplTrait::BoxReturnExpr to provide the Box::new suggestion.

it is cleaner to let the compiler guide the user (if the user adds Box, the compiler will then suggest Box::new() in the return expressions).

No? Sometimes it's unavoidable but I'm not a fan of suggesting fixes that create new errors suggesting more fixes etc. This case may be obvious to you and me but remember it's sometimes hard to tell if the compiler is actually being helpful or if it's guiding you into a dead end.

Valid. I will update the pr.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[mejrs]

Thanks!

@bors r+ rollup

View changes since this review

[rust-bors]

📌 Commit a677828 has been approved by mejrs

It is now in the queue for this repository.