Clarify constructor splitting in exhaustiveness checking #80242

Nadrieril · 2020-12-20T17:59:01Z

I reworked the explanation of the algorithm completely to make it properly account for the various extensions we've added. This includes constructor splitting, which was previously not clearly included in the algorithm. This makes wildcards less magical; I added some detailed examples; and this distinguishes clearly between constructors that only make sense in patterns (like ranges) and those that make sense for values (like Some). This reformulation had been floating around in my mind for a while, and I'm quite happy with how it turned out. Let me know how you feel about it.
I also factored out all three cases of splitting (wildcards, ranges and slices) into dedicated structs to encapsulate the complicated bits.
I measured no perf impact but I don't trust my local measurements for refactors since #79284.

r? @varkor
@rustbot modify labels: +A-exhaustiveness-checking

lqd · 2020-12-20T18:44:38Z

@bors try @rust-timer queue

rust-timer · 2020-12-20T18:44:39Z

Awaiting bors try build completion.

bors · 2020-12-20T18:44:49Z

⌛ Trying commit ddc29bf59218179b7b829d9f7a99121d8e9932fb with merge 92e2d3444aa509193c1d1e464229151050a23df7...

rust-log-analyzer · 2020-12-20T19:12:17Z

The job dist-x86_64-linux failed! Check out the build log: (web) (plain)

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

 Documenting rustc_typeck v0.0.0 (/checkout/compiler/rustc_typeck)
 Documenting rustc_mir v0.0.0 (/checkout/compiler/rustc_mir)
    Checking rustc_plugin_impl v0.0.0 (/checkout/compiler/rustc_plugin_impl)
 Documenting rustc_plugin_impl v0.0.0 (/checkout/compiler/rustc_plugin_impl)
error: unresolved link to `super::check_match::MatchVisitor::lower_pattern`
     |
     |
1014 |     /// [`super::check_match::MatchVisitor::lower_pattern`].
     |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item named `MatchVisitor` in module `check_match`
     |
     = note: `-D broken-intra-doc-links` implied by `-D warnings`

error: unresolved link to `super::check_match::MatchVisitor::lower_pattern`
     |
     |
1033 | /// [`super::check_match::MatchVisitor::lower_pattern`].
     |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item named `MatchVisitor` in module `check_match`
error: aborting due to 2 previous errors

error: could not document `rustc_mir_build`


Caused by:
  process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustdoc --edition=2018 --crate-type lib --crate-name rustc_mir_build compiler/rustc_mir_build/src/lib.rs --target x86_64-unknown-linux-gnu -o /checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/doc --error-format=json --json=diagnostic-rendered-ansi -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/release/deps -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/release/deps --extern rustc_apfloat=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_apfloat-e9bfb5f44b74579f.rmeta --extern rustc_arena=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_arena-e2f417d3f6c44169.rmeta --extern rustc_ast=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_ast-af8907c51d0619b2.rmeta --extern rustc_attr=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_attr-2f11728d63006441.rmeta --extern rustc_data_structures=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_data_structures-900d11de835d34e6.rmeta --extern rustc_errors=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_errors-1aabeb89ce38e926.rmeta --extern rustc_hir=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_hir-782d95bf352570dd.rmeta --extern rustc_index=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_index-984047fbd60b0386.rmeta --extern rustc_infer=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_infer-01d356349ffa8afd.rmeta --extern rustc_middle=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_middle-fc5655365d5ee0f8.rmeta --extern rustc_serialize=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_serialize-0f41645d10f64d62.rmeta --extern rustc_session=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_session-b88d46fe3041e688.rmeta --extern rustc_span=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_span-3f54fec704da7e59.rmeta --extern rustc_target=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_target-a7d4facd2979b981.rmeta --extern rustc_trait_selection=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_trait_selection-d813db1c1fb0c7e8.rmeta --extern smallvec=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/release/deps/libsmallvec-e1ca8c1925105298.rmeta --extern tracing=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-rustc/x86_64-unknown-linux-gnu/release/deps/libtracing-166f52f5b1e92350.rmeta -Dwarnings -Winvalid_codeblock_attributes --crate-version '1.50.0-nightly
  (92e2d3444
  2020-12-20)' --document-private-items --enable-index-page -Zunstable-options -Znormalize-docs` (exit code: 1)
warning: could not parse code block as Rust code
   --> compiler/rustc_typeck/src/check/upvar.rs:350:9
    |
350 |       /// ```
---

error: build failed


command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "doc" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "16" "--release" "--locked" "--color" "always" "--features" "jemalloc llvm max_level_info" "--manifest-path" "/checkout/compiler/rustc/Cargo.toml" "--no-deps" "-p" "rustc_typeck" "-p" "rustc_symbol_mangling" "-p" "rustc_query_system" "-p" "rustc_ty_utils" "-p" "rustc_session" "-p" "rustc_hir_pretty" "-p" "rustc_ast_passes" "-p" "rustc_error_codes" "-p" "rustc_expand" "-p" "coverage_test_macros" "-p" "rustc_mir_build" "-p" "rustc_codegen_llvm" "-p" "rustc_index" "-p" "rustc_ast_lowering" "-p" "rustc_middle" "-p" "rustc_feature" "-p" "rustc_errors" "-p" "rustc_plugin_impl" "-p" "rustc_target" "-p" "rustc_llvm" "-p" "rustc_ast" "-p" "rustc_arena" "-p" "rustc_save_analysis" "-p" "rustc_type_ir" "-p" "rustc_lexer" "-p" "rustc_graphviz" "-p" "rustc_mir" "-p" "rustc_privacy" "-p" "rustc_parse_format" "-p" "rustc_interface" "-p" "rustc_driver" "-p" "rustc_apfloat" "-p" "rustc_builtin_macros" "-p" "rustc_incremental" "-p" "rustc_traits" "-p" "rustc_span" "-p" "rustc_macros" "-p" "rustc_trait_selection" "-p" "rustc_resolve" "-p" "rustc_hir" "-p" "rustc_codegen_ssa" "-p" "rustc_infer" "-p" "rustc_fs_util" "-p" "rustc_lint_defs" "-p" "rustc_passes" "-p" "rustc_lint" "-p" "rustc_parse" "-p" "rustc_attr" "-p" "rustc_serialize" "-p" "rustc_metadata" "-p" "rustc_data_structures" "-p" "rustc_ast_pretty"


failed to run: /checkout/obj/build/bootstrap/debug/bootstrap dist --host x86_64-unknown-linux-gnu --target x86_64-unknown-linux-gnu --include-default-paths src/tools/build-manifest
Build completed unsuccessfully in 0:24:06

bors · 2020-12-20T19:12:27Z

💔 Test failed - checks-actions

lqd · 2020-12-20T20:05:45Z

@bors try @rust-timer queue

rust-timer · 2020-12-20T20:05:46Z

Awaiting bors try build completion.

bors · 2020-12-20T20:05:56Z

⌛ Trying commit 6d885fb79cc8105204f03736a4b5cc13da4377ec with merge 63732d777c405f4a8e36687aaee37dadf67b14bc...

bors · 2020-12-20T20:55:29Z

☀️ Try build successful - checks-actions
Build commit: 63732d777c405f4a8e36687aaee37dadf67b14bc (63732d777c405f4a8e36687aaee37dadf67b14bc)

rust-timer · 2020-12-20T20:55:31Z

Queued 63732d777c405f4a8e36687aaee37dadf67b14bc with parent b0e5c7d, future comparison URL.

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

rust-timer · 2020-12-21T00:28:52Z

Finished benchmarking try commit (63732d777c405f4a8e36687aaee37dadf67b14bc): 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

bjorn3 · 2020-12-21T09:10:52Z

This is a slight regression.

varkor

I'm happy with the changes after the few minor comments, but maybe you have an idea about how to avoid the regression, @Nadrieril? I think it's negligible enough not to worry, but perhaps an #[inline] in the right place could do the trick.

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

varkor · 2020-12-21T19:05:30Z

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

+///     [..] => {}
+/// }
+/// ```
+/// Here are the results of specialization for the first few lengths:


This example is nice and clear :)

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

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

rust-log-analyzer · 2020-12-22T06:23:59Z

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:

Nadrieril · 2020-12-22T07:05:56Z

Thanks for spotting all my typos! I've stared at it for too long I can't see them myself anymore ^^.
The perf regression seems to come from 17d9a5f where I add the Missing constructor. I've noticed that match-stress-enum gets consistently affected when I add or remove variants to the Constructor enum, so it makes sense. Not much I can do about that one.
#78242 will cause a conflict. I'll have a rebase ready for when it gets merged.

bors · 2020-12-22T13:54:25Z

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

Co-authored-by: varkor <github@varkor.com>

varkor · 2020-12-22T15:56:08Z

Thanks!

@bors r+

bors · 2020-12-22T15:56:09Z

📌 Commit be23694 has been approved by varkor

bors · 2020-12-22T21:51:10Z

⌛ Testing commit be23694 with merge 969b42d...

bors · 2020-12-23T00:41:40Z

☀️ Test successful - checks-actions
Approved by: varkor
Pushing 969b42d to master...

rust-highfive assigned varkor Dec 20, 2020

rustbot added the A-exhaustiveness-checking Relating to exhaustiveness / usefulness checking of patterns label Dec 20, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 20, 2020

bors 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 20, 2020

Nadrieril force-pushed the explain-and-factor-splitting branch from ddc29bf to 6d885fb Compare December 20, 2020 19:21

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 20, 2020

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Dec 21, 2020

varkor reviewed Dec 21, 2020

View reviewed changes

Nadrieril force-pushed the explain-and-factor-splitting branch from 6494fba to 3e5ba51 Compare December 22, 2020 06:11

Nadrieril force-pushed the explain-and-factor-splitting branch from 3e5ba51 to 7cf3783 Compare December 22, 2020 06:58

Add some tests

42b77c7

Nadrieril and others added 12 commits December 22, 2020 15:20

Factor out SplitIntRange used for integer range splitting

5a24b2c

Run the annoying lint separately

7948f91

Factor out SplitVarLenSlice used for slice splitting

9d0c2ed

Rebrand MissingConstructors as SplitWildcard

bbb4ac0

Inline all_constructors

3141f2d

Inline the constructor-specific split functions

8b38b68

Pass Matrix explicitly instead of via PatCtxt

43d445c

Completely rework the explanation of the algorithm

2a541ce

Make the special "missing patterns" constructor real

53e03fb

Simplify field filtering

1c176d1

Apply suggestions from code review

85fdb34

Co-authored-by: varkor <github@varkor.com>

Fix a comment

be23694

Nadrieril force-pushed the explain-and-factor-splitting branch from 7cf3783 to be23694 Compare December 22, 2020 15:20

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 23, 2020

bors merged commit 969b42d into rust-lang:master Dec 23, 2020

rustbot added this to the 1.50.0 milestone Dec 23, 2020

This was referenced Dec 23, 2020

Refractor a few more types to rustc_type_ir #79951

Merged

Rollup of 9 pull requests #80282

Closed

Nadrieril deleted the explain-and-factor-splitting branch December 23, 2020 04:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clarify constructor splitting in exhaustiveness checking #80242

Clarify constructor splitting in exhaustiveness checking #80242

Nadrieril commented Dec 20, 2020

lqd commented Dec 20, 2020

rust-timer commented Dec 20, 2020

bors commented Dec 20, 2020

rust-log-analyzer commented Dec 20, 2020

bors commented Dec 20, 2020

lqd commented Dec 20, 2020

rust-timer commented Dec 20, 2020

bors commented Dec 20, 2020

bors commented Dec 20, 2020

rust-timer commented Dec 20, 2020

rust-timer commented Dec 21, 2020

bjorn3 commented Dec 21, 2020

varkor left a comment

varkor Dec 21, 2020

rust-log-analyzer commented Dec 22, 2020

Nadrieril commented Dec 22, 2020

bors commented Dec 22, 2020

varkor commented Dec 22, 2020

bors commented Dec 22, 2020

bors commented Dec 22, 2020

bors commented Dec 23, 2020

Clarify constructor splitting in exhaustiveness checking #80242

Clarify constructor splitting in exhaustiveness checking #80242

Conversation

Nadrieril commented Dec 20, 2020

lqd commented Dec 20, 2020

rust-timer commented Dec 20, 2020

bors commented Dec 20, 2020

rust-log-analyzer commented Dec 20, 2020

bors commented Dec 20, 2020

lqd commented Dec 20, 2020

rust-timer commented Dec 20, 2020

bors commented Dec 20, 2020

bors commented Dec 20, 2020

rust-timer commented Dec 20, 2020

rust-timer commented Dec 21, 2020

bjorn3 commented Dec 21, 2020

varkor left a comment

Choose a reason for hiding this comment

varkor Dec 21, 2020

Choose a reason for hiding this comment

rust-log-analyzer commented Dec 22, 2020

Nadrieril commented Dec 22, 2020

bors commented Dec 22, 2020

varkor commented Dec 22, 2020

bors commented Dec 22, 2020

bors commented Dec 22, 2020

bors commented Dec 23, 2020