NLL feature complete (adds `feature(nll)`)! #46862

nikomatsakis · 2017-12-20T01:36:18Z

This is the final PR for the nll-master branch; it brings over all remaining content.

The contents of the branch include:

track causal information and use it to report extended errors
handle impl Trait in NLL code
improve printing of outlives errors
add #![feature(nll)] and some more sample tests

The commits should for the most part build independently.

bors · 2017-12-20T06:40:05Z

☔ The latest upstream changes (presumably #46733) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2017-12-20T17:37:08Z

☔ The latest upstream changes (presumably #46874) made this pull request unmergeable. Please resolve the merge conflicts.

We don't need to know the vector of region bounds; we only care if there were any region bounds at all.

The result is not especially illuminating, but that's ok.

The type isn't a good idea because we want to treat constants uniformly, regardless of what type of value they produce.

No functional change.

No funtional change.

No functional change.

We now add the suitable `impl Trait` constraints.

arielb1 · 2017-12-20T19:27:09Z

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

@@ -1075,3 +1114,18 @@ impl<'gcx, 'tcx> ClosureRegionRequirementsExt<'gcx, 'tcx> for ClosureRegionRequi
        })
    }
 }
+
+trait CauseExt {


~~Why an extension trait?~~ We have to wait until the next snapshot for #![feature(arbitrary_self_types)]

Old habits die hard =)

Closes rust-lang#45988

In particular, -Znll might as well imply -Zborrowck=mir by default, just like `#![feature(nll)]` does. Also, if NLL is in use, no reason to emit end regions. The NLL pass just strips them out anyway.

Also, keep reporting AST-based region errors that are not occuring in a fn body.

The "match exact bits of CFG" approach was fragile and uninformative.

arielb1 · 2017-12-20T19:56:17Z

src/librustc/infer/anon_types/mod.rs

+                    // `parent_def_id`. During the first phase of type-check, this
+                    // is true, but during NLL type-check, we sometimes encounter
+                    // `impl Trait` types in e.g. inferred closure signatures that
+                    // are not 'local' to the current function and hence which


This comment is confusing.

If we had typeof or impl Trait associated types, this could just as well happen within the signature of a function. The thing is, we want to make a distinction between "syntactic" impl Trait types that are "bound to our function" and "free" impl Trait types that are not.

Aka if we have this:

mod some_mod { type Foo = impl Iterator; // not sure about the exact syntax } mod my_mod { fn foo(x: ::some_mod::Foo) -> impl Iterator { x } }

Then in foo, ::some_mod::Foo is a "free" TyAnon while the return type is a "bound" TyAnon. Luckily for us, uses of TyAnon can't be recursive (for now?), so if a TyAnon has our own DefId then it is definitely bound to our function.

I'm not sure this is the best way to express this, but that's how things are set up for now and we are not going to refactor the world for NLL.

In the case you give, we are still returning an impl Iterator, and it would still be local to foo() -- that is, when this function executes, we would have effectively desugared to:

mod my_mod { abstract type FooReturn: Iterator; fn foo(x: ::some_mod::Foo) -> FooReturn { x }

and hence we would be "instantiating" FooReturn (whose value would then get inferred to a TyAnon representing some_mod::Foo).

In fact, this is basically exactly the case that this code is designed to handle in NLL: you can construct a very similar scenario with closures and return type inference today.

I'm not sure this is the best way to express this, but that's how things are set up for now and we are not going to refactor the world for NLL.

I'm not really sure what this quite means. It doesn't seem like NLL requires any particular refactoring of the world here -- it's just that NLL exposes us to more complex cases than we had to handle before, but it's not like it's a problem to handle them. I can try to reword the comment, anyhow.

Pushed a second attempt! =)

arielb1 · 2017-12-20T19:57:42Z

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

+                None
+            });
+
+        // Finally


truncated comment?

arielb1 · 2017-12-20T22:40:02Z

src/librustc_mir/util/liveness.rs

+        // `mir-opt/nll/liveness-call-subtlety.rs`. To do things
+        // properly, we would apply the def in call only to the
+        // input from the success path and not the unwind
+        // path. -nmatsakis


call leaves garbage in the destination on the unwind path, so the old value of the variable is still irrelevant - i.e. it's still a def.

theotherphil · 2017-12-20T22:43:30Z

src/librustc/infer/anon_types/mod.rs

+/// appear in the return type).
+#[derive(Copy, Clone, Debug)]
+pub struct AnonTypeDecl<'tcx> {
+    /// The substitutions that we apply to the abstract that that this


Nit: "that that"

arielb1 · 2017-12-20T23:03:24Z

@bors r+

You could try to fix the comment if you want

bors · 2017-12-20T23:03:25Z

📌 Commit d925f4d has been approved by arielb1

arielb1 · 2017-12-20T23:11:07Z

@bors p=1

bors · 2017-12-20T23:26:27Z

⌛ Testing commit d925f4d with merge fdfb007...

@pnkfelix

NLL feature complete (adds `feature(nll)`)! This is the final PR for the nll-master branch; it brings over all remaining content. The contents of the branch include: - track causal information and use it to report extended errors - handle `impl Trait` in NLL code - improve printing of outlives errors - add `#![feature(nll)]` and some more sample tests The commits should for the most part build independently. r? @pnkfelix (and/or @arielb1)

bors · 2017-12-21T02:05:17Z

☀️ Test successful - status-appveyor, status-travis
Approved by: arielb1
Pushing fdfb007 to master...

…matsakis Remove migrate borrowck mode Closes rust-lang#58781 Closes rust-lang#43234 # Stabilization proposal This PR proposes the stabilization of `#![feature(nll)]` and the removal of `-Z borrowck`. Current borrow checking behavior of item bodies is currently done by first infering regions *lexically* and reporting any errors during HIR type checking. If there *are* any errors, then MIR borrowck (NLL) never occurs. If there *aren't* any errors, then MIR borrowck happens and any errors there would be reported. This PR removes the lexical region check of item bodies entirely and only uses MIR borrowck. Because MIR borrowck could never *not* be run for a compiled program, this should not break any programs. It does, however, change diagnostics significantly and allows a slightly larger set of programs to compile. Tracking issue: rust-lang#43234 RFC: https://github.com/rust-lang/rfcs/blob/master/text/2094-nll.md Version: 1.63 (2022-06-30 => beta, 2022-08-11 => stable). ## Motivation Over time, the Rust borrow checker has become "smarter" and thus allowed more programs to compile. There have been three different implementations: AST borrowck, MIR borrowck, and polonius (well, in progress). Additionally, there is the "lexical region resolver", which (roughly) solves the constraints generated through HIR typeck. It is not a full borrow checker, but does emit some errors. The AST borrowck was the original implementation of the borrow checker and was part of the initially stabilized Rust 1.0. In mid 2017, work began to implement the current MIR borrow checker and that effort ompleted by the end of 2017, for the most part. During 2018, efforts were made to migrate away from the AST borrow checker to the MIR borrow checker - eventually culminating into "migrate" mode - where HIR typeck with lexical region resolving following by MIR borrow checking - being active by default in the 2018 edition. In early 2019, migrate mode was turned on by default in the 2015 edition as well, but with MIR borrowck errors emitted as warnings. By late 2019, these warnings were upgraded to full errors. This was followed by the complete removal of the AST borrow checker. In the period since, various errors emitted by the MIR borrow checker have been improved to the point that they are mostly the same or better than those emitted by the lexical region resolver. While there do remain some degradations in errors (tracked under the [NLL-diagnostics tag](https://github.com/rust-lang/rust/issues?q=is%3Aopen+is%3Aissue+label%3ANLL-diagnostics), those are sufficiently small and rare enough that increased flexibility of MIR borrow check-only is now a worthwhile tradeoff. ## What is stabilized As said previously, this does not fundamentally change the landscape of accepted programs. However, there are a [few](https://github.com/rust-lang/rust/issues?q=is%3Aopen+is%3Aissue+label%3ANLL-fixed-by-NLL) cases where programs can compile under `feature(nll)`, but not otherwise. There are two notable patterns that are "fixed" by this stabilization. First, the `scoped_threads` feature, which is a continutation of a pre-1.0 API, can sometimes emit a [weird lifetime error](rust-lang#95527) without NLL. Second, actually seen in the standard library. In the `Extend` impl for `HashMap`, there is an implied bound of `K: 'a` that is available with NLL on but not without - this is utilized in the impl. As mentioned before, there are a large number of diagnostic differences. Most of them are better, but some are worse. None are serious or happen often enough to need to block this PR. The biggest change is the loss of error code for a number of lifetime errors in favor of more general "lifetime may not live long enough" error. While this may *seem* bad, the former error codes were just attempts to somewhat-arbitrarily bin together lifetime errors of the same type; however, on paper, they end up being roughly the same with roughly the same kinds of solutions. ## What isn't stabilized This PR does not completely remove the lexical region resolver. In the future, it may be possible to remove that (while still keeping HIR typeck) or to remove it together with HIR typeck. ## Tests Many test outputs get updated by this PR. However, there are number of tests specifically geared towards NLL under `src/test/ui/nll` ## History * On 2017-07-14, [tracking issue opened](rust-lang#43234) * On 2017-07-20, [initial empty MIR pass added](rust-lang#43271) * On 2017-08-29, [RFC opened](rust-lang/rfcs#2094) * On 2017-11-16, [Integrate MIR type-checker with NLL](rust-lang#45825) * On 2017-12-20, [NLL feature complete](rust-lang#46862) * On 2018-07-07, [Don't run AST borrowck on mir mode](rust-lang#52083) * On 2018-07-27, [Add migrate mode](rust-lang#52681) * On 2019-04-22, [Enable migrate mode on 2015 edition](rust-lang#59114) * On 2019-08-26, [Don't downgrade errors on 2015 edition](rust-lang#64221) * On 2019-08-27, [Remove AST borrowck](rust-lang#64790)

Remove migrate borrowck mode Closes #58781 Closes #43234 # Stabilization proposal This PR proposes the stabilization of `#![feature(nll)]` and the removal of `-Z borrowck`. Current borrow checking behavior of item bodies is currently done by first infering regions *lexically* and reporting any errors during HIR type checking. If there *are* any errors, then MIR borrowck (NLL) never occurs. If there *aren't* any errors, then MIR borrowck happens and any errors there would be reported. This PR removes the lexical region check of item bodies entirely and only uses MIR borrowck. Because MIR borrowck could never *not* be run for a compiled program, this should not break any programs. It does, however, change diagnostics significantly and allows a slightly larger set of programs to compile. Tracking issue: #43234 RFC: https://github.com/rust-lang/rfcs/blob/master/text/2094-nll.md Version: 1.63 (2022-06-30 => beta, 2022-08-11 => stable). ## Motivation Over time, the Rust borrow checker has become "smarter" and thus allowed more programs to compile. There have been three different implementations: AST borrowck, MIR borrowck, and polonius (well, in progress). Additionally, there is the "lexical region resolver", which (roughly) solves the constraints generated through HIR typeck. It is not a full borrow checker, but does emit some errors. The AST borrowck was the original implementation of the borrow checker and was part of the initially stabilized Rust 1.0. In mid 2017, work began to implement the current MIR borrow checker and that effort ompleted by the end of 2017, for the most part. During 2018, efforts were made to migrate away from the AST borrow checker to the MIR borrow checker - eventually culminating into "migrate" mode - where HIR typeck with lexical region resolving following by MIR borrow checking - being active by default in the 2018 edition. In early 2019, migrate mode was turned on by default in the 2015 edition as well, but with MIR borrowck errors emitted as warnings. By late 2019, these warnings were upgraded to full errors. This was followed by the complete removal of the AST borrow checker. In the period since, various errors emitted by the MIR borrow checker have been improved to the point that they are mostly the same or better than those emitted by the lexical region resolver. While there do remain some degradations in errors (tracked under the [NLL-diagnostics tag](https://github.com/rust-lang/rust/issues?q=is%3Aopen+is%3Aissue+label%3ANLL-diagnostics), those are sufficiently small and rare enough that increased flexibility of MIR borrow check-only is now a worthwhile tradeoff. ## What is stabilized As said previously, this does not fundamentally change the landscape of accepted programs. However, there are a [few](https://github.com/rust-lang/rust/issues?q=is%3Aopen+is%3Aissue+label%3ANLL-fixed-by-NLL) cases where programs can compile under `feature(nll)`, but not otherwise. There are two notable patterns that are "fixed" by this stabilization. First, the `scoped_threads` feature, which is a continutation of a pre-1.0 API, can sometimes emit a [weird lifetime error](rust-lang/rust#95527) without NLL. Second, actually seen in the standard library. In the `Extend` impl for `HashMap`, there is an implied bound of `K: 'a` that is available with NLL on but not without - this is utilized in the impl. As mentioned before, there are a large number of diagnostic differences. Most of them are better, but some are worse. None are serious or happen often enough to need to block this PR. The biggest change is the loss of error code for a number of lifetime errors in favor of more general "lifetime may not live long enough" error. While this may *seem* bad, the former error codes were just attempts to somewhat-arbitrarily bin together lifetime errors of the same type; however, on paper, they end up being roughly the same with roughly the same kinds of solutions. ## What isn't stabilized This PR does not completely remove the lexical region resolver. In the future, it may be possible to remove that (while still keeping HIR typeck) or to remove it together with HIR typeck. ## Tests Many test outputs get updated by this PR. However, there are number of tests specifically geared towards NLL under `src/test/ui/nll` ## History * On 2017-07-14, [tracking issue opened](rust-lang/rust#43234) * On 2017-07-20, [initial empty MIR pass added](rust-lang/rust#43271) * On 2017-08-29, [RFC opened](rust-lang/rfcs#2094) * On 2017-11-16, [Integrate MIR type-checker with NLL](rust-lang/rust#45825) * On 2017-12-20, [NLL feature complete](rust-lang/rust#46862) * On 2018-07-07, [Don't run AST borrowck on mir mode](rust-lang/rust#52083) * On 2018-07-27, [Add migrate mode](rust-lang/rust#52681) * On 2019-04-22, [Enable migrate mode on 2015 edition](rust-lang/rust#59114) * On 2019-08-26, [Don't downgrade errors on 2015 edition](rust-lang/rust#64221) * On 2019-08-27, [Remove AST borrowck](rust-lang/rust#64790)

rust-highfive assigned pnkfelix Dec 20, 2017

nikomatsakis mentioned this pull request Dec 20, 2017

ICE with NLL #46628

Closed

nikomatsakis force-pushed the nll-master branch from 1ce4a8d to dbb21b3 Compare December 20, 2017 09:14

kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 20, 2017

nikomatsakis changed the title ~~Nll 6~~ Nll 6 -- feature(nll)! Dec 20, 2017

nikomatsakis and others added 15 commits December 20, 2017 14:04

simplify AnonTypeDecl in the impl trait code

f3335c6

We don't need to know the vector of region bounds; we only care if there were any region bounds at all.

region_infer/values.rs: rustfmt

f6741d0

Add tracking of causes for nll

e447b54

rustfmt: borrow_check/mod.rs

39b0e49

dump out causal information for "free region" errors

594c386

The result is not especially illuminating, but that's ok.

use Rc to store nonlexical_regioncx in Borrows

741ef41

integrate -Znll-dump-cause into borrowck

0e64a75

get the DefiningTy from the body_owner_kind not type

fe89f4b

The type isn't a good idea because we want to treat constants uniformly, regardless of what type of value they produce.

propagate region_bound_pairs into MIR type-check

4a967c9

extract instantiate_anon_types to the InferCtxt

e96f4be

No functional change.

extract constrain_anon_types to the InferCtxt

8e64ba8

No funtional change.

extract the writeback code for anon types into InferCtxt

7f50e7c

No functional change.

pass UniversalRegions to MIR type-checker instead of fields

a66c651

No functional change.

extract input_output code into its own module

da63aaa

No functional change.

connect NLL type checker to the impl trait code

93afb1a

We now add the suitable `impl Trait` constraints.

nikomatsakis mentioned this pull request Dec 20, 2017

Epoch.Next Tracking Issue #46889

Closed

arielb1 reviewed Dec 20, 2017

View reviewed changes

spastorino and others added 6 commits December 20, 2017 14:38

Move MirVisitable to visit.rs

58b0506

Move categorize logic out of visit_local function

6d2987c

Add three point error handling to borrowck

3a185a5

Closes rust-lang#45988

only dump causes if we have nothing better

e28d03f

move nice-region-error reporting into its own module

4089d14

make util fns private to nice_region_error

93498e0

nikomatsakis added 4 commits December 20, 2017 14:38

document and tweak the nll, use_mir, etc helpers

cfa4ffa

In particular, -Znll might as well imply -Zborrowck=mir by default, just like `#![feature(nll)]` does. Also, if NLL is in use, no reason to emit end regions. The NLL pass just strips them out anyway.

when using feature(nll), don't warn about AST-based region errors

80c510e

Also, keep reporting AST-based region errors that are not occuring in a fn body.

add some run-pass tests for NLL showing that things work as expected

cba8256

convert region-liveness-drop{-,-no-}may-dangle.rs into ui tests

3f490ca

The "match exact bits of CFG" approach was fragile and uninformative.

nikomatsakis force-pushed the nll-master branch from 4dcf1c9 to 3f490ca Compare December 20, 2017 19:38

arielb1 reviewed Dec 20, 2017

View reviewed changes

nikomatsakis changed the title ~~Nll 6 -- feature(nll)!~~ NLL feature complete (adds feature(nll))! Dec 20, 2017

nikomatsakis added 2 commits December 20, 2017 16:48

improve comment about instantiating anon types

4f549fe

fix truncated comment

d925f4d

arielb1 reviewed Dec 20, 2017

View reviewed changes

theotherphil reviewed Dec 20, 2017

View reviewed changes

bors merged commit d925f4d into rust-lang:master Dec 21, 2017

bors mentioned this pull request Dec 21, 2017

Default Type Parameter Fallback: Revival #46206

Closed

This was referenced Dec 21, 2017

add #![feature(nll)] to supplement -Znll #46716

Closed

Want -Z borrowck=migrate to test planning migration to non-lexical lifetimes #46908

Closed

ghost mentioned this pull request Dec 21, 2017

NLL regression: cannot assign to *self because it is borrowed #46917

Closed

bradleybeddoes mentioned this pull request Mar 4, 2018

Add examples using sessions gotham-rs/gotham#168

Merged

jackh726 mentioned this pull request Jun 3, 2022

Remove migrate borrowck mode #95565

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NLL feature complete (adds `feature(nll)`)! #46862

NLL feature complete (adds `feature(nll)`)! #46862

nikomatsakis commented Dec 20, 2017 •

edited

Loading

bors commented Dec 20, 2017

bors commented Dec 20, 2017

arielb1 Dec 20, 2017 •

edited

Loading

nikomatsakis Dec 20, 2017

arielb1 Dec 20, 2017

nikomatsakis Dec 20, 2017

nikomatsakis Dec 20, 2017

arielb1 Dec 20, 2017

arielb1 Dec 20, 2017 •

edited

Loading

theotherphil Dec 20, 2017

arielb1 commented Dec 20, 2017

bors commented Dec 20, 2017

arielb1 commented Dec 20, 2017

bors commented Dec 20, 2017

bors commented Dec 21, 2017

NLL feature complete (adds feature(nll))! #46862

NLL feature complete (adds feature(nll))! #46862

Conversation

nikomatsakis commented Dec 20, 2017 • edited Loading

bors commented Dec 20, 2017

bors commented Dec 20, 2017

arielb1 Dec 20, 2017 • edited Loading

Choose a reason for hiding this comment

nikomatsakis Dec 20, 2017

Choose a reason for hiding this comment

arielb1 Dec 20, 2017

Choose a reason for hiding this comment

nikomatsakis Dec 20, 2017

Choose a reason for hiding this comment

nikomatsakis Dec 20, 2017

Choose a reason for hiding this comment

arielb1 Dec 20, 2017

Choose a reason for hiding this comment

arielb1 Dec 20, 2017 • edited Loading

Choose a reason for hiding this comment

theotherphil Dec 20, 2017

Choose a reason for hiding this comment

arielb1 commented Dec 20, 2017

bors commented Dec 20, 2017

arielb1 commented Dec 20, 2017

bors commented Dec 20, 2017

bors commented Dec 21, 2017

NLL feature complete (adds `feature(nll)`)! #46862

NLL feature complete (adds `feature(nll)`)! #46862

nikomatsakis commented Dec 20, 2017 •

edited

Loading

arielb1 Dec 20, 2017 •

edited

Loading

arielb1 Dec 20, 2017 •

edited

Loading