do not run MIR type checker twice #48061

nikomatsakis · 2018-02-07T22:27:59Z

The MIR type checker currently runs twice when NLL is enabled: once as a sanity check, and once "for real". No need for that.

rust-highfive · 2018-02-07T22:28:04Z

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

eddyb · 2018-02-07T22:28:55Z

@bors r+

bors · 2018-02-07T22:28:56Z

📌 Commit 27d615a has been approved by eddyb

kennytm · 2018-02-08T10:16:33Z

@bors r-

There are multiple UI failures in the CI, most of them looks like the errors being reordered.

[00:59:36] failures:
[00:59:36]     [ui] ui/nll/ty-outlives/projection-no-regions-closure.rs
[00:59:36]     [ui] ui/nll/ty-outlives/projection-one-region-closure.rs
[00:59:36]     [ui] ui/nll/ty-outlives/projection-one-region-trait-bound-closure.rs
[00:59:36]     [ui] ui/nll/ty-outlives/projection-one-region-trait-bound-static-closure.rs
[00:59:36]     [ui] ui/nll/ty-outlives/projection-two-region-trait-bound-closure.rs
[00:59:36]     [ui] ui/nll/ty-outlives/ty-param-closure-approximate-lower-bound.rs
[00:59:36]     [ui] ui/nll/ty-outlives/ty-param-closure-outlives-from-where-clause.rs

nikomatsakis · 2018-02-08T17:04:50Z

Argh. OK =)

shepmaster · 2018-02-17T15:36:57Z

Ping from triage, @nikomatsakis — will you be able to address the UI test failures sometime soon?

nikomatsakis · 2018-02-22T23:47:05Z

@bors r=eddyb

bors · 2018-02-22T23:47:05Z

📌 Commit b3fcbe2 has been approved by eddyb

kennytm · 2018-02-23T02:11:30Z

@bors r-

The test compile-fail/borrowck/two-phase-nonrecv-autoref.rs failed, there is no error when NLL is enabled (???).

[01:06:34] ---- [compile-fail] compile-fail/borrowck/two-phase-nonrecv-autoref.rs stdout ----
[01:06:34] 	
[01:06:34] error in revision `nll`: /checkout/src/test/compile-fail/borrowck/two-phase-nonrecv-autoref.rs:102: expected error not found: cannot move a value of type
[01:06:34] 
[01:06:34] error in revision `nll`: /checkout/src/test/compile-fail/borrowck/two-phase-nonrecv-autoref.rs:102: expected error not found: cannot move a value of type
[01:06:34] 
[01:06:34] error in revision `nll`: 0 unexpected errors found, 2 expected errors not found
[01:06:34] status: exit code: 101
[01:06:34] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/compile-fail/borrowck/two-phase-nonrecv-autoref.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/compile-fail" "--target=x86_64-unknown-linux-gnu" "--cfg" "nll" "--error-format" "json" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/compile-fail/borrowck/two-phase-nonrecv-autoref.stage2-x86_64-unknown-linux-gnu" "-Crpath" "-O" "-Zmiri" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-Z" "borrowck=mir" "-Z" "two-phase-borrows" "-Z" "nll" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/compile-fail/borrowck/two-phase-nonrecv-autoref.stage2-x86_64-unknown-linux-gnu.aux" "-A" "unused"
[01:06:34] not found errors (from test file): [
[01:06:34]     Error {
[01:06:34]         line_num: 102,
[01:06:34]         kind: Some(
[01:06:34]             Error
[01:06:34]         ),
[01:06:34]         msg: "cannot move a value of type"
[01:06:34]     },
[01:06:34]     Error {
[01:06:34]         line_num: 102,
[01:06:34]         kind: Some(
[01:06:34]             Error
[01:06:34]         ),
[01:06:34]         msg: "cannot move a value of type"
[01:06:34]     }
[01:06:34] ]
[01:06:34] 
[01:06:34] thread '[compile-fail] compile-fail/borrowck/two-phase-nonrecv-autoref.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:1253:13
[01:06:34] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:06:34] 
[01:06:34] 
[01:06:34] failures:
[01:06:34]     [compile-fail] compile-fail/borrowck/two-phase-nonrecv-autoref.rs

nikomatsakis · 2018-02-23T15:36:44Z

@kennytm hmm I don't have that test, I guess I have to rebase

nikomatsakis · 2018-02-23T15:39:53Z

Ah, I think that this is a case of not emitting duplicate errors (this is the vicinity of line 102):

    fn twice_ten_oo(f: Box<FnOnce(i32) -> i32>) {
        f(f(10));
        //[lxl]~^             ERROR cannot move a value of type
        //[lxl]~^^            ERROR cannot move a value of type
        //[lxl]~^^^           ERROR use of moved value: `*f`
        //[nll]~^^^^          ERROR cannot move a value of type
        //[nll]~^^^^^         ERROR cannot move a value of type
        //[nll]~^^^^^^        ERROR cannot move a value of type
        //[nll]~^^^^^^^       ERROR cannot move a value of type
        //[nll]~^^^^^^^^      ERROR use of moved value: `*f`
        //[g2p]~^^^^^^^^^     ERROR cannot move a value of type
        //[g2p]~^^^^^^^^^^    ERROR cannot move a value of type
        //[g2p]~^^^^^^^^^^^   ERROR cannot move a value of type
        //[g2p]~^^^^^^^^^^^^  ERROR cannot move a value of type
        //[g2p]~^^^^^^^^^^^^^ ERROR use of moved value: `*f`
    }

The type checker invokes the borrow checker for closures it finds, so removing the NLL type checker affects ordering of errors somewhat.

nikomatsakis · 2018-02-23T20:58:16Z

@bors r=eddyb

bors · 2018-02-23T20:58:17Z

📌 Commit bcd9968 has been approved by eddyb

…eck-twice, r=eddyb do not run MIR type checker twice The MIR type checker currently runs twice when NLL is enabled: once as a sanity check, and once "for real". No need for that.

Rollup of 15 pull requests - Successful merges: #47987, #48056, #48061, #48084, #48143, #48185, #48206, #48208, #48232, #48246, #48258, #48317, #48353, #48356, #48402 - Failed merges:

Manishearth · 2018-02-24T18:37:06Z

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

@@ -1585,6 +1585,12 @@ impl MirPass for TypeckMir {
        let id = tcx.hir.as_local_node_id(def_id).unwrap();
        debug!("run_pass: {:?}", def_id);

+        // When NLL is enabled, the borrow checker runs the typeck
+        // itself, so we don't need this MIR pass anymore.
+        if tcx.sess.nll() {


[00:12:34] error[E0599]: no method named `nll` found for type `&rustc::session::Session` in the current scope [00:12:34] --> librustc_mir/borrow_check/nll/type_check/mod.rs:1590:21 [00:12:34] | [00:12:34] 1590 | if tcx.sess.nll() { [00:12:34] | ^^^

in the rollup

It's removed by f787bdd .

Actually, that PR seems to be more rollup-breaking, I'll put this back in, sorry about that

@nrc

…eck-twice, r=eddyb Fixes rust-lang#47311. r? @nrc

Rollup of 15 pull requests - Successful merges: #47987, #48056, #48061, #48084, #48143, #48185, #48206, #48208, #48232, #48246, #48258, #48317, #48353, #48356, #48402 - Failed merges:

rust-highfive assigned eddyb Feb 7, 2018

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Feb 7, 2018

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 8, 2018

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 Feb 22, 2018

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 23, 2018

do not run MIR type checker twice

380e388

nikomatsakis force-pushed the nll-do-not-run-mir-typeck-twice branch from b3fcbe2 to 9084f1d Compare February 23, 2018 15:37

nikomatsakis added 2 commits February 23, 2018 10:40

update tests and reference files

2370b60

The type checker invokes the borrow checker for closures it finds, so removing the NLL type checker affects ordering of errors somewhat.

explain why we don't need to run type-checker when NLL is enabled

bcd9968

nikomatsakis force-pushed the nll-do-not-run-mir-typeck-twice branch from 9084f1d to bcd9968 Compare February 23, 2018 15:46

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 Feb 23, 2018

Manishearth mentioned this pull request Feb 24, 2018

Rollup of 15 pull requests #48510

Merged

Manishearth reviewed Feb 24, 2018

View reviewed changes

Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 24, 2018

Rollup merge of rust-lang#48061 - nikomatsakis:nll-do-not-run-mir-typ…

182f882

…eck-twice, r=eddyb Fixes rust-lang#47311. r? @nrc

bors merged commit bcd9968 into rust-lang:master Feb 24, 2018

do not run MIR type checker twice #48061

do not run MIR type checker twice #48061

Uh oh!

Conversation

nikomatsakis commented Feb 7, 2018

Uh oh!

rust-highfive commented Feb 7, 2018

Uh oh!

eddyb commented Feb 7, 2018

Uh oh!

bors commented Feb 7, 2018

Uh oh!

kennytm commented Feb 8, 2018

Uh oh!

nikomatsakis commented Feb 8, 2018

Uh oh!

shepmaster commented Feb 17, 2018

Uh oh!

nikomatsakis commented Feb 22, 2018

Uh oh!

bors commented Feb 22, 2018

Uh oh!

kennytm commented Feb 23, 2018

Uh oh!

nikomatsakis commented Feb 23, 2018

Uh oh!

nikomatsakis commented Feb 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nikomatsakis commented Feb 23, 2018

Uh oh!

bors commented Feb 23, 2018

Uh oh!

Manishearth Feb 24, 2018

Choose a reason for hiding this comment

Uh oh!

Manishearth Feb 24, 2018

Choose a reason for hiding this comment

Uh oh!

Manishearth Feb 24, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nikomatsakis commented Feb 23, 2018 •

edited

Loading