Exhaustiveness: keep the original `thir::Pat` around #119233

Nadrieril · 2023-12-22T23:33:31Z

This PR makes it possible for exhaustiveness to look at the original thir::Pat, which I'll need at least for the small_gaps lint (without that we can't distinguish inclusive and exclusive ranges within exhaustiveness). This PR is almost entirely lifetime-wrangling.

rustbot · 2023-12-22T23:33:40Z

r? @TaKO8Ki

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

rustbot · 2023-12-22T23:33:42Z

Some changes occurred in rustc_ty_utils::consts.rs

cc @BoxyUwU

compiler-errors · 2023-12-23T00:58:26Z

r? compiler-errors

I'll review this maybe tomorrow morning

WIP: Lint small gaps between ranges In the discussion to stabilize exclusive range patterns (rust-lang#37854), it has often come up that they're likely to cause off-by-one mistakes. We already have the `overlapping_range_endpoints` lint, so I [proposed](rust-lang#37854 (comment)) a lint to catch the complementary mistake. This PR adds a new `small_gaps_between_ranges` lint that catches likely off-by-one errors with range patterns. Here's the idea (see the test file for more examples): ```rust match x { 0..10 => ..., // WARN: this range doesn't match `10_u8` because `..` is a non-inclusive range 11..20 => ..., // this seems to continue range `0_u8..10_u8`, but `10_u8` isn't matched by either of them _ => ..., } // help: use an inclusive range instead: `0_u8..=10_u8` ``` More precisely: for any exclusive range `lo..hi`, if `hi+1` is matched by another range but `hi` isn't, we suggest writing an inclusive range `lo..=hi` instead. We don't lint `lo..T::MAX` but we could. WARNING: the first 3 commits come from rust-lang#119233, ignore those. r? ghost

compiler-errors

r=me, but I'm a bit confused why 'p and 'thir are different... do you mind double-checking that that's necessary?

compiler-errors · 2023-12-25T22:06:43Z

compiler/rustc_mir_build/src/errors.rs

@@ -453,14 +453,14 @@ pub enum UnusedUnsafeEnclosing {
    },
 }

-pub(crate) struct NonExhaustivePatternsTypeNotEmpty<'p, 'tcx, 'm> {
-    pub cx: &'m RustcMatchCheckCtxt<'p, 'tcx>,
+pub(crate) struct NonExhaustivePatternsTypeNotEmpty<'p, 'thir, 'tcx, 'm> {


When is 'p shorter than 'thir? Isn't 'p the lifetime of the "pattern"? Shouldn't that be from the THIR anyways?

Hehe, my solution to lifetime problems tends to be "add more lifetimes".

'thir is the lifetime of thir::Pat patterns we indeed get from the thir. 'p is the lifetime of the DeconstructedPats we allocate in the arena.

I agree that it feels like 'p should be all we need, lemme try that again now that I understand the situation better

If 'thir and 'p are both covariant, then they should be able to be unified. But maybe not -- I guess give it a shot but r=me if it doesn't end up working out.

compiler-errors · 2023-12-25T22:07:55Z

@rustbot author

compiler-errors · 2023-12-26T03:13:18Z

I was able to cherry-pick this PR quite easily onto #119307, so I would prefer if we land my PR first. It's going to be a nightmare to have to remove 'thir if this one lands first.

See master...compiler-errors:rust:thir-too for proof :)

bors · 2023-12-26T05:25:03Z

☔ The latest upstream changes (presumably #119146) made this pull request unmergeable. Please resolve the merge conflicts.

Nadrieril · 2023-12-26T09:31:49Z

Rebased this onto your #119307, indeed that was very straightforward

compiler-errors

r=me when other pr lands

…adrieril Clean up some lifetimes in `rustc_pattern_analysis` This PR removes some redundant lifetimes. I figured out that we were shortening the lifetime of an arena-allocated `&'p DeconstructedPat<'p>` to `'a DeconstructedPat<'p>`, which forced us to carry both lifetimes when we could otherwise carry just one. This PR also removes and elides some unnecessary lifetimes. I also cherry-picked 0292eb9, and then simplified more lifetimes in `MatchVisitor`, which should make rust-lang#119233 a very simple PR! r? Nadrieril

Rollup merge of rust-lang#119307 - compiler-errors:pat-lifetimes, r=Nadrieril Clean up some lifetimes in `rustc_pattern_analysis` This PR removes some redundant lifetimes. I figured out that we were shortening the lifetime of an arena-allocated `&'p DeconstructedPat<'p>` to `'a DeconstructedPat<'p>`, which forced us to carry both lifetimes when we could otherwise carry just one. This PR also removes and elides some unnecessary lifetimes. I also cherry-picked 0292eb9, and then simplified more lifetimes in `MatchVisitor`, which should make rust-lang#119233 a very simple PR! r? Nadrieril

Nadrieril · 2023-12-26T22:15:29Z

@bors r=compiler-errors

bors · 2023-12-26T22:15:31Z

📌 Commit fc0be3c has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2023-12-26T23:31:38Z

⌛ Testing commit fc0be3c with merge ab8e1fa...

…ompiler-errors Exhaustiveness: keep the original `thir::Pat` around This PR makes it possible for exhaustiveness to look at the original `thir::Pat`, which I'll need at least for the [`small_gaps`](rust-lang#118879) lint (without that we can't distinguish inclusive and exclusive ranges within exhaustiveness). This PR is almost entirely lifetime-wrangling.

rust-log-analyzer · 2023-12-27T00:16:51Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

bors · 2023-12-27T00:17:20Z

💔 Test failed - checks-actions

Nadrieril · 2023-12-27T06:44:17Z

@bors retry

bors · 2023-12-27T07:48:31Z

⌛ Testing commit fc0be3c with merge 9da4432...

bors · 2023-12-27T09:45:45Z

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

rust-timer · 2023-12-27T11:15:46Z

Finished benchmarking commit (9da4432): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

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)	3.7%	[3.6%, 3.9%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

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)	-	-	0
Improvements ✅ (primary)	-0.9%	[-0.9%, -0.9%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.9%	[-0.9%, -0.9%]	1

Binary size

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

Bootstrap: 671.494s -> 670.712s (-0.12%)
Artifact size: 312.45 MiB -> 312.42 MiB (-0.01%)

rustbot assigned TaKO8Ki Dec 22, 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 22, 2023

Nadrieril mentioned this pull request Dec 23, 2023

Lint singleton gaps after exclusive ranges #118879

Merged

Nadrieril force-pushed the keep-whole-pat-around branch from d945d2a to e579fc6 Compare December 23, 2023 00:30

This comment was marked as outdated.

Sign in to view

rustbot assigned compiler-errors and unassigned TaKO8Ki Dec 23, 2023

This comment was marked as outdated.

Sign in to view

Nadrieril force-pushed the keep-whole-pat-around branch from e579fc6 to 4a88c2b Compare December 23, 2023 14:14

This comment was marked as outdated.

Sign in to view

Nadrieril force-pushed the keep-whole-pat-around branch from 4a88c2b to 87e93f6 Compare December 24, 2023 21:10

compiler-errors approved these changes Dec 25, 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 25, 2023

compiler-errors mentioned this pull request Dec 26, 2023

Clean up some lifetimes in rustc_pattern_analysis #119307

Merged

Nadrieril force-pushed the keep-whole-pat-around branch from 87e93f6 to b931cf5 Compare December 26, 2023 09:31

compiler-errors approved these changes Dec 26, 2023

View reviewed changes

Keep reference to the original Pat in DeconstructedPat

fc0be3c

Nadrieril force-pushed the keep-whole-pat-around branch from b931cf5 to fc0be3c Compare December 26, 2023 22:15

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

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 27, 2023

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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 27, 2023

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

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

rustbot added this to the 1.77.0 milestone Dec 27, 2023

Nadrieril deleted the keep-whole-pat-around branch December 27, 2023 10:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exhaustiveness: keep the original `thir::Pat` around #119233

Exhaustiveness: keep the original `thir::Pat` around #119233

Nadrieril commented Dec 22, 2023 •

edited

Loading

rustbot commented Dec 22, 2023

rustbot commented Dec 22, 2023

This comment was marked as outdated.

compiler-errors commented Dec 23, 2023

This comment was marked as outdated.

This comment was marked as outdated.

compiler-errors left a comment

compiler-errors Dec 25, 2023

Nadrieril Dec 25, 2023 •

edited

Loading

compiler-errors Dec 25, 2023

compiler-errors commented Dec 25, 2023

compiler-errors commented Dec 26, 2023 •

edited

Loading

bors commented Dec 26, 2023

Nadrieril commented Dec 26, 2023

compiler-errors left a comment

Nadrieril commented Dec 26, 2023

bors commented Dec 26, 2023

bors commented Dec 26, 2023

rust-log-analyzer commented Dec 27, 2023

bors commented Dec 27, 2023

Nadrieril commented Dec 27, 2023

bors commented Dec 27, 2023

bors commented Dec 27, 2023

rust-timer commented Dec 27, 2023

Exhaustiveness: keep the original thir::Pat around #119233

Exhaustiveness: keep the original thir::Pat around #119233

Conversation

Nadrieril commented Dec 22, 2023 • edited Loading

rustbot commented Dec 22, 2023

rustbot commented Dec 22, 2023

This comment was marked as outdated.

compiler-errors commented Dec 23, 2023

This comment was marked as outdated.

This comment was marked as outdated.

compiler-errors left a comment

Choose a reason for hiding this comment

compiler-errors Dec 25, 2023

Choose a reason for hiding this comment

Nadrieril Dec 25, 2023 • edited Loading

Choose a reason for hiding this comment

compiler-errors Dec 25, 2023

Choose a reason for hiding this comment

compiler-errors commented Dec 25, 2023

compiler-errors commented Dec 26, 2023 • edited Loading

bors commented Dec 26, 2023

Nadrieril commented Dec 26, 2023

compiler-errors left a comment

Choose a reason for hiding this comment

Nadrieril commented Dec 26, 2023

bors commented Dec 26, 2023

bors commented Dec 26, 2023

rust-log-analyzer commented Dec 27, 2023

bors commented Dec 27, 2023

Nadrieril commented Dec 27, 2023

bors commented Dec 27, 2023

bors commented Dec 27, 2023

rust-timer commented Dec 27, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Exhaustiveness: keep the original `thir::Pat` around #119233

Exhaustiveness: keep the original `thir::Pat` around #119233

Nadrieril commented Dec 22, 2023 •

edited

Loading

Nadrieril Dec 25, 2023 •

edited

Loading

compiler-errors commented Dec 26, 2023 •

edited

Loading