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

Suggest `try_into` when possible #60159

Merged
merged 3 commits into from Apr 30, 2019

Conversation

Projects
None yet
5 participants
@estebank
Copy link
Contributor

commented Apr 21, 2019

CC #47168

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Apr 21, 2019

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

| ^^^^^^^^^
| ^^
| |
| expected i16, found i8

This comment has been minimized.

Copy link
@oli-obk

oli-obk Apr 22, 2019

Contributor

I wonder if we should change the mismathced types root message to expected i16, found i8 so the suggestion can be inlined properly

This comment has been minimized.

Copy link
@estebank

estebank Apr 22, 2019

Author Contributor

It took me a second to understand but now I see what you mean... That's probably something worthy of doing in a separate PR. The actual code change should be smallish, but the effect in stderr files will probably be extensive.

@estebank

This comment has been minimized.

Copy link
Contributor Author

commented Apr 29, 2019

@rust-highfive rust-highfive assigned oli-obk and unassigned pnkfelix Apr 29, 2019


error[E0308]: mismatched types
--> $DIR/repeat_count.rs:28:23
|
LL | let f = [0_usize; -1_isize];
| ^^^^^^^^ expected usize, found isize
help: you can convert an `isize` to `usize` or panic if it the converted value wouldn't fit
|
LL | let f = [0_usize; (-1_isize).try_into().unwrap()];

This comment has been minimized.

Copy link
@oli-obk

oli-obk Apr 29, 2019

Contributor

for now ok, but we should try to not suggest code that is known to panic

This comment has been minimized.

Copy link
@estebank

estebank Apr 29, 2019

Author Contributor

This is indeed a bad corner case :-/

This comment has been minimized.

Copy link
@estebank

estebank Apr 29, 2019

Author Contributor

Filed #60384

@oli-obk
Copy link
Contributor

left a comment

how does this code interact with constants? Do we have some tests for that? For now we should not be suggesting that const FOO: u32 = bar(); should use try_from, or from irrelevant of the return type of bar

@estebank

This comment has been minimized.

Copy link
Contributor Author

commented Apr 29, 2019

how does this code interact with constants?

I'm checking but, I think we have any guards against that at the moment, given this current example where rustc gives suggestions in expr context, but not const context.


It does the right thing:

const C: usize = 32;
const D: i32 = 1usize;
const E: i32 = C;
const G: i8 = 1usize;
const H: i8 = C;

fn main() {
    let c: i32 = 1i8;
    let x: u8 = C;
    let x: u8 = D;
    let x: u8 = E;
}
error[E0308]: mismatched types
 --> file.rs:2:16
  |
2 | const D: i32 = 1usize;
  |                ^^^^^^ expected i32, found usize

error[E0308]: mismatched types
 --> file.rs:3:16
  |
3 | const E: i32 = C;
  |                ^ expected i32, found usize

error[E0308]: mismatched types
 --> file.rs:4:15
  |
4 | const G: i8 = 1usize;
  |               ^^^^^^ expected i8, found usize

error[E0308]: mismatched types
 --> file.rs:5:15
  |
5 | const H: i8 = C;
  |               ^ expected i8, found usize

error[E0308]: mismatched types
 --> file.rs:8:18
  |
8 |     let c: i32 = 1i8;
  |                  ^^^ expected i32, found i8
help: change the type of the numeric literal from `i8` to `i32`
  |
8 |     let c: i32 = 1i32;
  |                  ^^^^

error[E0308]: mismatched types
 --> file.rs:9:17
  |
9 |     let x: u8 = C;
  |                 ^ expected u8, found usize
help: you can convert an `usize` to `u8` or panic if it the converted value wouldn't fit
  |
9 |     let x: u8 = C.try_into().unwrap();
  |                 ^^^^^^^^^^^^^^^^^^^^^

error[E0308]: mismatched types
  --> file.rs:10:17
   |
10 |     let x: u8 = D;
   |                 ^ expected u8, found i32
help: you can convert an `i32` to `u8` or panic if it the converted value wouldn't fit
   |
10 |     let x: u8 = D.try_into().unwrap();
   |                 ^^^^^^^^^^^^^^^^^^^^^

error[E0308]: mismatched types
  --> file.rs:11:17
   |
11 |     let x: u8 = E;
   |                 ^ expected u8, found i32
help: you can convert an `i32` to `u8` or panic if it the converted value wouldn't fit
   |
11 |     let x: u8 = E.try_into().unwrap();
   |                 ^^^^^^^^^^^^^^^^^^^^^
@oli-obk

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

@estebank

This comment has been minimized.

Copy link
Contributor Author

commented Apr 29, 2019

@oli-obk oh :(

@estebank estebank force-pushed the estebank:type-mismatch-cast branch from f474297 to 31eb5cc Apr 30, 2019

@estebank

This comment has been minimized.

Copy link
Contributor Author

commented Apr 30, 2019

@oli-obk const fns are now accounted for.

@oli-obk

This comment has been minimized.

Copy link
Contributor

commented Apr 30, 2019

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

commented Apr 30, 2019

📌 Commit 31eb5cc has been approved by oli-obk

@bors

This comment has been minimized.

Copy link
Contributor

commented Apr 30, 2019

⌛️ Testing commit 31eb5cc with merge 862a878...

bors added a commit that referenced this pull request Apr 30, 2019

Auto merge of #60159 - estebank:type-mismatch-cast, r=oli-obk
Suggest `try_into` when possible

CC #47168
@bors

This comment has been minimized.

Copy link
Contributor

commented Apr 30, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: oli-obk
Pushing 862a878 to master...

@bors bors added the merged-by-bors label Apr 30, 2019

@bors bors merged commit 31eb5cc into rust-lang:master Apr 30, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.