Rest In Peace, AST borrowck (2012-2019) #64790

Centril · 2019-09-25T23:35:14Z

After having served us for 7 years, the AST borrow-checker is no more.

This PR starts from the commit rm -rf librustc_ast_borrowck, building on #64221, and is probably best read commit by commit.

Migrate mode is not removed yet as it may be useful for NLL => polonius and it is also used for the mutable_borrow_reservation_conflict issue (#59159).

r? @matthewjasper

lqd · 2019-09-26T07:55:49Z

You've checked our borrows well, since April 26/27 2012
So long, and thanks for all the compile errors.

src/test/ui/issues/issue-45696-scribble-on-boxed-borrow.rs

jhpratt · 2019-09-26T17:28:59Z

As someone who is not terribly familiar with the compiler internals, are there any practical implications of this change?

Centril · 2019-09-26T18:04:00Z

@jhpratt My understanding is that it will allow us to do cleanups in more places, e.g. with respect to rvalue-promotion and whatnot and that might make us more nimble in terms of improvements.

For end-users there should be no change other than closing some soundness holes but that is done first as a part of #64221 which this PR depends on.

Centril · 2019-09-27T01:03:47Z

Rebased (#64221 landed). Should be ready to go now. :)

matthewjasper

r=me with comment addressed

src/librustc_driver/pretty.rs

Centril · 2019-09-27T07:22:26Z

@bors r=matthewjasper p=3 rollup=never

bors · 2019-09-27T07:22:27Z

📌 Commit d3d5fe78a43a9c7bdb8d381cce53d15d19dd2d23 has been approved by matthewjasper

mark-i-m · 2019-09-27T14:26:12Z

Many, many congratulations to those who have worked towards this milestone 🎉🎉🎉

Centril · 2019-09-27T18:15:52Z

@bors r=matthewjasper p=3 rollup=never

bors · 2019-09-27T18:15:53Z

📌 Commit 001357f has been approved by matthewjasper

bors · 2019-09-27T21:17:17Z

⌛ Testing commit 001357f with merge c6f87c2...

@matthewjasper

Rest In Peace, AST borrowck (2012-2019) After having served us for 7 years, the AST borrow-checker is no more. This PR starts from the commit `rm -rf librustc_ast_borrowck`, building on #64221, and is probably best read commit by commit. Migrate mode is not removed yet as it may be useful for NLL => polonius and it is also used for the `mutable_borrow_reservation_conflict` issue (#59159). r? @matthewjasper ------------------------ ![ast-borrowck-rip](https://user-images.githubusercontent.com/855702/65646791-91a87600-dffc-11e9-9814-deed6b821c80.png)

bors · 2019-09-28T01:18:04Z

☀️ Test successful - checks-azure
Approved by: matthewjasper
Pushing c6f87c2 to master...

Centril · 2019-09-28T01:19:06Z

🎉 🍾 🎈 Score! 🎈 🍾 🎉

Veetaha · 2019-12-21T18:44:29Z

Oh, God, you made this feel so emotional...
Poor, poor AST borrowck ;(

…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 matthewjasper Sep 25, 2019

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

pnkfelix mentioned this pull request Sep 25, 2019

NLL: complete transition from migrate to full NLL #57895

Closed

6 tasks

nikomatsakis mentioned this pull request Sep 26, 2019

non-lexical lifetimes (NLL) tracking issue #43234

Closed

23 tasks

This comment has been minimized.

Sign in to view

pnkfelix reviewed Sep 26, 2019

View reviewed changes

src/test/ui/issues/issue-45696-scribble-on-boxed-borrow.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

Centril mentioned this pull request Sep 26, 2019

Rust 2015: No longer downgrade NLL errors #64221

Merged

Centril force-pushed the rip-ast-borrowck branch from 1d0c0b4 to 1192d76 Compare September 27, 2019 00:55

matthewjasper reviewed Sep 27, 2019

View reviewed changes

src/librustc_driver/pretty.rs Show resolved Hide resolved

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 Sep 27, 2019

This comment has been minimized.

Sign in to view

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 Sep 27, 2019

This comment has been minimized.

Sign in to view

Centril added 7 commits September 27, 2019 19:48

rm -rf librustc_ast_borrowck

a094926

Remove librustc_ast_borrowck from driver & interface.

83dcdd9

don't borrowck::check_crate(tcx) anymore.

463b197

Remove unpretty=flowgraph.

1eb280e

cleanup check_match wrt. SignalledError.

defd508

cleanup dead ast-borrowck / migrate-mode code.

dfd365f

Remove AccessKind::Move.

a07b030

Centril added 3 commits September 27, 2019 19:54

Inline the remaining hir::Arm::top_pats_hack

0022baa

-Z unpretty message: include expanded,hygiene

9920402

--bless --compare-mode=nll

001357f

Centril force-pushed the rip-ast-borrowck branch from d3d5fe7 to 001357f Compare September 27, 2019 18:14

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 27, 2019

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 28, 2019

bors merged commit 001357f into rust-lang:master Sep 28, 2019

Centril deleted the rip-ast-borrowck branch September 28, 2019 01:18

bors mentioned this pull request Sep 28, 2019

No StableHasherResult everywhere #64824

Merged

bjorn3 mentioned this pull request Nov 17, 2019

-Zborrowck and #![feature(nll)] still exist #66494

Closed

hanna-kruppe mentioned this pull request Jan 2, 2020

kill rustc::cfg consumers #51002

Closed

3 tasks

steffahn mentioned this pull request Apr 11, 2020

ICE: No move_errors should be allowed in MIR borrowck #70934

Closed

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

Rest In Peace, AST borrowck (2012-2019) #64790

Rest In Peace, AST borrowck (2012-2019) #64790

Centril commented Sep 25, 2019 •

edited

Loading

This comment has been minimized.

lqd commented Sep 26, 2019

This comment has been minimized.

jhpratt commented Sep 26, 2019

Centril commented Sep 26, 2019 •

edited

Loading

Centril commented Sep 27, 2019

matthewjasper left a comment

Centril commented Sep 27, 2019

bors commented Sep 27, 2019

This comment has been minimized.

This comment has been minimized.

mark-i-m commented Sep 27, 2019

Centril commented Sep 27, 2019

bors commented Sep 27, 2019

bors commented Sep 27, 2019

bors commented Sep 28, 2019

Centril commented Sep 28, 2019 •

edited

Loading

Veetaha commented Dec 21, 2019

Rest In Peace, AST borrowck (2012-2019) #64790

Rest In Peace, AST borrowck (2012-2019) #64790

Conversation

Centril commented Sep 25, 2019 • edited Loading

This comment has been minimized.

lqd commented Sep 26, 2019

This comment has been minimized.

jhpratt commented Sep 26, 2019

Centril commented Sep 26, 2019 • edited Loading

Centril commented Sep 27, 2019

matthewjasper left a comment

Choose a reason for hiding this comment

Centril commented Sep 27, 2019

bors commented Sep 27, 2019

This comment has been minimized.

This comment has been minimized.

mark-i-m commented Sep 27, 2019

Centril commented Sep 27, 2019

bors commented Sep 27, 2019

bors commented Sep 27, 2019

bors commented Sep 28, 2019

Centril commented Sep 28, 2019 • edited Loading

🎉 🍾 🎈 Score! 🎈 🍾 🎉

Veetaha commented Dec 21, 2019

Centril commented Sep 25, 2019 •

edited

Loading

Centril commented Sep 26, 2019 •

edited

Loading

Centril commented Sep 28, 2019 •

edited

Loading