Suggest `=` to `==` in more cases, even in the face of reference mismatch #119328

estebank · 2023-12-26T21:15:26Z

Given foo: &String and bar: str, suggest == when given if foo = bar {}:

error[E0308]: mismatched types
  --> $DIR/assignment-expected-bool.rs:37:8
   |
LL |     if foo = bar {}
   |        ^^^^^^^^^ expected `bool`, found `()`
   |
help: you might have meant to compare for equality
   |
LL |     if foo == bar {}
   |             +

rustbot · 2023-12-26T21:15:34Z

r? @compiler-errors

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

compiler-errors

r=me after fixing

compiler-errors · 2023-12-26T21:22:41Z

compiler/rustc_hir_typeck/src/expr.rs

@@ -1131,8 +1131,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
            let mut err = self.demand_suptype_diag(expr.span, expected_ty, actual_ty).unwrap();
            let lhs_ty = self.check_expr(lhs);
            let rhs_ty = self.check_expr(rhs);
+            let might_coerce = |lhs: Ty<'tcx>, rhs: Ty<'tcx>| {


This helper is named pretty vaguely -- it's not checking if lhs and rhs might coerce, it's checking if the lhs and rhs's refs can coerce.

Suggested change

let might_coerce = |lhs: Ty<'tcx>, rhs: Ty<'tcx>| {

let refs_can_coerce = |lhs: Ty<'tcx>, rhs: Ty<'tcx>| {

Yeah. It's weird because from the user's point of view it is Ty, but auto-deref is at play in binops (which is why I peel all refs and add one layer after).

compiler/rustc_hir_typeck/src/expr.rs

compiler-errors · 2023-12-26T21:23:51Z

compiler/rustc_hir_typeck/src/expr.rs

-                (Applicability::MaybeIncorrect, self.can_coerce(rhs_ty, actual_lhs_ty))
+                (
+                    Applicability::MaybeIncorrect,
+                    self.can_coerce(rhs_ty, actual_lhs_ty) | might_coerce(rhs_ty, actual_lhs_ty),


binary ||

Suggested change

self.can_coerce(rhs_ty, actual_lhs_ty) | might_coerce(rhs_ty, actual_lhs_ty),

self.can_coerce(rhs_ty, actual_lhs_ty) || might_coerce(rhs_ty, actual_lhs_ty),

I've been making this mistake often lately, I wonder what's causing that 🤔

Maybe that I write alternative patterns way more often than logical ors? No idea 🤷

compiler/rustc_hir_typeck/src/expr.rs

…atch Given `foo: &String` and `bar: str`, suggest `==` when given `if foo = bar {}`: ``` error[E0308]: mismatched types --> $DIR/assignment-expected-bool.rs:37:8 | LL | if foo = bar {} | ^^^^^^^^^ expected `bool`, found `()` | help: you might have meant to compare for equality | LL | if foo == bar {} | + ```

estebank · 2023-12-27T00:21:18Z

@bors r=compiler-errors

bors · 2023-12-27T00:21:21Z

📌 Commit dc30eb1 has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2023-12-27T00:22:30Z

⌛ Testing commit dc30eb1 with merge eee93d8...

bors · 2023-12-27T02:19:28Z

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing eee93d8 to master...

rust-timer · 2023-12-27T05:56:06Z

Finished benchmarking commit (eee93d8): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

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
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-19.2%	[-22.7%, -15.3%]	3
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-19.2%	[-22.7%, -15.3%]	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)	-	-	0
Regressions ❌ (secondary)	2.6%	[2.6%, 2.6%]	1
Improvements ✅ (primary)	-2.8%	[-2.8%, -2.8%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.8%	[-2.8%, -2.8%]	1

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.1%	[2.1%, 2.1%]	1
Improvements ✅ (primary)	-15.5%	[-19.0%, -13.4%]	3
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-15.5%	[-19.0%, -13.4%]	3

Binary size

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

Bootstrap: 669.634s -> 671.325s (0.25%)
Artifact size: 312.43 MiB -> 312.40 MiB (-0.01%)

estebank · 2023-12-27T17:00:03Z

The perf improvement is spurious: #118431 (comment)

rustbot assigned compiler-errors Dec 26, 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 Dec 26, 2023

estebank marked this pull request as ready for review December 26, 2023 21:15

compiler-errors requested changes Dec 26, 2023

View reviewed changes

rustbot 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 Dec 26, 2023

estebank force-pushed the might_coerce_eq_typo branch from 1eb9334 to dc30eb1 Compare December 27, 2023 00:20

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 Dec 27, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 27, 2023

bors merged commit eee93d8 into rust-lang:master Dec 27, 2023
12 checks passed

rustbot added this to the 1.77.0 milestone Dec 27, 2023

estebank mentioned this pull request Dec 27, 2023

Emit better suggestions for &T == T and T == &T #118431

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Suggest `=` to `==` in more cases, even in the face of reference mismatch #119328

Suggest `=` to `==` in more cases, even in the face of reference mismatch #119328

estebank commented Dec 26, 2023

rustbot commented Dec 26, 2023

compiler-errors left a comment

compiler-errors Dec 26, 2023

estebank Dec 26, 2023

compiler-errors Dec 26, 2023

estebank Dec 26, 2023

estebank commented Dec 27, 2023

bors commented Dec 27, 2023

bors commented Dec 27, 2023

bors commented Dec 27, 2023

rust-timer commented Dec 27, 2023

estebank commented Dec 27, 2023

	let might_coerce = \|lhs: Ty<'tcx>, rhs: Ty<'tcx>\| {
	let refs_can_coerce = \|lhs: Ty<'tcx>, rhs: Ty<'tcx>\| {

	self.can_coerce(rhs_ty, actual_lhs_ty) \| might_coerce(rhs_ty, actual_lhs_ty),
	self.can_coerce(rhs_ty, actual_lhs_ty) \|\| might_coerce(rhs_ty, actual_lhs_ty),

Suggest = to == in more cases, even in the face of reference mismatch #119328

Suggest = to == in more cases, even in the face of reference mismatch #119328

Conversation

estebank commented Dec 26, 2023

rustbot commented Dec 26, 2023

compiler-errors 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

estebank commented Dec 27, 2023

bors commented Dec 27, 2023

bors commented Dec 27, 2023

bors commented Dec 27, 2023

rust-timer commented Dec 27, 2023

Overall result: ✅ improvements - no action needed

estebank commented Dec 27, 2023

Suggest `=` to `==` in more cases, even in the face of reference mismatch #119328

Suggest `=` to `==` in more cases, even in the face of reference mismatch #119328