.try_into().unwrap() is suggested even when .into() would work

[jonas-schievink]

fn returns_usize() -> usize {
    let x = 0u8;
    x
}

error[E0308]: mismatched types
 --> src/lib.rs:3:5
  |
1 | fn returns_usize() -> usize {
  |                       ----- expected `usize` because of return type
2 |     let x = 0u8;
3 |     x
  |     ^ expected `usize`, found `u8`
  |
help: you can convert an `u8` to `usize` and panic if the converted value wouldn't fit
  |
3 |     x.try_into().unwrap()
  |

It would be better to suggest x.into() instead, which is shorter, cannot fail, and doesn't require importing a trait.

This issue has been assigned to @ryr3 via this comment.

[estebank]

You need to change the following

rust/src/librustc_typeck/check/demand.rs

Lines 755 to 770 in 6dee5f1

	(&ty::Int(ref exp), &ty::Int(ref found)) => {
	let is_fallible = match (found.bit_width(), exp.bit_width()) {
	(Some(found), Some(exp)) if found > exp => true,
	(None, _) | (_, None) => true,
	_ => false,
	};
	suggest_to_change_suffix_or_into(err, is_fallible);
	true
	}
	(&ty::Uint(ref exp), &ty::Uint(ref found)) => {
	let is_fallible = match (found.bit_width(), exp.bit_width()) {
	(Some(found), Some(exp)) if found > exp => true,
	(None, _) | (_, None) => true,
	_ => false,
	};
	suggest_to_change_suffix_or_into(err, is_fallible);

so that when one of the bit_with is None and the other is 8 or 16 bits is_fallible is set to true.

[ryr3]

Hi, this seems like a good first issue. Can I take this up as my first contribution?

[estebank]

@ryr3 sure! Go ahead. Visit the rustc guide if this is your first time compiling rustc.

[Dylan-DPC-zz]

@rustbot assign @ryr3

[LeSeulArtichaut]

so that when one of the bit_with is None and the other is 8 or 16 bits is_fallible is set to true

@estebank Isn't it the opposite? If we try to convert a u8 to a usize, we want is_faillible to be false so we suggest .into() instead of .try_into().unwrap().

[tsandstr]

Yeah. It's the other way around.

[ryr3]

Sorry about those notifications, all. I was experimenting a bit.
Would that fix be good for a PR here, @estebank ?

ryr3@39d2b4a

[LeSeulArtichaut]

@ryr3 If it works you should open a PR I think. Have you fixed the tests that will be affected by this change? You can do so by running x.py with the --bless flag, like: x.py test -i --stage 1 --bless src/test/ui.

[estebank]

There might not be tests exercising this code path, but you can add some cases to src/test/ui/integer-literal-suffix-inference.rs.

The code change looks good. You will want to also run ./x.py fmt just to make sure it doesn't get rejected due to formatting style (although the diff I saw looks good to my eyes). Feel free to create a PR with these changes, I'll be more than happy to review/approve.

[LeSeulArtichaut]

There might not be tests exercising this code path, but you can add some cases to src/test/ui/integer-literal-suffix-inference.rs

There still are some. For example:

rust/src/test/ui/numeric/numeric-cast.stderr

Lines 23 to 32 in 53712f8

	error[E0308]: mismatched types
	--> $DIR/numeric-cast.rs:27:18
	|
	LL | foo::<usize>(x_u16);
	| ^^^^^ expected `usize`, found `u16`
	|
	help: you can convert an `u16` to `usize` and panic if the converted value wouldn't fit
	|
	LL | foo::<usize>(x_u16.try_into().unwrap());
	| ^^^^^^^^^^^^^^^^^^^^^^^^^

needs to be changed

[estebank]

@LeSeulArtichaut that one seems almost incidental. Given that the test matrix in this case is small enough, it's a good idea to exhaust it in a central place, both for the positive cases (we suggest .into()) and the negative cases (anything bigger than 16bits suggests .try_into().unwrap()). It's my fault that the *size cases weren't in that test already 😬.

[ryr3]

With the changes I've made now, it's such that the compiler will suggest usize be .into() when the function requires a u8. Is that fine, because usize will greater than u8 on most targets?
And From(usize) is not implemented for u8/u16 anyway so that would be a bad suggestion.

Also, when I'm writing tests for the *size cases, should I use the usize::max_value() and min value functions or just their result on the largest target?

[LeSeulArtichaut]

If I understand the code correctly, in your changes, the pattern (None, Some(8 | 16)) represents the case where we expect a usize value, but we have a u8 or u16 value instead. In this case, we want is_fallible to be false.
The pattern (Some(8 | 16), None) however, represents the case where we expect a u8 or a u16 but we have a usize. Then, we want is_fallible to be true for the reasons you explained.
So if you delete the second pattern, the case where we want to cast usize to u8/u16 will get picked up by the next arm and is_fallible will be true as expected.

As for the testing, I think the value doesn't really matter because those checks are about types, not values themselves. Rust doesn't make implicit conversions. You can just use random values like 5 or 2, or usize::max_value() if you want.

[ryr3]

Thanks! I just realized that it would be the other way around here, because in

rust/src/librustc_typeck/check/demand.rs

Line 756 in 6dee5f1

let is_fallible = match (found.bit_width(), exp.bit_width()) {

and

rust/src/librustc_typeck/check/demand.rs

Line 765 in 6dee5f1

let is_fallible = match (found.bit_width(), exp.bit_width()) {

the match parameters are found, exp whereas they are in the order of exp, found everywhere else, including the compiler messages.
Should I rearrange that too (within the match statements only where appropriate) or leave it as it is?

[estebank]

It'll be great if you can bring some consistency in the order.