Misc HIR typeck type mismatch tweaks

[compiler-errors]

These are all intended to improve #112104, but I couldn't get it to actually suggest adding as_ref to the LHS of the equality expr without some hacks that I may play around with some more.

Each commit's title should explain what it's doing except for perhaps the last one, which addresses the bogus suggestion on #112104 itself.

[rustbot]

r? @WaffleLapkin

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

[estebank]

Can we add a check for the following?

let _: Result<&usize, _> = &Ok(42);

[estebank]

The expectation in the ticket was for this to suggest .as_ref(), right?

[compiler-errors]

Yes, which is why I didn't close it just yet.

[estebank]

Sad to lose these.

[compiler-errors]

I'll see if I can add it back

[compiler-errors]

Wait, I'm not actually sure what you're talking about when looking at this now 😅

For the &Some case, we still give the right suggestion below:

help: try using `.as_ref()` to convert `&Option<String>` to `Option<&String>`
   |
LL -     takes_option(&res);
LL +     takes_option(res.as_ref());

And for the &None case, we no longer suggest None.as_ref(), in favor of just suggesting removing the & which is far more accurate.

[WaffleLapkin]

@compiler-errors the tests look good, thanks for the work! I'll review the code shortly...

One question though, does the code support something like this:

let _: Option<&usize> = Some(1);

I'd expect rustc to suggest .as_ref() here, since it should be fine with auto ref coercions (in this exact case it could also suggest &1, but given an opaque Option<usize> we should probably try suggesting as_ref).

[compiler-errors]

let _: Option<&usize> = Some(1);

@WaffleLapkin: I don't think it does. I debated adding support for autoref-ish suggestions, but that case specifically has a higher likelihood than the others to lead to later borrowck errors compared to just deref-related suggestions because it may require taking a reference to a temporary or something.

[compiler-errors]

Oops, correction: it does suggest y.as_ref(), but yeah, it needs to be an "opaque" option like:

let y = Some(1);
let _: Option<&usize> = y;

Which says:

help: try using `.as_ref()` to convert `Option<{integer}>` to `Option<&usize>`
  |
3 |     let _: Option<&usize> = y.as_ref();
  |                              +++++++++

Otherwise the compiler prefers to suggest Some(&1) which I do agree is a better suggestion when possible.

[estebank]

Despite the concerns of this not being idiomatic, I'm more than ok with landing with a suggestion like this.

[WaffleLapkin]

Why this checks for string specifically? (and not for example for a type with as_ref with an appropriate signature or something like that)

[compiler-errors]

Because it's complicated, 🤷

I didn't implement this specific suggestion, just moved it around, and I don't really want to make it particularly more complicated 😅

[WaffleLapkin]

String::as_str without the closure, maybe?

[compiler-errors]

I think this applies with any number of peeled refs, so this won't work:

fn f(x: Option<&&&&String>) {
  let _: Option<&str> = x;
}

[WaffleLapkin]

r=me with(out) nits

[compiler-errors]

@bors r=WaffleLapkin

[bors]

📌 Commit a918822 has been approved by WaffleLapkin

It is now in the queue for this repository.

[bors]

⌛ Testing commit a918822 with merge 9c843d9...

[bors]

☀️ Test successful - checks-actions
Approved by: WaffleLapkin
Pushing 9c843d9 to master...

[rust-timer]

Finished benchmarking commit (9c843d9): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.7%	[0.7%, 0.8%]	3
Regressions ❌ (secondary)	0.4%	[0.3%, 0.7%]	11
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.2%]	1
All ❌✅ (primary)	0.7%	[0.7%, 0.8%]	3

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	3.4%	[3.4%, 3.4%]	1
Regressions ❌ (secondary)	2.0%	[2.0%, 2.0%]	1
Improvements ✅ (primary)	-1.3%	[-2.9%, -0.4%]	5
Improvements ✅ (secondary)	-2.2%	[-2.2%, -2.2%]	1
All ❌✅ (primary)	-0.5%	[-2.9%, 3.4%]	6

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.5%	[2.5%, 2.5%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 649.387s -> 649.729s (0.05%)

[pnkfelix]

• only primary benchmark to regress was helloworld (3 incr check variants), and not by all that much (relatively speaking)
• secondary regressions were mainly to unify-linearly, await-call-tree, token-stream-stress.
• impact seems acceptable, marking as triaged.

@rustbot label: +perf-regression-triaged

[compiler-errors]

Also I don't think I changed anything that isn't on the error path.