Initial implementation of or-patterns by dlrobertson · Pull Request #61708 · rust-lang/rust (original) (raw)

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

alexreg, Centril, ljedrz, mark-i-m, jplatte, Aaron1011, taiki-e, ebkalderon, and dnrusakov reacted with thumbs up emoji phansch, Centril, and BatmanAoD reacted with hooray emoji

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

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"

This comment has been minimized.

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.

// 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> {

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?

This comment has been minimized.

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

Initial implementation of parsing or-patterns e.g., Some(Foo | Bar). This is a partial implementation of RFC 2535.

📌 Commit 1870537 has been approved by centril

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 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 added a commit to rust-lang/rust-clippy that referenced this pull request

Aug 18, 2019

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.

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:
```
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:
```
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:
```
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:
```
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 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 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 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