NLL: User type annotations refactor, associated constant patterns and ref bindings. by davidtwco · Pull Request #55937 · rust-lang/rust

davidtwco · 2018-11-13T21:59:27Z

Fixes #55511 and Fixes #55401. Contributes to #54943.

This PR performs a large refactoring on user type annotations, checks user type annotations for associated constants in patterns and that user type annotations for ref bindings are respected.

r? @nikomatsakis

src/librustc/mir/mod.rs

src/librustc/mir/visit.rs

src/librustc_mir/borrow_check/nll/type_check/mod.rs

nikomatsakis

Looks great! Left one nit.

r=me once tests passed and comment is added

src/librustc_mir/borrow_check/nll/type_check/mod.rs

davidtwco · 2018-11-22T23:12:25Z

Pushed the first version of this that does what it says on the tin. Will need a fresh review as a lot has changed.

davidtwco · 2018-11-24T12:01:42Z

Added a fix for #55511 to this PR. Still seeing an ICE on three tests for the original fix, worked out that is due to this addition:

https://github.com/rust-lang/rust/blob/bde1f0d98bd1927baf5f53bd5c0d54d1382f1c45/src/librustc_traits/type_op.rs#L151-L154

But without that addition, the test change in src/test/ui/regions/regions-free-region-ordering-caller1.rs doesn't happen. Still working on it.

davidtwco · 2018-11-25T14:27:02Z

Added a fix for #55401 too. Yet to look into the failures for the original PR issue.

nikomatsakis · 2019-01-02T19:48:44Z

I filed #57280, as this seems like a bug (and a regression).

Work around rust-lang/rust#55937

Breakage was due to rust-lang#55937, which seemed to introduce a regression (tracked over at rust-lang#57280).

Changes: ```` Update Clippy Move TestFailures when collecting failures Update languageserver-types to 0.51.1 update clippy hash and rustc_tools_util and use rustc_tools_util from crates.io Work around rust-lang#55937 Update Clippy... again Update Clippy Update clippy ````

@oli-obk

submodules: update clippy and rls Fixes clippy toolstate Changes: ```` Update to latest compiletest-rs release add testcase for #3462 deps: bump rustc_tools_util version from 0.1.0 to 0.1.1 just in case... rustc_tool_utils: fix failure to create proper non-repo version string when used in crates on crates.io, bump version UI test cleanup: Extract ifs_same_cond tests UI test cleanup: Extract for_kv_map lint tests Fix test for #57250 Limit infinite_iter collect() check to known types Some improvements to util documentation Use hashset for name blacklist Reformat random_state tests Use node_id_to_type_opt instead of node_it_to_type in random_state Check pattern equality while checking declaration equality random_state lint Use an FxHashSet for valid idents in documentation lint Fix suggestion for unnecessary_ref lint Update CONTRIBUTING.md for rustfix tests Update .fixed files via update-references.sh Run rustfix on first UI test Use WIP branch for compiletest_rs ```` Also updates RLS and removes the patching of rustc_tool_utils from cargo.toml RLS changes: ```` update clippy hash and rustc_tools_util and use rustc_tools_util from crates.io Work around #55937 Update Clippy... again Update Clippy Update clippy ```` r? @oli-obk

Changes: ```` Update Clippy Move TestFailures when collecting failures Update languageserver-types to 0.51.1 update clippy hash and rustc_tools_util and use rustc_tools_util from crates.io Work around rust-lang#55937 Update Clippy... again Update Clippy Update clippy ````

@oli-obk

submodules: update clippy and rls Fixes clippy toolstate Changes: ```` Update to latest compiletest-rs release add testcase for #3462 deps: bump rustc_tools_util version from 0.1.0 to 0.1.1 just in case... rustc_tool_utils: fix failure to create proper non-repo version string when used in crates on crates.io, bump version UI test cleanup: Extract ifs_same_cond tests UI test cleanup: Extract for_kv_map lint tests Fix test for #57250 Limit infinite_iter collect() check to known types Some improvements to util documentation Use hashset for name blacklist Reformat random_state tests Use node_id_to_type_opt instead of node_it_to_type in random_state Check pattern equality while checking declaration equality random_state lint Use an FxHashSet for valid idents in documentation lint Fix suggestion for unnecessary_ref lint Update CONTRIBUTING.md for rustfix tests Update .fixed files via update-references.sh Run rustfix on first UI test Use WIP branch for compiletest_rs ```` Also updates RLS and removes the patching of rustc_tool_utils from cargo.toml RLS changes: ```` update clippy hash and rustc_tools_util and use rustc_tools_util from crates.io Work around #55937 Update Clippy... again Update Clippy Update clippy ```` r? @oli-obk

This commit fixes a bug introduced by rust-lang#55937 which started checking user type annotations for associated type patterns. Where lowering a associated constant expression would previously return a `PatternKind::Constant`, it now returns a `PatternKind::AscribeUserType` with a `PatternKind::Constant` inside, this commit unwraps that to access the constant pattern inside and behaves as before.

@pnkfelix

Lower constant patterns with ascribed types. Fixes #57960. This PR fixes a bug introduced by #55937 which started checking user type annotations for associated type patterns. Where lowering a associated constant expression would previously return a `PatternKind::Constant`, it now returns a `PatternKind::AscribeUserType` with a `PatternKind::Constant` inside, this PR unwraps that to access the constant pattern inside and behaves as before. r? @pnkfelix

This commit fixes a bug introduced by rust-lang#55937 which started checking user type annotations for associated type patterns. Where lowering a associated constant expression would previously return a `PatternKind::Constant`, it now returns a `PatternKind::AscribeUserType` with a `PatternKind::Constant` inside, this commit unwraps that to access the constant pattern inside and behaves as before.

mir_build: Avoid some useless work when visiting "primary" bindings While looking over `visit_primary_bindings`, I noticed that it does a bunch of extra work to build up a collection of “user-type projections”, even though 2/3 of its call sites don't even use them. Those callers can get the same result via `thir::Pat::walk_always`. (And it turns out that doing so also avoids creating some redundant user-type entries in MIR for some binding constructs.) I also noticed that even when the user-type projections *are* used, the process of building them ends up eagerly cloning some nested vectors at every recursion step, even in cases where they won't be used because the current subpattern has no bindings. To avoid this, the visit method now assembles a linked list on the stack containing the information that *would* be needed to create projections, and only creates the concrete projections as needed when a primary binding is encountered. Some relevant prior PRs: - rust-lang#55274 - rust-lang@0bfe184 in rust-lang#55937

mir_build: Avoid some useless work when visiting "primary" bindings While looking over `visit_primary_bindings`, I noticed that it does a bunch of extra work to build up a collection of “user-type projections”, even though 2/3 of its call sites don't even use them. Those callers can get the same result via `thir::Pat::walk_always`. (And it turns out that doing so also avoids creating some redundant user-type entries in MIR for some binding constructs.) I also noticed that even when the user-type projections *are* used, the process of building them ends up eagerly cloning some nested vectors at every recursion step, even in cases where they won't be used because the current subpattern has no bindings. To avoid this, the visit method now assembles a linked list on the stack containing the information that *would* be needed to create projections, and only creates the concrete projections as needed when a primary binding is encountered. Some relevant prior PRs: - rust-lang#55274 - rust-lang@0bfe184 in rust-lang#55937 --- There should be no user-visible change in compiler output.

Rollup merge of rust-lang#137465 - Zalathar:visit-primary, r=oli-obk mir_build: Avoid some useless work when visiting "primary" bindings While looking over `visit_primary_bindings`, I noticed that it does a bunch of extra work to build up a collection of “user-type projections”, even though 2/3 of its call sites don't even use them. Those callers can get the same result via `thir::Pat::walk_always`. (And it turns out that doing so also avoids creating some redundant user-type entries in MIR for some binding constructs.) I also noticed that even when the user-type projections *are* used, the process of building them ends up eagerly cloning some nested vectors at every recursion step, even in cases where they won't be used because the current subpattern has no bindings. To avoid this, the visit method now assembles a linked list on the stack containing the information that *would* be needed to create projections, and only creates the concrete projections as needed when a primary binding is encountered. Some relevant prior PRs: - rust-lang#55274 - rust-lang@0bfe184 in rust-lang#55937 --- There should be no user-visible change in compiler output.

rust-highfive assigned nikomatsakis Nov 13, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 13, 2018