Skip to content

typeck: refactor patterns => pat.rs + make the def_bm algo more declarative #63862

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 25 commits into from
Aug 25, 2019

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Aug 24, 2019

Spurred by the relative difficulty I had in working up an explanation of how default match bindings work in #63118 (comment), this PR refactors the type checking of patterns into pat.rs.

The PR is probably best read commit-by-commit and includes various changes beyond the following, which are the most important highlights:

  • The algorithm for determining def_bm is encoded in a more declarative fashion now with important sub-steps divided into functions that make sense as logical units (and as described in the reference). This is done starting with "extract is_no_ref_pat." to "extract calc_default_binding_mode".

  • Dedicated functions like check_pat_{lit,range,ident,tuple,box,ref,slice} are then introduced for the various kinds of patterns to make things overall more readable.

  • fn check_pat_top(...) becomes the sole entry point to type checking patterns.

    This will take care of initializing the default binding mode (hence: def_bm) to BindByValue and is called by all contexts that have a pattern that needs to be type checked (functions, match, if let, let, ...). The overall result is that the notion of def_bm is internal to checking patterns.

  • Various diagnostics are extracted to dedicated functions to disturb the flow of type checking logic less.

r? @oli-obk

Centril added 23 commits August 24, 2019 19:13
This clarifies the fact that type checking patterns unconditionally
starts with `BindByValue` as the default binding mode making the
notion of a default binding mode internal to type checking patterns.
It's just shorter and we usually don't use the `_walk` suffix.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 24, 2019
@Centril
Copy link
Contributor Author

Centril commented Aug 24, 2019

@oli-obk I've done a similar dedup in check_pat_box also. =)

Copy link
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors r+ p=1

@Centril
Copy link
Contributor Author

Centril commented Aug 25, 2019

Let's try this again... ;)

@bors r=oli-obk p=1

@bors
Copy link
Collaborator

bors commented Aug 25, 2019

📌 Commit 5a7e1cb has been approved by oli-obk

@bors 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 Aug 25, 2019
@bors
Copy link
Collaborator

bors commented Aug 25, 2019

⌛ Testing commit 5a7e1cb with merge 783469c...

bors added a commit that referenced this pull request Aug 25, 2019
typeck: refactor patterns => `pat.rs`  + make the `def_bm` algo more declarative

Spurred by the relative difficulty I had in working up an explanation of how default match bindings work in #63118 (comment), this PR refactors the type checking of patterns into `pat.rs`.

The PR is probably best read commit-by-commit and includes various changes beyond the following, which are the most important highlights:

- The algorithm for determining `def_bm` is encoded in a more declarative fashion now with important sub-steps divided into functions that make sense as logical units (and as described in the reference). This is done starting with *"extract `is_no_ref_pat`."* to *"extract `calc_default_binding_mode`"*.

- Dedicated functions like `check_pat_{lit,range,ident,tuple,box,ref,slice}` are then introduced for the various kinds of patterns to make things overall more readable.

- `fn check_pat_top(...)` becomes the sole entry point to type checking patterns.

   This will take care of initializing the default binding mode (hence: `def_bm`) to `BindByValue` and is called by all contexts that have a pattern that needs to be type checked (functions, `match`, `if let`, `let`, ...). The overall result is that the notion of `def_bm` is internal to checking patterns.

- Various diagnostics are extracted to dedicated functions to disturb the flow of type checking logic less.

r? @oli-obk
@bors
Copy link
Collaborator

bors commented Aug 25, 2019

☀️ Test successful - checks-azure
Approved by: oli-obk
Pushing 783469c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 25, 2019
@bors bors merged commit 5a7e1cb into rust-lang:master Aug 25, 2019
@Centril Centril deleted the match-cleanup branch August 25, 2019 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants