Exhaustiveness: remove the need for arena-allocation within the algorithm #119581

Nadrieril · 2024-01-04T17:02:11Z

After #119688, exhaustiveness checking doesn't need access to the arena anymore. This simplifies the lifetime story and makes it compile on stable without the extra dependency.

r? @compiler-errors

rustbot · 2024-01-04T17:02:21Z

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Nadrieril · 2024-01-04T17:03:25Z

That's got a high potential of perf impact.

@bors try @rust-timer queue

Exhaustiveness: remove the need for arena-allocation within the algorithm WARNING: skip the first commit, it's from rust-lang#119329 which is getting merged. This nicely cleans up the lifetime story: after this PR, all the `&'p DeconstructedPat` ever handled in the algorithm are coming from user input; we never build one ourselves. r? `@compiler-errors`

bors · 2024-01-04T17:04:37Z

⌛ Trying commit 044ff22 with merge da5b704...

compiler-errors · 2024-01-04T17:14:49Z

compiler/rustc_pattern_analysis/src/lib.rs

@@ -87,11 78,9 @@ pub trait TypeCx: Sized fmt::Debug {
 /// Context that provides information global to a match.
 #[derive(derivative::Derivative)]
 #[derivative(Clone(bound = ""), Copy(bound = ""))]
-pub struct MatchCtxt<'a, 'p, Cx: TypeCx> {
+pub struct MatchCtxt<'a, Cx: TypeCx> {


Can you replace MatchCtxt<'a, Cx> with &'a Cx in all of its usages?

No, there are other things I'll need to put in there later

Currently planning:

the overlapping_range_endpoints Vec I introduce in Exhaustiveness: track overlapping ranges precisely #119396

some kind of step counter to gracefully abort on exponential cases (requested by r-a Add fuel to match checking rust-analyzer#9528)

bors · 2024-01-04T18:31:27Z

☀️ Try build successful - checks-actions
Build commit: da5b704 (da5b704d1e483b378f2384880bb010940ca7ef53)

rust-timer · 2024-01-04T19:54:29Z

Finished benchmarking commit (da5b704): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf 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)	2.8%	[2.7%, 3.0%]	6
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

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

Cycles

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

Binary size

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

Bootstrap: 670.05s -> 669.832s (-0.03%)
Artifact size: 311.83 MiB -> 311.88 MiB (0.02%)

rustbot · 2024-01-04T22:28:13Z

The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging.

cc @davidtwco, @wesleywiser

Nadrieril · 2024-01-04T22:57:30Z

(I'm actually removing a third-party dependency (which I added quite recently (oh well)))

Nadrieril · 2024-01-05T09:49:54Z

I'm realizing this PR does a few different things. Would you prefer me to split it up for ease of review?

compiler-errors · 2024-01-05T23:47:21Z

I'm realizing this PR does a few different things. Would you prefer me to split it up for ease of review?

Yes, I'd like this to be split I think, or at least a write-up that explains wtf it's doing, lol. The PR description is kinda barren.

…s, r=compiler-errors Exhaustiveness: use an `Option` instead of allocating fictitious patterns In the process of exhaustiveness checking, `Matrix` stores a 2D array of patterns. Those are subpatterns of the patterns we were provided as input, _except_ sometimes we allocate some extra wildcard patterns to fill a hole during specialization. Morally though, we could store `Option<&'p DeconstructedPat>` in the matrix, where `None` signifies a wildcard. That way we'd only have "real" patterns in the matrix and we wouldn't need the arena to allocate these wildcards. This is what this PR does. This is part of me splitting up rust-lang#119581 for ease of review. r? `@compiler-errors`

…iler-errors Exhaustiveness: use an `Option` instead of allocating fictitious patterns In the process of exhaustiveness checking, `Matrix` stores a 2D array of patterns. Those are subpatterns of the patterns we were provided as input, _except_ sometimes we allocate some extra wildcard patterns to fill a hole during specialization. Morally though, we could store `Option<&'p DeconstructedPat>` in the matrix, where `None` signifies a wildcard. That way we'd only have "real" patterns in the matrix and we wouldn't need the arena to allocate these wildcards. This is what this PR does. This is part of me splitting up rust-lang/rust#119581 for ease of review. r? `@compiler-errors`

bors · 2024-01-11T05:08:26Z

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

Nadrieril · 2024-01-11T12:52:38Z

@rustbot ready

compiler-errors · 2024-01-14T23:43:45Z

@bors r

bors · 2024-01-14T23:43:48Z

📌 Commit db36304 has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2024-01-15T00:04:11Z

⌛ Testing commit db36304 with merge 9567c3e...

bors · 2024-01-15T02:01:39Z

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

rust-timer · 2024-01-15T06:45:04Z

Finished benchmarking commit (9567c3e): 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)

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

Cycles

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

Binary size

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

Bootstrap: 668.845s -> 666.737s (-0.32%)
Artifact size: 308.21 MiB -> 308.19 MiB (-0.01%)

…iler-errors Exhaustiveness: use an `Option` instead of allocating fictitious patterns In the process of exhaustiveness checking, `Matrix` stores a 2D array of patterns. Those are subpatterns of the patterns we were provided as input, _except_ sometimes we allocate some extra wildcard patterns to fill a hole during specialization. Morally though, we could store `Option<&'p DeconstructedPat>` in the matrix, where `None` signifies a wildcard. That way we'd only have "real" patterns in the matrix and we wouldn't need the arena to allocate these wildcards. This is what this PR does. This is part of me splitting up rust-lang/rust#119581 for ease of review. r? `@compiler-errors`

rustbot assigned compiler-errors Jan 4, 2024

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 Jan 4, 2024

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 4, 2024

This comment has been minimized.

Sign in to view

compiler-errors reviewed Jan 4, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jan 4, 2024

Nadrieril force-pushed the detangle-arena branch from 044ff22 to 59f105b Compare January 4, 2024 22:28

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jan 4, 2024

Nadrieril removed A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jan 4, 2024

Nadrieril mentioned this pull request Jan 4, 2024

Add rustc_pattern_analysis to list of auto published crates rust-analyzer/rustc-auto-publish#3

Merged

Nadrieril 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 Jan 6, 2024

This was referenced Jan 6, 2024

Exhaustiveness: remove Matrix.wildcard_row #119667

Merged

Exhaustiveness: use an Option instead of allocating fictitious patterns #119688

Merged

Nadrieril removed the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Jan 7, 2024

Nadrieril force-pushed the detangle-arena branch from 521a823 to d2d41da Compare January 11, 2024 12:52

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jan 11, 2024

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

Nadrieril removed the A-testsuite Area: The testsuite used to check the correctness of rustc label Jan 11, 2024

This comment was marked as outdated.

Sign in to view

rustc_pattern_analysis no longer needs to be passed an arena

db36304

Nadrieril force-pushed the detangle-arena branch from d2d41da to db36304 Compare January 14, 2024 14:56

rustbot added the A-testsuite Area: The testsuite used to check the correctness of rustc label Jan 14, 2024

compiler-errors approved these changes Jan 14, 2024

View reviewed changes

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 Jan 14, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 15, 2024

bors merged commit 9567c3e into rust-lang:master Jan 15, 2024
12 checks passed

rustbot added this to the 1.77.0 milestone Jan 15, 2024

bors mentioned this pull request Jan 15, 2024

Exhaustiveness: simplify empty pattern logic #119835

Merged

Nadrieril deleted the detangle-arena branch January 15, 2024 11:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exhaustiveness: remove the need for arena-allocation within the algorithm #119581

Exhaustiveness: remove the need for arena-allocation within the algorithm #119581

Nadrieril commented Jan 4, 2024 •

edited

Loading

rustbot commented Jan 4, 2024

Nadrieril commented Jan 4, 2024

This comment has been minimized.

bors commented Jan 4, 2024

This comment has been minimized.

compiler-errors Jan 4, 2024

Nadrieril Jan 4, 2024

compiler-errors Jan 4, 2024

Nadrieril Jan 4, 2024 •

edited

Loading

bors commented Jan 4, 2024

This comment has been minimized.

rust-timer commented Jan 4, 2024

rustbot commented Jan 4, 2024

Nadrieril commented Jan 4, 2024

Nadrieril commented Jan 5, 2024

compiler-errors commented Jan 5, 2024

bors commented Jan 11, 2024

Nadrieril commented Jan 11, 2024

This comment was marked as outdated.

compiler-errors commented Jan 14, 2024

bors commented Jan 14, 2024

bors commented Jan 15, 2024

bors commented Jan 15, 2024

rust-timer commented Jan 15, 2024

Exhaustiveness: remove the need for arena-allocation within the algorithm #119581

Exhaustiveness: remove the need for arena-allocation within the algorithm #119581

Conversation

Nadrieril commented Jan 4, 2024 • edited Loading

rustbot commented Jan 4, 2024

Nadrieril commented Jan 4, 2024

This comment has been minimized.

bors commented Jan 4, 2024

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nadrieril Jan 4, 2024 • edited Loading

Choose a reason for hiding this comment

bors commented Jan 4, 2024

This comment has been minimized.

rust-timer commented Jan 4, 2024

Overall result: ❌ regressions - ACTION NEEDED

rustbot commented Jan 4, 2024

Nadrieril commented Jan 4, 2024

Nadrieril commented Jan 5, 2024

compiler-errors commented Jan 5, 2024

bors commented Jan 11, 2024

Nadrieril commented Jan 11, 2024

This comment was marked as outdated.

compiler-errors commented Jan 14, 2024

bors commented Jan 14, 2024

bors commented Jan 15, 2024

bors commented Jan 15, 2024

rust-timer commented Jan 15, 2024

Overall result: no relevant changes - no action needed

Nadrieril commented Jan 4, 2024 •

edited

Loading

Nadrieril Jan 4, 2024 •

edited

Loading