-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
integrate MIR type-checker with NLL inference #45825
integrate MIR type-checker with NLL inference #45825
Conversation
(Seems like this doesn't bootstrap, I had only been building with |
Fixed the problem that was preventing bootstrapping in the final commit. Removing the |
☔ The latest upstream changes (presumably #45757) made this pull request unmergeable. Please resolve the merge conflicts. |
7f2c82f
to
edf4328
Compare
Rebased. |
☔ The latest upstream changes (presumably #45932) made this pull request unmergeable. Please resolve the merge conflicts. |
@arielb1 so -- there's a lot of code here I know -- would it be helpful if I broke it up into smaller PRs that you can review independently? (For the time being, I've created a "shadow master" on my own repo to allow NLL work to continue, since it seems like we're not landing things on master fast enough to do all the work there.) |
Nope, I just missed this not being WIP. |
One thing I was thinking. This PR made some changes to how the MIR type-checker works -- in particular, it tries to process all obligations fully. This could have an impact on performance. I was thinking of making those changes conditional on Slightly longer term, I have a branch trying to convert trait selection into a standalone query, which would (I believe) address this issue, by allowing us to cache the results independent of where they occurred. But that's still experimental at this point. |
Monomorphic trait evaluation is already very cached, so I don't think this should be much of a problem. |
//! type-check the closure body (e.g., here it informs us that `T` | ||
//! outlives the late-bound region `'a`). | ||
//! | ||
//! > That said, in writing this, I have come to wonder: this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you have an for<'a> |x: &'a mut (&'a u8, ?0)|
closure, and later on you have ?0 = &'b u8
, then the closure gains an 'b: 'a
implied bound, and you can do x.0 = x.1;
, e.g. in:
fn foo<U, F: for<'a> FnMut(&'a mut (&'a u8, U))>(_f: F) {}
fn main() {
foo(|x| { x.0 = x.1; });
}
Whether this is important in some practical case, I have no idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yes, maybe that was the sort of case that made me push it later. Quite likely. I remember a lot of these sorts of things arising during the move to unboxed closures. Anyway it seems fine to do it this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So just fix the comment to not "wonder"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, will do.
/// | ||
/// Not legal during a snapshot. | ||
pub fn take_and_reset_data(&mut self) -> RegionConstraintData<'tcx> { | ||
mem::replace(&mut self.data, RegionConstraintData::default()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this also reset the other fields (e.g. lubs
and glbs
sound important), to avoid leakage? Or maybe these are just not used in MIR regionck?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. I think you are correct about lubs and glbs. At first I think they were ok, but I guess then we would be caching a LUB from some other locale that may not be a LUB here. Good catch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(To be clear: We're not actually using LUB/GLB in NLL, but it seems good to be thorough anyhow.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed. I also was forgetting about unification_table
. =)
src/librustc/traits/mod.rs
Outdated
// properly, and that code is currently largely confined to | ||
// regionck (though I made some efforts to extract it | ||
// out). -nmatsakis | ||
let _ = infcx.ignore_region_obligations(body_id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think wfcheck already checks that these obligations hold (that's it, that the param env is WF if its predicates hold), and this code should be going away anyway with lazy normalization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is why I didn't try to fix this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
Note that we don't have any guarantees that these trait evaluations are monomorphic. This stuff is occuring on the MIR, right? |
variable to an empty value. We then process each outlives constraint | ||
repeatedly, growing region variables until a fixed-point is reached. | ||
Region variables can be grown using a least-upper-bound relation on | ||
the region lattice in a fairly straight-forward fashion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
@@ -1026,6 +1023,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { | |||
let var_name = self.tcx.hir.name(var_node_id); | |||
format!(" for capture of `{}` by closure", var_name) | |||
} | |||
infer::NLL(..) => bug!("NLL variable found in lexical phase"), | |||
}; | |||
|
|||
struct_span_err!(self.tcx.sess, var_origin.span(), E0495, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
See the [general inference README](../README.md) for an overview of | ||
how lexical-region-solving fits into the bigger picture. | ||
|
||
Region constraint collect uses a somewhat more involved algorithm than |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is lexical_region_resolve/README.md
, why does it talk about "Region constraint collect"?
I think just s/Region constraint collect/Region inference/
works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed.
debug!("resolve_var({:?}) = {:?}", rid, result); | ||
result | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
src/librustc/infer/mod.rs
Outdated
// regionck to be sure that it has found *all* the region | ||
// obligations (otherwise, it's easy to fail to walk to a | ||
// particular node-id). | ||
region_obligations: RefCell<NodeMap<Vec<RegionObligation<'tcx>>>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this handle snapshots correctly - i.e. are region obligations that were created in a "child" fulfillcx in a probe ignored? type_known_to_meet_bound
creates a "child" fulfillment context, and I'm not sure it can't be called from typeck snapshots.
If you think nobody should be registering region obligations in a snapshot, which should eventually be the case with MIR regionck, it's a good idea to add an assertion to prevent things blowing up.
858f49e
to
8c109f5
Compare
@bors r=arielb1 |
📌 Commit 8c109f5 has been approved by |
…ielb1 integrate MIR type-checker with NLL inference This branch refactors NLL type inference so that it uses the MIR type-checker to gather constraints. Along the way, it also refactors how region constraints are gathered in the normal inference context mildly. The new setup is like this: - What used to be `region_inference` is split into two parts: - `region_constraints`, which just collects up sets of constraints - `lexical_region_resolve`, which does the iterative, lexical region resolution - When `resolve_regions_and_report_errors` is invoked, the inference engine converts the constraints into final values. - In the MIR type checker, however, we do not invoke this method, but instead periodically take the region constraints and package them up for the NLL solver to use later. - This allows us to track when and where those constraints were incurred. - We also remove the central fulfillment context from the MIR type checker, instead instantiating new fulfillment contexts at each point. This allows us to capture the set of obligations that occurred at a particular point, and also to ensure that if the same obligation arises at two points, we will enforce the region constraints at both locations. - The MIR type checker is also enhanced to instantiate late-bound-regions with fresh variables and handle a few other corner cases that arose. - I also extracted some of the 'outlives' logic from the regionck, which will be needed later (see future work) to handle the type-outlives relationships. One concern I have with this branch: since the MIR type checker is used even without the `-Znll` switch, I'm not sure if it will impact performance. One simple fix here would be to only enable the MIR type-checker if debug-assertions are enabled, since it just serves to validate the MIR. Longer term I hope to address this by improving the interface to the trait solver to be more query-based (ongoing work). There is plenty of future work left. Here are two things that leap to mind: - **Type-region outlives.** Currently, the NLL solver will ICE if it is required to handle a constraint like `T: 'a`. Fixing this will require a small amount of refactoring to extract the implied bounds code. I plan to follow a file-up bug on this (hopefully with mentoring instructions). - **Testing.** It's a good idea to enumerate some of the tricky scenarios that need testing, but I think it'd be nice to try and parallelize some of the actual test writing (and resulting bug fixing): - Same obligation occurring at two points. - Well-formedness and trait obligations of various kinds (which are not all processed by the current MIR type-checker). - More tests for how subtyping and region inferencing interact. - More suggestions welcome! r? @arielb1
☀️ Test successful - status-appveyor, status-travis |
…ielb1 typeck aggregate rvalues in MIR type checker This branch is an attempt to land content by @spastorino and @Nashenas88 that was initially landed on nll-master while we waited for #45825 to land. The biggest change it contains is that it extends the MIR type-checker to also type-check MIR aggregate rvalues (at least partially). Specifically, it checks that the operands provided for each field have the right type. It does not yet check that their well-formedness predicates are met. That is #45827. It also does not check other kinds of rvalues (that is #45959). @spastorino is working on those issues now. r? @arielb1
…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)
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)
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)
This branch refactors NLL type inference so that it uses the MIR type-checker to gather constraints. Along the way, it also refactors how region constraints are gathered in the normal inference context mildly. The new setup is like this:
region_inference
is split into two parts:region_constraints
, which just collects up sets of constraintslexical_region_resolve
, which does the iterative, lexical region resolutionresolve_regions_and_report_errors
is invoked, the inference engine converts the constraints into final values.One concern I have with this branch: since the MIR type checker is used even without the
-Znll
switch, I'm not sure if it will impact performance. One simple fix here would be to only enable the MIR type-checker if debug-assertions are enabled, since it just serves to validate the MIR. Longer term I hope to address this by improving the interface to the trait solver to be more query-based (ongoing work).There is plenty of future work left. Here are two things that leap to mind:
T: 'a
. Fixing this will require a small amount of refactoring to extract the implied bounds code. I plan to follow a file-up bug on this (hopefully with mentoring instructions).r? @arielb1