Improve reference cast help message

[GuillaumeGomez]

Fixes #37338.

[rust-highfive]

r? @eddyb

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

[eddyb]

You completely changed the meaning though, now it's bad if the user wanted to do the pointer thing.
cc @rust-lang/compiler

[GuillaumeGomez]

But at least it's not incorrect anymore.

[TimNN]

@GuillaumeGomez: I'm not sure, but wouldn't your patch suggest dereferencing the pointer also in the following case:

let _ = &String::new() as usize;

Here, the original message is more accurate, I think.

[fhartwig]

In @TimNN's example, the original message clearly makes more sense. In the example from #37338, there is genuine ambiguity about which one the user wanted to do, but I would argue that the new suggestion is probably better because I suspect that the deref case is the more common one.

Would it makes sense to check the RPtr's type and only make the new suggestion if it's a numeric type? Or maybe suggest both solutions in that case, since there isn't a way to figure out which one the user meant.

[eddyb]

I'd argue that only usize is a resonable target for a pointer-to-integer cast, so we could just not suggest anything otherwise - simplest option - besides, @jonathandturner has some general type mismatch suggestion ideas which should apply here as well.

[GuillaumeGomez]

@TimNN: Well saw and not well saw at the same time. In this case, you should have to explicitly cast into a pointer before casting to usize. However, it just puts under the light a big issue: the compiler cannot guess what the user intends to do in some cases and only provide the simplest solution.

In this case, more people will certainly think that you just want to get the length and you put & by mistake.

@fhartwig: Still difficult. But we could provide both helps. What do you think about this @eddyb?

[eddyb]

@GuillaumeGomez IMO having both is more confusing.
Like I said, simplest thing to do is only provide the ptr help when as usize is being attempted.

[GuillaumeGomez]

Ok, I'll update it this way then.

[estebank]

@GuillaumeGomez, I hadn't seen that this PR was already out, so I took a stab at it as well. I still feel there's some value in PR #37442, as it handles the case @TimNN pushed forward as it is now, and specializes the cases of &{integer} and &{float} being cast to {integer} or {float} (as @fhartwig indicated).

For a given file:

fn main() {
    vec![0.0].iter().map(|s| s as i16).collect::<Vec<i16>>();
    let _ = &String::new() as usize;
}

PR #37442 outputs:

error: casting `&f64` as `i16` is invalid
 --> file3.rs:2:35
  |
2 |     vec![0.0].iter().map(|s| s as i16).collect::<Vec<i16>>();
  |                              -    ^^^ casting `&f64` as `i16` is invalid
  |                              |
  |                              try dereferencing for the cast to work

error: casting `&std::string::String` as `usize` is invalid
 --> file3.rs:3:13
  |
3 |     let _ = &String::new() as usize;
  |             ^^^^^^^^^^^^^^^^^^^^^^^ cast through a raw pointer first

error: aborting due to 2 previous errors

[GuillaumeGomez]

@estebank: I still need to update it anyway. I intend to do it this week-end.

[GuillaumeGomez]

Updated.

[frewsxcv]

Travis failure looks unrelated.

Building stage2 tool linkchecker (x86_64-unknown-linux-gnu)
 Downloading url v1.2.2
 Downloading matches v0.1.3
 Downloading idna v0.1.0
 Downloading unicode-normalization v0.1.2
warning: spurious network error (2 tries remaining): [28] Timeout was reached
error: unable to get packages from source

Caused by:
  failed to verify the checksum of `unicode-normalization v0.1.2`

[GuillaumeGomez]

I restart it.

[GuillaumeGomez]

cc @eddyb

[eddyb]

You seem to have removed too many messages.

r? @jonathandturner

[sophiajt]

Agree with @eddyb - why remove the help messages?

[GuillaumeGomez]

@jonathandturner: It comes after a discussion between @eddyb and me. Since a lot of messages were incorrect in some cases, it should be better to just remove them. However, after the last messages, I think I misunderstood him. Any further explanation would be appreciated. :)

[eddyb]

What I mean is only providing the "pointer to integer" help if usize is the target, i.e. hiding it for other integer types.

[GuillaumeGomez]

Updated.

[eddyb]

I don't think you need to change anything in this file.

[eddyb]

These should still emit the help. It's only when that usize is replaced with another type that it's wrong.

[eddyb]

How could this have disappeared?

[arielb1]

Inches is a function pointer.

[GuillaumeGomez]

@eddyb: Didn't pay attention to the tests I updated. Fixed now.

[eddyb]

@GuillaumeGomez Can you add non-usize cases to the existing tests, to show that they don't suggest?
IMO we should also never suggest anything for NeedViaUsize, who casts pointers to floating-point?!

[GuillaumeGomez]

That's another debate I think haha. If you can imagine something weird, someone will do something weirder.

[GuillaumeGomez]

cc @eddyb

[arielb1]

NeedViaUsize was introduced because all directions of casts used to be OK under the pre-1.4.0 cast system. It can now be removed and replaced by IllegalCast.

[arielb1]

@bors r+

[bors]

📌 Commit 37903bf has been approved by arielb1

[bors]

⌛ Testing commit 37903bf with merge f22fdb0...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-cargotest, auto-linux-64-cross-freebsd, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-mac-cross-ios-opt, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-rustbuild
Approved by: arielb1
Pushing f22fdb0 to master...