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

Centril · 2019-09-03T02:49:59Z

Following up on work in #63693 and #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 #54883
cc @dlrobertson @matthewjasper
r? @petrochenkov

src/librustc/hir/lowering/expr.rs

src/libsyntax/ast.rs

src/librustc_resolve/late.rs

src/libsyntax/ast.rs

src/libsyntax/visit.rs

Centril · 2019-09-04T00:46:37Z

Addressed the comments, hopefully. :)

src/librustc_resolve/late.rs

petrochenkov · 2019-09-04T23:04:01Z

r=me with the remaining comment addressed

Centril · 2019-09-04T23:45:37Z

@bors r=petrochenkov

bors · 2019-09-04T23:45:39Z

📌 Commit 359c3c0865c8a929925972675eb58200cec07133 has been approved by petrochenkov

Centril · 2019-09-05T01:47:18Z

Tested locally with ./x.py test src/liballoc --stage 1 before and after rebase. Cannot reproduce the failure. Let's see if rebase changes anything in CI.

@oli-obk

unused_parens: account for or-patterns and `&(mut x)` Fixes rust-lang#55342. Fixes rust-lang#64106. cc rust-lang#54883 cc rust-lang#64111 r? @oli-obk

@oli-obk

unused_parens: account for or-patterns and `&(mut x)` Fixes rust-lang#55342. Fixes rust-lang#64106. cc rust-lang#54883 cc rust-lang#64111 r? @oli-obk

Centril · 2019-09-05T07:19:45Z

Rebased & fixed the fallout from #64128 in 16ba502.

@bors r=petrochenkov

bors · 2019-09-05T07:19:46Z

📌 Commit 16ba502 has been approved by petrochenkov

@dlrobertson

…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

@dlrobertson

…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

@ghost

Rollup of 5 pull requests Successful merges: - #63676 (Use wasi crate for Core API) - #64094 (Improve searching in rustdoc and add tests) - #64111 (or-patterns: Uniformly use `PatKind::Or` in AST & Fix/Cleanup resolve) - #64156 (Assume non-git LLVM is fresh if the stamp file exists) - #64175 (Fix invalid span generation when it should be div) Failed merges: - #63806 (Upgrade rand to 0.7) r? @ghost

@dlrobertson

…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

@dlrobertson

…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

@ghost

Rollup of 10 pull requests Successful merges: - #63676 (Use wasi crate for Core API) - #64094 (Improve searching in rustdoc and add tests) - #64111 (or-patterns: Uniformly use `PatKind::Or` in AST & Fix/Cleanup resolve) - #64156 (Assume non-git LLVM is fresh if the stamp file exists) - #64161 (Point at variant on pattern field count mismatch) - #64174 (Add missing code examples on Iterator trait) - #64175 (Fix invalid span generation when it should be div) - #64186 (std: Improve downstream codegen in `Command::env`) - #64190 (fill metadata in rustc_lexer's Cargo.toml) - #64198 (Add Fuchsia to actually_monotonic) Failed merges: r? @ghost

Rustup to rust-lang/rust#64111 changelog: none

@dlrobertson

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

@dlrobertson

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

@dlrobertson

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

@petrochenkov

…etrochenkov resolve: more declarative `fresh_binding` Following up on rust-lang#64111, this PR redefines `fresh_binding` wrt. `already_bound_and` and `already_bound_or` in a more declarative and simplified fashion. cc rust-lang#54883 r? @petrochenkov

rust-highfive assigned petrochenkov Sep 3, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 3, 2019

Centril added the F-or_patterns `#![feature(or_patterns)]` label Sep 3, 2019

This comment has been minimized.

Sign in to view

Centril commented Sep 3, 2019

View reviewed changes

src/librustc/hir/lowering/expr.rs Show resolved Hide resolved

Centril mentioned this pull request Sep 3, 2019

unused_parens: account for or-patterns and &(mut x) #64128

Merged

petrochenkov reviewed Sep 3, 2019

View reviewed changes

petrochenkov 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 Sep 3, 2019

Centril added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 4, 2019

Centril force-pushed the ast-only-patkind-or branch from aad2579 to 97cce77 Compare September 4, 2019 01:02

petrochenkov reviewed Sep 4, 2019

View reviewed changes

src/librustc_resolve/late.rs Outdated Show resolved Hide resolved

petrochenkov 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 Sep 4, 2019

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 4, 2019

This comment has been minimized.

Sign in to view

Centril force-pushed the ast-only-patkind-or branch from 359c3c0 to bc023ec Compare September 5, 2019 01:46

Centril added 4 commits September 5, 2019 08:33

resolve: extract resolve_params.

af06bfb

resolve: cleanup using with_rib, etc.

f8835ee

resolve: simplify resolve_arm.

6d537d4

resolve: extract try_resolve_as_non_binding.

219ddde

Centril force-pushed the ast-only-patkind-or branch from bc023ec to 16ba502 Compare September 5, 2019 07:17

Centril mentioned this pull request Sep 5, 2019

Rollup of 7 pull requests #64181

Closed

Centril mentioned this pull request Sep 5, 2019

Rollup of 5 pull requests #64182

Closed

Mark-Simulacrum mentioned this pull request Sep 6, 2019

Rollup of 13 pull requests #64200

Closed

Centril mentioned this pull request Sep 6, 2019

Rollup of 10 pull requests #64209

Merged

bors merged commit 16ba502 into rust-lang:master Sep 6, 2019

Centril deleted the ast-only-patkind-or branch September 6, 2019 11:41

This was referenced Sep 6, 2019

Tracking issue for RFC 2535, 2530, 2175, "Or patterns, i.e Foo(Bar(x) | Baz(x))" #54883

Closed

clippy-driver no longer builds after rust-lang/rust#64209 #64215

Closed

WIP: Initial implementation of or-pattern handling in MIR #63688

Closed

mati865 added a commit to mati865/rust-clippy that referenced this pull request Sep 6, 2019

Rustup to rust-lang/rust#64111

5afa216

bors added a commit to rust-lang/rust-clippy that referenced this pull request Sep 6, 2019

Auto merge of #4512 - mati865:rustup, r=phansch

804f8e6

Rustup to rust-lang/rust#64111 changelog: none

Centril mentioned this pull request Sep 16, 2019

or-patterns: Push PatKind/PatternKind::Or at top level to HIR & HAIR #64508

Merged

Centril mentioned this pull request Nov 22, 2019

resolve: more declarative fresh_binding #66639

Merged

Centril mentioned this pull request Feb 5, 2020

Const param as PatKind::Path interpreted as a binding instead of being rejected #68853

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Centril commented Sep 3, 2019

This comment has been minimized.

Centril commented Sep 4, 2019

petrochenkov commented Sep 4, 2019

Centril commented Sep 4, 2019

bors commented Sep 4, 2019

This comment has been minimized.

Centril commented Sep 5, 2019

Centril commented Sep 5, 2019

bors commented Sep 5, 2019

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

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

Conversation

Centril commented Sep 3, 2019

This comment has been minimized.

Centril commented Sep 4, 2019

petrochenkov commented Sep 4, 2019

Centril commented Sep 4, 2019

bors commented Sep 4, 2019

This comment has been minimized.

Centril commented Sep 5, 2019

Centril commented Sep 5, 2019

bors commented Sep 5, 2019

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

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