Prevent spurious `unreachable pattern` lints #115937

oli-obk · 2023-09-18T15:55:00Z

But it means we'll get more non-exhaustive patterns

rustbot · 2023-09-18T15:55:11Z

Some changes might have occurred in exhaustiveness checking

RalfJung · 2023-09-18T17:30:44Z

Hm, that's not clearly better, is it? Just replacing one spurious error by another. I guess there is no way around an 'Error' variant.

oli-obk · 2023-09-18T17:51:45Z

An error variant will be equivalent to skipping non-exhaustive patterns errors if any patterns return true for has_error (which such an error constant will do).

I can try adding this here

Nadrieril · 2023-09-18T18:40:06Z

An error variant will be equivalent to skipping non-exhaustive patterns errors if any patterns return true for has_error (which such an error constant will do).

I can try adding this here

I'd like that, that feels cleanest to me

rustbot · 2023-09-19T11:50:06Z

Some changes might have occurred in exhaustiveness checking

cc @Nadrieril

compiler/rustc_mir_build/src/thir/pattern/check_match.rs

tests/ui/pattern/usefulness/consts-opaque.rs

compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs

bors · 2023-09-21T15:02:45Z

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

bors · 2023-09-26T16:21:50Z

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

bors · 2023-09-27T16:03:59Z

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

compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs

Nadrieril · 2023-10-03T20:15:10Z

tests/ui/const-generics/generic_const_exprs/typeid-equality-by-subtyping.stderr

+error[E0015]: cannot match on `TypeId` in constant functions
+  --> $DIR/typeid-equality-by-subtyping.rs:18:9
+   |
+LL |         WHAT_A_TYPE => 0,
+   |         ^^^^^^^^^^^
+   |
+   = note: `TypeId` cannot be compared in compile-time, and therefore cannot be used in `match`es
+note: impl defined here, but it is not `const`
+  --> $SRC_DIR/core/src/any.rs:LL:COL
+   = note: calls in constant functions are limited to constant functions, tuple structs and tuple variants
+
+error: aborting due to 2 previous errors


Where is this error coming from? I guess it was there before but got skipped because of previous errors?

oh yea. We're actually generating a PartialEq call for it now, because we have an actual pattern now instead of a wildcard, which has no additional code.

Nadrieril · 2023-10-03T20:21:54Z

Oops I missed that I got assigned 😄

LGTM! The behavior is what I hoped, and const-to-pat looks good. Just the error propagation machinery is out of my expertise. r=me if you're confident about that bit, unless @RalfJung has more comments.

This will fix #78057

Nadrieril · 2023-10-11T01:12:12Z

I found another case of spurious such lints in #116623 (in the tests I added). Whichever PR gets merged first, the other can add a fix.
EDIT: or I'll do that in a new PR, no worries

Nadrieril · 2023-10-11T12:39:20Z

@rustbot label +S-waiting-on-author -S-waiting-on-review

Means you'll get more `non-exhaustive` patterns

… of recreating it with `delay_span_bug`

oli-obk · 2023-10-11T13:12:19Z

@bors r=Nadrieril

bors · 2023-10-11T13:12:21Z

📌 Commit eca786c has been approved by Nadrieril

It is now in the queue for this repository.

bors · 2023-10-11T14:49:52Z

⌛ Testing commit eca786c with merge 3c23df4...

bors · 2023-10-11T16:44:56Z

☀️ Test successful - checks-actions
Approved by: Nadrieril
Pushing 3c23df4 to master...

rust-timer · 2023-10-11T18:07:12Z

Finished benchmarking commit (3c23df4): comparison URL.

Overall result: ❌ regressions - 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)	1.0%	[1.0%, 1.0%]	1
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)	5.6%	[5.6%, 5.6%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.3%	[-1.3%, -1.3%]	1
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: 627.767s -> 628.128s (0.06%)
Artifact size: 271.31 MiB -> 271.28 MiB (-0.01%)

Nadrieril · 2023-10-11T18:09:41Z

Looks like the perf drop exactly mirrors the surprising perf gain I got in my previous PR

Prevent more spurious unreachable pattern lints Continues the work of rust-lang#115937 by introducing `PatKind::Error`, to be used instead of `PatKind::Wild` when an error was raised during pattern lowering. Most of match checking lints are skipped when a `PatKind::Error` is encountered. This avoids confusing extra warnings when a pattern is malformed. Now `PatKind::Wild` should indicate an actual wildcard pattern. r? `@oli-obk`

Rollup merge of rust-lang#116715 - Nadrieril:patkind-error, r=oli-obk Prevent more spurious unreachable pattern lints Continues the work of rust-lang#115937 by introducing `PatKind::Error`, to be used instead of `PatKind::Wild` when an error was raised during pattern lowering. Most of match checking lints are skipped when a `PatKind::Error` is encountered. This avoids confusing extra warnings when a pattern is malformed. Now `PatKind::Wild` should indicate an actual wildcard pattern. r? `@oli-obk`

rustbot assigned Nadrieril Sep 18, 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 Sep 18, 2023

RalfJung reviewed Sep 19, 2023

View reviewed changes

compiler/rustc_mir_build/src/thir/pattern/check_match.rs Show resolved Hide resolved

RalfJung reviewed Sep 19, 2023

View reviewed changes

tests/ui/pattern/usefulness/consts-opaque.rs Show resolved Hide resolved

RalfJung reviewed Sep 19, 2023

View reviewed changes

compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs Outdated Show resolved Hide resolved

oli-obk force-pushed the spurious_unreachable_pattern branch from 6237927 to 53ed4db Compare September 22, 2023 09:35

This comment has been minimized.

Sign in to view

oli-obk force-pushed the spurious_unreachable_pattern branch from 53ed4db to 615547c Compare September 22, 2023 09:43

oli-obk mentioned this pull request Sep 26, 2023

Always fall back to PartialEq when a constant in a pattern is not recursively structural-eq #105750

Merged

oli-obk force-pushed the spurious_unreachable_pattern branch from 615547c to 8c52eaa Compare September 27, 2023 07:25

oli-obk mentioned this pull request Sep 27, 2023

thir::pattern: update some comments and error type names #115887

Merged

oli-obk force-pushed the spurious_unreachable_pattern branch from 8c52eaa to a693c7c Compare September 28, 2023 16:00

Nadrieril reviewed Oct 3, 2023

View reviewed changes

compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs Outdated Show resolved Hide resolved

Nadrieril reviewed Oct 3, 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 Oct 11, 2023

oli-obk added 2 commits October 11, 2023 12:49

Prevent spurious unreachable pattern lints

d1fd11f

Means you'll get more `non-exhaustive` patterns

Avoid emitting the non_exhaustive error if other errors already occurred

e83467c

Remember the ErrorReported used to silence follow up errors instead…

eca786c

… of recreating it with `delay_span_bug`

oli-obk force-pushed the spurious_unreachable_pattern branch from a693c7c to eca786c Compare October 11, 2023 13:11

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 Oct 11, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 11, 2023

bors merged commit 3c23df4 into rust-lang:master Oct 11, 2023
12 checks passed

rustbot added this to the 1.75.0 milestone Oct 11, 2023

This was referenced Oct 11, 2023

use PatKind::Error when an ADT const value has violation #116522

Merged

Rollup of 5 pull requests #116636

Closed

oli-obk deleted the spurious_unreachable_pattern branch October 11, 2023 18:35

Dajamante mentioned this pull request Oct 12, 2023

Pull upstream master 2023 10 12 ferrocene/ferrocene#48

Merged

Nadrieril mentioned this pull request Oct 14, 2023

Prevent more spurious unreachable pattern lints #116715

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent spurious `unreachable pattern` lints #115937

Prevent spurious `unreachable pattern` lints #115937

oli-obk commented Sep 18, 2023 •

edited

Loading

rustbot commented Sep 18, 2023

RalfJung commented Sep 18, 2023 via email

oli-obk commented Sep 18, 2023

Nadrieril commented Sep 18, 2023

rustbot commented Sep 19, 2023

bors commented Sep 21, 2023

This comment has been minimized.

bors commented Sep 26, 2023

bors commented Sep 27, 2023

Nadrieril Oct 3, 2023

oli-obk Oct 11, 2023

Nadrieril commented Oct 3, 2023

Nadrieril commented Oct 11, 2023 •

edited

Loading

Nadrieril commented Oct 11, 2023

oli-obk commented Oct 11, 2023

bors commented Oct 11, 2023

bors commented Oct 11, 2023

bors commented Oct 11, 2023

rust-timer commented Oct 11, 2023

Nadrieril commented Oct 11, 2023

Prevent spurious unreachable pattern lints #115937

Prevent spurious unreachable pattern lints #115937

Conversation

oli-obk commented Sep 18, 2023 • edited Loading

rustbot commented Sep 18, 2023

RalfJung commented Sep 18, 2023 via email

oli-obk commented Sep 18, 2023

Nadrieril commented Sep 18, 2023

rustbot commented Sep 19, 2023

bors commented Sep 21, 2023

This comment has been minimized.

bors commented Sep 26, 2023

bors commented Sep 27, 2023

Nadrieril Oct 3, 2023

Choose a reason for hiding this comment

oli-obk Oct 11, 2023

Choose a reason for hiding this comment

Nadrieril commented Oct 3, 2023

Nadrieril commented Oct 11, 2023 • edited Loading

Nadrieril commented Oct 11, 2023

oli-obk commented Oct 11, 2023

bors commented Oct 11, 2023

bors commented Oct 11, 2023

bors commented Oct 11, 2023

rust-timer commented Oct 11, 2023

Overall result: ❌ regressions - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Nadrieril commented Oct 11, 2023

Prevent spurious `unreachable pattern` lints #115937

Prevent spurious `unreachable pattern` lints #115937

oli-obk commented Sep 18, 2023 •

edited

Loading

Nadrieril commented Oct 11, 2023 •

edited

Loading