[DO NOT MERGE] Enable exhaustive_patterns by default

[camelid]

cc #51085

This doesn't stabilize the feature, it just makes it a no-op. This is to
get a perf run as suggested here.

cc @varkor

[rust-highfive]

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

[camelid]

r? @ghost

[camelid]

It seems like this feature doesn't work right with PhantomData? See the error in CI:

error: unreachable pattern
   --> compiler/rustc_traits/src/chalk/lowering.rs:490:13
    |
490 |             chalk_ir::LifetimeData::Phantom(_, _) => unimplemented!(),
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `-D unreachable-patterns` implied by `-D warnings`

error: aborting due to previous error

[varkor]

LifetimeData::Phantom is defined here: https://github.com/rust-lang/chalk/blob/389a261fa374701441aa64dbdf801d8ff2fcac8f/chalk-ir/src/lib.rs#L1198, and contains a Void parameter, so it is unreachable.

[camelid]

I see, it literally can't be constructed. I forgot about that :)

I removed the match arm and asked in #wg-traits what the purpose of that variant is.

[camelid]

Bootstrapping compilers are so weird 😄

[lcnr]

try doesn't care about tests

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion

[bors]

⌛ Trying commit ea30582cd3597054cf556cc8c695c89ca391fbce with merge 0a6c0a7ffa7f6640700cf2da97187fad8711a6d1...

[bors]

☀️ Try build successful - checks-actions
Build commit: 0a6c0a7ffa7f6640700cf2da97187fad8711a6d1 (0a6c0a7ffa7f6640700cf2da97187fad8711a6d1)

[rust-timer]

Queued 0a6c0a7ffa7f6640700cf2da97187fad8711a6d1 with parent ec039bd, future comparison URL.

[rust-timer]

Finished benchmarking try commit (0a6c0a7ffa7f6640700cf2da97187fad8711a6d1): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot modify labels: +S-waiting-on-review -S-waiting-on-perf

[varkor]

It seems exhaustive_patterns still has very poor performance implications. cc @Nadrieril

[Nadrieril]

Ouch ok. The feature only removes possible branches, so it shouldn't affect performance of the exhaustiveness algorithm itself. Thus the blame seems to lie in the inhabitedness check, as noticed here.
Given that we have to call it at least for every struct&enum (sub)pattern, and that it itself recursively analyses the type, we get more or less O(number_of_matches * number_of_branches * depth_of_pattern * depth_of_type). Caching would turn that into O(depth_of_type) for every type we ever match on.
So my diagnostic is: I expect that caching the inhabitedness check would massively improve perf. Could that be a query? I know nothing about the query architecture but I'd be happy to try.

[camelid]

I know nothing about the query architecture but I'd be happy to try.

Same here, and I would be interested in following along so I understand it better (or at all) :)

[varkor]

I have to admit that whenever I've worked with queries in the past, I've just followed my nose based on how existing queries have been implemented :) It sounds worth a try to me, though!

[rust-timer]

Queued d344606ba6e6c745d677da8ae46f754ec61e5d98 with parent 5be3f9f, future comparison URL.

[rust-timer]

Finished benchmarking try commit (d344606ba6e6c745d677da8ae46f754ec61e5d98): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot modify labels: +S-waiting-on-review -S-waiting-on-perf

[camelid]

Looks like with @Nadrieril's great work on performance, we may now be able to finally stabilize this feature! 🎉

Though I think there might have been some other concerns not related to performance? Though I think they also can be resolved in a backwards-compatible way?

[Nadrieril]

Well I'm waiting on an answer to the never patterns issue that I summarized. This feature is probably introducing unsoundness, which is very not good.

[Nadrieril]

My current plan is to wait for a reply to #78123 and comment in #51085 explaining that the feature probably introduces unsoundness, depending on future plans of what is and isn't UB. I expect we will have to turn never patterns into a proper RFC before we can stabilize exhaustive_patterns.
In parallel, #79670 should get merged and then we can do a final perf test here to separate the perf impact of exhaustive_patterns from the one of my PR.

[Nadrieril]

#79670 got merged! Let's measure again.

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

[bors]

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

1. git checkout enable-exhaustive-patterns (switch to your branch)
2. git fetch upstream master (retrieve the latest master)
3. git rebase upstream/master -p (rebase on top of it)
4. Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
5. git push self enable-exhaustive-patterns --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message

Auto-merging src/test/ui/pattern/usefulness/integer-ranges/pointer-sized-int.rs
Auto-merging src/test/ui/pattern/usefulness/integer-ranges/pointer-sized-int.deny.stderr
Auto-merging src/test/ui/pattern/usefulness/integer-ranges/pointer-sized-int.allow.stderr
Auto-merging src/test/ui/pattern/usefulness/empty-match.normal.stderr
Auto-merging src/test/ui/pattern/usefulness/empty-match.exhaustive_patterns.stderr
Auto-merging compiler/rustc_traits/src/chalk/lowering.rs
Auto-merging compiler/rustc_mir_build/src/thir/pattern/usefulness.rs
Auto-merging compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs
CONFLICT (content): Merge conflict in compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs
Auto-merging compiler/rustc_middle/src/ty/mod.rs
Auto-merging compiler/rustc_middle/src/ty/inhabitedness/def_id_forest.rs
CONFLICT (content): Merge conflict in compiler/rustc_middle/src/ty/inhabitedness/def_id_forest.rs
Auto-merging compiler/rustc_middle/src/query/mod.rs
Automatic merge failed; fix conflicts and then commit the result.

[Nadrieril]

Oh, I forgot this PR included #79670 ><. @camelid could you rebase it?

[Nadrieril]

I resolved the conflicts myself using the GitHub interface; it's nice!
@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

[bors]

⌛ Trying commit eb4a187 with merge 82e461272563be4e708e0a70d043653baed29179...

[rust-log-analyzer]

The job mingw-check failed! Check out the build log: (web) (plain)

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

configure: rust.channel         := nightly
configure: rust.debug-assertions := True
configure: llvm.assertions      := True
configure: dist.missing-tools   := True
configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
configure: writing `config.toml` in current directory
configure: 
configure: run `python /checkout/x.py --help`
configure: 
---
skip untracked path cpu-usage.csv during rustfmt invocations
skip untracked path src/doc/book/ during rustfmt invocations
skip untracked path src/doc/rust-by-example/ during rustfmt invocations
skip untracked path src/llvm-project/ during rustfmt invocations
Diff in /checkout/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs at line 921:
                 // as though it had an "unknown" constructor to avoid exposing its emptiness. The
                 // exception is if the pattern is at the top level, because we want empty matches to be
                 // considered exhaustive.
-                let is_secretly_empty = def.variants.is_empty()
-                    && !true
Running `"/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/rustfmt" "--config-path" "/checkout" "--edition" "2018" "--unstable-features" "--skip-children" "--check" "/checkout/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs"` failed.
If you're running `tidy`, try again with `--bless`. Or, if you just want to format code, run `./x.py fmt` instead.
-                    && !pcx.is_top_level;
+                let is_secretly_empty = def.variants.is_empty() && !true && !pcx.is_top_level;
 
                 if is_secretly_empty || is_declared_nonexhaustive {
                     smallvec![NonExhaustive]
Build completed unsuccessfully in 0:00:17

[bors]

☀️ Try build successful - checks-actions
Build commit: 82e461272563be4e708e0a70d043653baed29179 (82e461272563be4e708e0a70d043653baed29179)

[rust-timer]

Queued 82e461272563be4e708e0a70d043653baed29179 with parent fd2df74, future comparison URL.

@rustbot label: +S-waiting-on-perf

[rust-timer]

Finished benchmarking try commit (82e461272563be4e708e0a70d043653baed29179): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

[camelid]

Overall, perf looks neutral! Perhaps very slight regressions, but I think within the margin of error. Thanks to @Nadrieril for putting in the work to speed it up!

[jackh726]

@camelid What's the plan for this going forward? Is there more to do or does this just need a review/decision? (I see that CI failed last run, but logs are expired)

[camelid]

@camelid What's the plan for this going forward? Is there more to do or does this just need a review/decision? (I see that CI failed last run, but logs are expired)

This was just an experiment to see what the perf situation was; I didn't close it because I wasn't sure if people wanted to do more tests with it. I believe what's left is making a decision about stabilization; @Nadrieril may know more.

[Nadrieril]

Performance used to be a blocker for the exhaustive_patterns feature. This PR was only there to check perf, and now it's fine so we can close it. Discussion will continue on the tracking issue #51085