Misc HIR typeck type mismatch tweaks #112116

compiler-errors · 2023-05-30T20:45:41Z

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 · 2023-05-30T20:45:47Z

r? @WaffleLapkin

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

tests/ui/suggestions/as-ref.stderr

tests/ui/issues/issue-100605.stderr

estebank

Can we add a check for the following?

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

estebank · 2023-05-31T21:05:37Z

tests/ui/associated-types/dont-suggest-cyclic-constraint.stderr

+error[E0308]: mismatched types
+  --> $DIR/dont-suggest-cyclic-constraint.rs:7:35
+   |
+LL |     debug_assert_eq!(iter.next(), Some(value));
+   |                                   ^^^^^^^^^^^ expected `Option<<I as Iterator>::Item>`, found `Option<&<I as Iterator>::Item>`
+   |
+   = note: expected enum `Option<<I as Iterator>::Item>`
+              found enum `Option<&<I as Iterator>::Item>`


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

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

estebank · 2023-05-31T21:07:49Z

tests/ui/issues/issue-100605.stderr

+   |     ------------ ^^^^ expected `Option<&String>`, found `&Option<String>`
+   |     |


Sad to lose these.

I'll see if I can add it back

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 · 2023-06-05T17:42:16Z

@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 · 2023-06-05T17:46:22Z

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 · 2023-06-05T17:54:54Z

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 · 2023-06-05T18:55:01Z

tests/ui/suggestions/as-ref.stderr

+help: try using `.as_ref()` to convert `&Result<{integer}, _>` to `Result<&usize, _>`
+   |
+LL -     let _: Result<&usize, _> = &Ok(42);
+LL +     let _: Result<&usize, _> = Ok(42).as_ref();


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

compiler/rustc_hir_typeck/src/demand.rs

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

WaffleLapkin · 2023-06-07T16:58:05Z

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

+            } else if let ty::Adt(adt, _) = found_ty_inner.peel_refs().kind()
+                && Some(adt.did()) == self.tcx.lang_items().string()
+                && peeled.is_str()


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

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 · 2023-06-07T16:59:19Z

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

+                err.span_suggestion_verbose(
+                    expr.span.shrink_to_hi(),
+                    fluent::hir_typeck_convert_to_str,
+                    ".map(|x| x.as_str())",


String::as_str without the closure, maybe?

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 · 2023-06-08T16:16:07Z

r=me with(out) nits

compiler-errors · 2023-06-08T17:13:12Z

@bors r=WaffleLapkin

bors · 2023-06-08T17:13:14Z

📌 Commit a918822 has been approved by WaffleLapkin

It is now in the queue for this repository.

bors · 2023-06-09T03:20:25Z

⌛ Testing commit a918822 with merge 9c843d9...

bors · 2023-06-09T06:38:14Z

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

rust-timer · 2023-06-09T07:53:22Z

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 · 2023-06-13T20:00:15Z

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 · 2023-06-13T20:04:53Z

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

rustbot assigned WaffleLapkin May 30, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 30, 2023

WaffleLapkin reviewed May 30, 2023

View reviewed changes

tests/ui/suggestions/as-ref.stderr Outdated Show resolved Hide resolved

rust-cloud-vms bot force-pushed the misc-hir-typeck-mismatch-tweaks branch from ce910f8 to 89032b4 Compare May 30, 2023 22:05

compiler-errors commented May 30, 2023

View reviewed changes

tests/ui/issues/issue-100605.stderr Outdated Show resolved Hide resolved

estebank approved these changes May 31, 2023

View reviewed changes

rust-cloud-vms bot force-pushed the misc-hir-typeck-mismatch-tweaks branch from 7a667ec to 6615862 Compare May 31, 2023 22:22

estebank reviewed Jun 5, 2023

View reviewed changes

WaffleLapkin approved these changes Jun 8, 2023

View reviewed changes

WaffleLapkin added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 8, 2023

compiler-errors added 5 commits June 8, 2023 16:17

Suggest type mismatches even when using ref syntax on binding

522ae84

More robust as_ref/as_deref suggestions

af54d58

Point at correct exprs for assert_eq type mismatch

acf257e

Don't suggest cyclic associated type constraint

c92140e

Peel borrows before suggesting as_ref/as_deref

a918822

rust-cloud-vms bot force-pushed the misc-hir-typeck-mismatch-tweaks branch from 6615862 to a918822 Compare June 8, 2023 16:37

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 8, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 9, 2023

bors merged commit 9c843d9 into rust-lang:master Jun 9, 2023

rustbot added this to the 1.72.0 milestone Jun 9, 2023

rustbot added the perf-regression Performance regression. label Jun 9, 2023

rustbot added the perf-regression-triaged The performance regression has been triaged. label Jun 13, 2023

pnkfelix mentioned this pull request Jun 13, 2023

Uplift clippy::undropped_manually_drops lint #111530

Merged

compiler-errors deleted the misc-hir-typeck-mismatch-tweaks branch August 11, 2023 20:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Misc HIR typeck type mismatch tweaks #112116

Misc HIR typeck type mismatch tweaks #112116

compiler-errors commented May 30, 2023 •

edited

Loading

rustbot commented May 30, 2023

estebank left a comment

estebank May 31, 2023

compiler-errors May 31, 2023

estebank May 31, 2023

compiler-errors May 31, 2023

compiler-errors May 31, 2023 •

edited

Loading

WaffleLapkin commented Jun 5, 2023

compiler-errors commented Jun 5, 2023

compiler-errors commented Jun 5, 2023 •

edited

Loading

estebank Jun 5, 2023

WaffleLapkin Jun 7, 2023

compiler-errors Jun 8, 2023

WaffleLapkin Jun 7, 2023

compiler-errors Jun 8, 2023

WaffleLapkin commented Jun 8, 2023

compiler-errors commented Jun 8, 2023

bors commented Jun 8, 2023

bors commented Jun 9, 2023

bors commented Jun 9, 2023

rust-timer commented Jun 9, 2023

pnkfelix commented Jun 13, 2023

compiler-errors commented Jun 13, 2023

		\| ------------ ^^^^ expected `Option<&String>`, found `&Option<String>`
		\| \|

Misc HIR typeck type mismatch tweaks #112116

Misc HIR typeck type mismatch tweaks #112116

Conversation

compiler-errors commented May 30, 2023 • edited Loading

rustbot commented May 30, 2023

estebank left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

compiler-errors May 31, 2023 • edited Loading

Choose a reason for hiding this comment

WaffleLapkin commented Jun 5, 2023

compiler-errors commented Jun 5, 2023

compiler-errors commented Jun 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WaffleLapkin commented Jun 8, 2023

compiler-errors commented Jun 8, 2023

bors commented Jun 8, 2023

bors commented Jun 9, 2023

bors commented Jun 9, 2023

rust-timer commented Jun 9, 2023

Overall result: ❌ regressions - ACTION NEEDED

pnkfelix commented Jun 13, 2023

compiler-errors commented Jun 13, 2023

compiler-errors commented May 30, 2023 •

edited

Loading

compiler-errors May 31, 2023 •

edited

Loading

compiler-errors commented Jun 5, 2023 •

edited

Loading