Suggest `=` to `==` in more cases, even in the face of reference mismatch by estebank · Pull Request #119328 · rust-lang/rust

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

            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 suggested 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

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

	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),

Uh oh!

Conversation

estebank commented Dec 26, 2023

Uh oh!

rustbot commented Dec 26, 2023

Uh oh!

compiler-errors left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

estebank commented Dec 27, 2023

Uh oh!

bors commented Dec 27, 2023

Uh oh!

bors commented Dec 27, 2023

Uh oh!

bors commented Dec 27, 2023

Uh oh!

rust-timer commented Dec 27, 2023

Overall result: ✅ improvements - no action needed

Uh oh!

estebank commented Dec 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants