Skip to content
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

Initial implementation of or-patterns #61708

Merged
merged 2 commits into from
Aug 18, 2019
Merged

Conversation

dlrobertson
Copy link
Contributor

@dlrobertson dlrobertson commented Jun 10, 2019

An incomplete implementation of or-patterns (e.g. Some(0 | 1) as a pattern). This patch set aims to implement initial parsing of or-patterns.

Related to: #54883

CC @alexreg @varkor
r? @Centril

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 10, 2019
Copy link
Contributor Author

@dlrobertson dlrobertson left a comment

Choose a reason for hiding this comment

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

Comments and critiques on how or-patterns are parsed would be very much appreciated.

PatternKind::AscribeUserType { .. } |
PatternKind::Array { .. } |
PatternKind::Wild |
PatternKind::Binding { .. } |
PatternKind::Leaf { .. } |
PatternKind::Deref { .. } => {
self.error_simplifyable(match_pair)
None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

My current plan is to lower each "subpattern" here and create the TestKind::Or variant that will contain the TestKind's for the "subpatterns"

src/libsyntax/parse/parser.rs Outdated Show resolved Hide resolved
src/librustc_mir/hair/pattern/mod.rs Outdated Show resolved Hide resolved
src/libsyntax/feature_gate.rs Outdated Show resolved Hide resolved
src/libsyntax/ast.rs Show resolved Hide resolved
@Centril
Copy link
Contributor

Centril commented Jun 10, 2019

r? @varkor

@rust-highfive rust-highfive assigned varkor and unassigned Centril Jun 10, 2019
@Centril

This comment has been minimized.

@petrochenkov petrochenkov self-assigned this Jun 10, 2019
@petrochenkov petrochenkov removed their assignment Jun 10, 2019
@bors

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@dlrobertson dlrobertson force-pushed the or-patterns-0 branch 3 times, most recently from db3f303 to 37d8f63 Compare June 21, 2019 19:19
Copy link
Contributor Author

@dlrobertson dlrobertson left a comment

Choose a reason for hiding this comment

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

Added some of my WIP work in MIR. I'd appreciate any feedback on the lowering there.

NB: or-patterns that include Wild or Switch currently fail. Range and SwitchInt seem to work though.

src/librustc_mir/build/matches/mod.rs Outdated Show resolved Hide resolved
// can let the test create its blocks before the rest of the match.
// This currently improves the speed of llvm when optimizing long
// string literal matches
trait TargetBlockBuilder<'tcx> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What used to be the make_target_blocks closure is now implemented in the impl of the TargetBlockBuilder trait. This is needed so that we can build out the target blocks based on the or-pattern's subtests.

@matthewjasper does this seem like a reasonable approach?

src/test/ui/or-patterns/basic-switch.rs Outdated Show resolved Hide resolved
src/test/ui/or-patterns/mix-with-wild.rs Outdated Show resolved Hide resolved
@rust-highfive

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@dlrobertson dlrobertson force-pushed the or-patterns-0 branch 2 times, most recently from e9c8d34 to c23e486 Compare June 23, 2019 01:29
@bors

This comment has been minimized.

@varkor

This comment has been minimized.

@Centril
Copy link
Contributor

Centril commented Aug 17, 2019

r=me with comments ^-- addressed for the parser changes and varkor on the rest since that didn't change.

varkor and others added 2 commits August 17, 2019 15:05
Initial implementation of parsing or-patterns e.g., `Some(Foo | Bar)`.
This is a partial implementation of RFC 2535.
@dlrobertson
Copy link
Contributor Author

@bors r=centril

@bors
Copy link
Contributor

bors commented Aug 17, 2019

📌 Commit 1870537 has been approved by centril

@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 17, 2019
@bors
Copy link
Contributor

bors commented Aug 18, 2019

⌛ Testing commit 1870537 with merge fc8765d...

bors added a commit that referenced this pull request Aug 18, 2019
Initial implementation of or-patterns

An incomplete implementation of or-patterns (e.g. `Some(0 | 1)` as a pattern). This patch set aims to implement initial parsing of `or-patterns`.

Related to: #54883

CC @alexreg @varkor
r? @Centril
@bors
Copy link
Contributor

bors commented Aug 18, 2019

☀️ Test successful - checks-azure
Approved by: centril
Pushing fc8765d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 18, 2019
@bors bors merged commit 1870537 into rust-lang:master Aug 18, 2019
@Centril Centril added the F-or_patterns `#![feature(or_patterns)]` label Aug 18, 2019
bors added a commit to rust-lang/rust-clippy that referenced this pull request Aug 18, 2019
Fix breakage due to  rust-lang/rust#61708

Includes commits from #4406

changelog: none
@dlrobertson dlrobertson deleted the or-patterns-0 branch August 19, 2019 23:15
Centril added a commit to Centril/rust that referenced this pull request Aug 26, 2019
…ebank

Fully implement or-pattern parsing

Builds upon the initial parsing in rust-lang#61708 to fully implement or-pattern (`p | q`) parsing as specified in [the grammar section of RFC 2535](https://github.com/rust-lang/rfcs/blob/master/text/2535-or-patterns.md#grammar).

Noteworthy:

- We allow or-patterns in `[p | q, ...]`.
- We allow or-patterns in `let` statements and `for` expressions including with leading `|`.
- We improve recovery for `p || q` (+ tests for that in `multiple-pattern-typo.rs`).
- We improve recovery for `| p | q` in inner patterns (tests in `or-patterns-syntactic-fail.rs`).
- We rigorously test or-pattern parsing (in `or-patterns-syntactic-{pass,fail}.rs`).
- We harden the feature gating tests.
- We do **_not_** change `ast.rs`. That is, `ExprKind::Let.0` and `Arm.pats` still accept `Vec<P<Pat>>`.
   I was starting work on that but it would be cleaner to do this in a separate PR so this one has a narrower scope.

cc @dlrobertson
cc the tracking issue rust-lang#54883.

r? @estebank
Centril added a commit to Centril/rust that referenced this pull request Sep 5, 2019
…ochenkov

or-patterns: Uniformly use `PatKind::Or` in AST & Fix/Cleanup resolve

Following up on work in rust-lang#63693 and rust-lang#61708, in this PR we:

- Uniformly use `PatKind::Or(...)` in AST:

   - Change `ast::Arm.pats: Vec<P<Pat>>` => `ast::Arm.pat: P<Pat>`

   - Change `ast::ExprKind::Let.0: Vec<P<Pat>>` => `ast::ExprKind::Let.0: P<Pat>`

- Adjust `librustc_resolve/late.rs` to correctly handle or-patterns at any level of nesting as a result.

  In particular, the already-bound check which rejects e.g. `let (a, a);` now accounts for or-patterns. The consistency checking (ensures no missing bindings and binding mode consistency) also now accounts for or-patterns. In the process, a bug was found in the current compiler which allowed:

   ```rust
   enum E<T> { A(T, T), B(T) }
   use E::*;
   fn foo() {
       match A(0, 1) {
           B(mut a) | A(mut a, mut a) => {}
       }
   }
   ```

   The new algorithms took a few iterations to get right. I tried several clever schemes but ultimately a version based on a stack of hashsets and recording product/sum contexts was chosen since it is more clearly correct.

- Clean up `librustc_resolve/late.rs` by, among other things, using a new `with_rib` function to better ensure stack dicipline.

- Do not push the change in AST to HIR for now to avoid doing too much in this PR. To cope with  this, we introduce a temporary hack in `rustc::hir::lowering` (clearly marked in the diff).

cc rust-lang#54883
cc @dlrobertson @matthewjasper
r? @petrochenkov
Centril added a commit to Centril/rust that referenced this pull request Sep 5, 2019
…ochenkov

or-patterns: Uniformly use `PatKind::Or` in AST & Fix/Cleanup resolve

Following up on work in rust-lang#63693 and rust-lang#61708, in this PR we:

- Uniformly use `PatKind::Or(...)` in AST:

   - Change `ast::Arm.pats: Vec<P<Pat>>` => `ast::Arm.pat: P<Pat>`

   - Change `ast::ExprKind::Let.0: Vec<P<Pat>>` => `ast::ExprKind::Let.0: P<Pat>`

- Adjust `librustc_resolve/late.rs` to correctly handle or-patterns at any level of nesting as a result.

  In particular, the already-bound check which rejects e.g. `let (a, a);` now accounts for or-patterns. The consistency checking (ensures no missing bindings and binding mode consistency) also now accounts for or-patterns. In the process, a bug was found in the current compiler which allowed:

   ```rust
   enum E<T> { A(T, T), B(T) }
   use E::*;
   fn foo() {
       match A(0, 1) {
           B(mut a) | A(mut a, mut a) => {}
       }
   }
   ```

   The new algorithms took a few iterations to get right. I tried several clever schemes but ultimately a version based on a stack of hashsets and recording product/sum contexts was chosen since it is more clearly correct.

- Clean up `librustc_resolve/late.rs` by, among other things, using a new `with_rib` function to better ensure stack dicipline.

- Do not push the change in AST to HIR for now to avoid doing too much in this PR. To cope with  this, we introduce a temporary hack in `rustc::hir::lowering` (clearly marked in the diff).

cc rust-lang#54883
cc @dlrobertson @matthewjasper
r? @petrochenkov
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Sep 6, 2019
…ochenkov

or-patterns: Uniformly use `PatKind::Or` in AST & Fix/Cleanup resolve

Following up on work in rust-lang#63693 and rust-lang#61708, in this PR we:

- Uniformly use `PatKind::Or(...)` in AST:

   - Change `ast::Arm.pats: Vec<P<Pat>>` => `ast::Arm.pat: P<Pat>`

   - Change `ast::ExprKind::Let.0: Vec<P<Pat>>` => `ast::ExprKind::Let.0: P<Pat>`

- Adjust `librustc_resolve/late.rs` to correctly handle or-patterns at any level of nesting as a result.

  In particular, the already-bound check which rejects e.g. `let (a, a);` now accounts for or-patterns. The consistency checking (ensures no missing bindings and binding mode consistency) also now accounts for or-patterns. In the process, a bug was found in the current compiler which allowed:

   ```rust
   enum E<T> { A(T, T), B(T) }
   use E::*;
   fn foo() {
       match A(0, 1) {
           B(mut a) | A(mut a, mut a) => {}
       }
   }
   ```

   The new algorithms took a few iterations to get right. I tried several clever schemes but ultimately a version based on a stack of hashsets and recording product/sum contexts was chosen since it is more clearly correct.

- Clean up `librustc_resolve/late.rs` by, among other things, using a new `with_rib` function to better ensure stack dicipline.

- Do not push the change in AST to HIR for now to avoid doing too much in this PR. To cope with  this, we introduce a temporary hack in `rustc::hir::lowering` (clearly marked in the diff).

cc rust-lang#54883
cc @dlrobertson @matthewjasper
r? @petrochenkov
Centril added a commit to Centril/rust that referenced this pull request Sep 6, 2019
…ochenkov

or-patterns: Uniformly use `PatKind::Or` in AST & Fix/Cleanup resolve

Following up on work in rust-lang#63693 and rust-lang#61708, in this PR we:

- Uniformly use `PatKind::Or(...)` in AST:

   - Change `ast::Arm.pats: Vec<P<Pat>>` => `ast::Arm.pat: P<Pat>`

   - Change `ast::ExprKind::Let.0: Vec<P<Pat>>` => `ast::ExprKind::Let.0: P<Pat>`

- Adjust `librustc_resolve/late.rs` to correctly handle or-patterns at any level of nesting as a result.

  In particular, the already-bound check which rejects e.g. `let (a, a);` now accounts for or-patterns. The consistency checking (ensures no missing bindings and binding mode consistency) also now accounts for or-patterns. In the process, a bug was found in the current compiler which allowed:

   ```rust
   enum E<T> { A(T, T), B(T) }
   use E::*;
   fn foo() {
       match A(0, 1) {
           B(mut a) | A(mut a, mut a) => {}
       }
   }
   ```

   The new algorithms took a few iterations to get right. I tried several clever schemes but ultimately a version based on a stack of hashsets and recording product/sum contexts was chosen since it is more clearly correct.

- Clean up `librustc_resolve/late.rs` by, among other things, using a new `with_rib` function to better ensure stack dicipline.

- Do not push the change in AST to HIR for now to avoid doing too much in this PR. To cope with  this, we introduce a temporary hack in `rustc::hir::lowering` (clearly marked in the diff).

cc rust-lang#54883
cc @dlrobertson @matthewjasper
r? @petrochenkov
Centril added a commit to Centril/rust that referenced this pull request Sep 22, 2019
or-patterns: Push `PatKind/PatternKind::Or` at top level to HIR & HAIR

Following up on work in rust-lang#64111, rust-lang#63693, and rust-lang#61708, in this PR:

- We change `hair::Arm.patterns: Vec<Pattern<'_>>` into `hir::Arm.pattern: Pattern<'_>`.

   - `fn hair::Arm::top_pats_hack` is introduced as a temporary crutch in MIR building to avoid more changes.

- We change `hir::Arm.pats: HirVec<P<Pat>>` into `hir::Arm.pat: P<Pat>`.

   - The hacks in `rustc::hir::lowering` are removed since the representation hack is no longer necessary.

   - In some places, `fn hir::Arm::top_pats_hack` is introduced to leave some things as future work.

   - Misc changes: HIR pretty printing is adjusted to behave uniformly wrt. top/inner levels, rvalue promotion is adjusted, regionck, and dead_code is also.

   - Type checking is adjusted to uniformly handle or-patterns at top/inner levels.
      To make things compile, `p_0 | ... | p_n` is redefined as a "reference pattern" in [`fn is_non_ref_pat`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_typeck/check/struct.FnCtxt.html#method.is_non_ref_pat) for now. This is done so that reference types are not eagerly stripped from the `expected: Ty<'tcx>`.

    - Liveness is adjusted wrt. the `unused_variables` and `unused_assignments` lints to handle top/inner levels uniformly and the handling of `fn` parameters, `let` locals, and `match` arms are unified in this respect. This is not tested for now as exhaustiveness checks are reachable and will ICE.

    - In `check_match`, checking `@` and by-move bindings is adjusted. However, exhaustiveness checking is not adjusted the moment and is handled by @dlrobertson in rust-lang#63688.

    - AST borrowck (`construct.rs`) is not adjusted as AST borrowck will be removed soon.

r? @matthewjasper
cc @dlrobertson @varkor @oli-obk
bors added a commit that referenced this pull request Sep 23, 2019
or-patterns: Push `PatKind/PatternKind::Or` at top level to HIR & HAIR

Following up on work in #64111, #63693, and #61708, in this PR:

- We change `hair::Arm.patterns: Vec<Pattern<'_>>` into `hir::Arm.pattern: Pattern<'_>`.

   - `fn hair::Arm::top_pats_hack` is introduced as a temporary crutch in MIR building to avoid more changes.

- We change `hir::Arm.pats: HirVec<P<Pat>>` into `hir::Arm.pat: P<Pat>`.

   - The hacks in `rustc::hir::lowering` are removed since the representation hack is no longer necessary.

   - In some places, `fn hir::Arm::top_pats_hack` is introduced to leave some things as future work.

   - Misc changes: HIR pretty printing is adjusted to behave uniformly wrt. top/inner levels, rvalue promotion is adjusted, regionck, and dead_code is also.

   - Type checking is adjusted to uniformly handle or-patterns at top/inner levels.
      To make things compile, `p_0 | ... | p_n` is redefined as a "reference pattern" in [`fn is_non_ref_pat`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_typeck/check/struct.FnCtxt.html#method.is_non_ref_pat) for now. This is done so that reference types are not eagerly stripped from the `expected: Ty<'tcx>`.

    - Liveness is adjusted wrt. the `unused_variables` and `unused_assignments` lints to handle top/inner levels uniformly and the handling of `fn` parameters, `let` locals, and `match` arms are unified in this respect. This is not tested for now as exhaustiveness checks are reachable and will ICE.

    - In `check_match`, checking `@` and by-move bindings is adjusted. However, exhaustiveness checking is not adjusted the moment and is handled by @dlrobertson in #63688.

    - AST borrowck (`construct.rs`) is not adjusted as AST borrowck will be removed soon.

r? @matthewjasper
cc @dlrobertson @varkor @oli-obk
Centril added a commit to Centril/rust that referenced this pull request Sep 25, 2019
or-patterns: Push `PatKind/PatternKind::Or` at top level to HIR & HAIR

Following up on work in rust-lang#64111, rust-lang#63693, and rust-lang#61708, in this PR:

- We change `hair::Arm.patterns: Vec<Pattern<'_>>` into `hir::Arm.pattern: Pattern<'_>`.

   - `fn hair::Arm::top_pats_hack` is introduced as a temporary crutch in MIR building to avoid more changes.

- We change `hir::Arm.pats: HirVec<P<Pat>>` into `hir::Arm.pat: P<Pat>`.

   - The hacks in `rustc::hir::lowering` are removed since the representation hack is no longer necessary.

   - In some places, `fn hir::Arm::top_pats_hack` is introduced to leave some things as future work.

   - Misc changes: HIR pretty printing is adjusted to behave uniformly wrt. top/inner levels, rvalue promotion is adjusted, regionck, and dead_code is also.

   - Type checking is adjusted to uniformly handle or-patterns at top/inner levels.
      To make things compile, `p_0 | ... | p_n` is redefined as a "reference pattern" in [`fn is_non_ref_pat`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_typeck/check/struct.FnCtxt.html#method.is_non_ref_pat) for now. This is done so that reference types are not eagerly stripped from the `expected: Ty<'tcx>`.

    - Liveness is adjusted wrt. the `unused_variables` and `unused_assignments` lints to handle top/inner levels uniformly and the handling of `fn` parameters, `let` locals, and `match` arms are unified in this respect. This is not tested for now as exhaustiveness checks are reachable and will ICE.

    - In `check_match`, checking `@` and by-move bindings is adjusted. However, exhaustiveness checking is not adjusted the moment and is handled by @dlrobertson in rust-lang#63688.

    - AST borrowck (`construct.rs`) is not adjusted as AST borrowck will be removed soon.

r? @matthewjasper
cc @dlrobertson @varkor @oli-obk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-or_patterns `#![feature(or_patterns)]` 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.

9 participants