[NLL] Remove `LiveVar`
The `LiveVar` type (and related) made it harder to reason about the code. It seemed as an abstraction that didn't bring any useful concept to the reader (when transitioning from the RFC theory to the actual implementation code).
It achieved a compactness in the vectors storing the def/use/drop information that was related only to the `LocalUseMap`. This PR went in the other direction and favored time over memory (but this decision can be easily reverted to the other side without reintroducing `LiveVar`).
What this PR aims at is to clarify that there's no significant transformation between the MIR `Local` and the `LiveVar` (now refactored as `live_locals: Vec<Local>`): we're just filtering (not mapping) the entire group of `Local`s into a meaningful subset that we should perform the liveness analysis on.
As a side note, there is no guarantee that the liveness analysis is performed only on (what the code calls) "live" variables, if the NLL facts are requested it will be performed on *any* variable so there can't be any assumptions on that regard. (Still, this PR didn't change the general naming convention to reduce the number of changes here and streamline the review process).
**Acceptance criteria:** This PR attempts to do only a minor refactoring and not to change the logic so it can't have any performance impact, particularly, it can't lose any of the significant performance improvement achieved in the great work done in https://github.com/rust-lang/rust/pull/52115.
r? @nikomatsakis
[NLL] Type check operations with pointer types
It seems these were forgotten about. Moving to `Rvalue::AddressOf` simplifies the coercions from references, but I want this to be fixed as soon as possible.
r? @pnkfelix
Type annotations are shared between the MIR of a function and the
promoted constants for that function, so keep them in the type checker
when we check the promoted MIR.
Function signatures with the `variadic` member set are actually
C-variadic functions. Make this a little more explicit by renaming the
`variadic` boolean value, `c_variadic`.
Use normal mutable borrows in matches
`ref mut` borrows are currently two-phase with NLL enabled. This changes them to be proper mutable borrows. To accommodate this, first the position of fake borrows is changed:
```text
[ 1. Pre-match ]
|
[ (old create fake borrows) ]
[ 2. Discriminant testing -- check discriminants ] <-+
| |
| (once a specific arm is chosen) |
| |
[ (old read fake borrows) ] |
[ 3. Create "guard bindings" for arm ] |
[ (create fake borrows) ] |
| |
[ 4. Execute guard code ] |
[ (read fake borrows) ] --(guard is false)-----------+
|
| (guard results in true)
|
[ 5. Create real bindings and execute arm ]
|
[ Exit match ]
```
The following additional changes are made to accommodate `ref mut` bindings:
* We no longer create fake `Shared` borrows. These borrows are no longer needed for soundness, just to avoid some arguably strange cases.
* `Shallow` borrows no longer conflict with existing borrows, avoiding conflicting access between the guard borrow access and the `ref mut` borrow.
There is some further clean up done in this PR:
* Avoid the "later used here" note for Shallow borrows (since it's not relevant with the message provided)
* Make any use of a two-phase borrow activate it.
* Simplify the cleanup_post_borrowck passes into a single pass.
cc #56254
r? @nikomatsakis
Extend `LocalUseMap`'s `IndexVec`s that track def/use/drop data to store the
original `Local` indexes and not the compacted `LiveVar` ones (favoring speed
and code simplicity over space). Remove the `NllLivenessMap` embedded inside it
since it's no longer needed to perform the `LiveVar`/`Local` conversion.
It was used in `compute_for_all_locals` to iterate only the `Local`s that need
liveness analysis (filtered through `compute`). Instead, explicitly extract that
reduced set (as `live_locals`) in `trace` and pass it to
`compute_for_all_locals`.
Change the variable type used in `compute_for_all_locals` from `LiveVar` to
`Local` and do the same for its helper functions (and the functions in
`LocalUseMap` they rely on):
* `add_defs_for` -> `LocalUseMap::defs`
* `compute_use_live_points_for` -> `LocalUseMap::uses`
* `compute_drop_live_points_for` -> `LocalUseMap::drops`
Push back the use of `LiveVar` to the `LocalUseMap` (where the other
`NllLivenessMap` remains embedded) functions which internally do the
`from_local` conversion.
Cleanup: rename node_id_to_type(_opt)
Renames `node_id_to_type(_opt)` to `hir_id_to_type(_opt)`; this makes it clear we are dealing with HIR nodes and their IDs here.
In addition, a drive-by commit removing `ty::item_path::hir_path_str` (as requested by @eddyb).
Cosmetic improvements to doc comments
This has been factored out from https://github.com/rust-lang/rust/pull/58036 to only include changes to documentation comments (throughout the rustc codebase).
r? @steveklabnik
Once you're happy with this, maybe we could get it through with r=1, so it doesn't constantly get invalidated? (I'm not sure this will be an issue, but just in case...) Anyway, thanks for your advice so far!
Fix span for closure return type when annotated.
Fixes#58053.
This PR adjusts the span used to label closure return types so that
if the user specifies the return type, i.e. `|_| -> X {}` instead of
`|_| {}`, we correctly highlight all of it and not just the last
character.
r? @pnkfelix
This commit adjusts the span used to label closure return types so that
if the user specifies the return type, i.e. `|_| -> X {}` instead of
`|_| {}`, we correctly highlight all of it and not just the last
character.
[NLL] Clean up handling of type annotations
* Renames (Canonical)?UserTypeAnnotation -> (Canonical)?UserType so that the name CanonicalUserTypeAnnotation is free.
* Keep the inferred type associated to user type annotations in the MIR, so that it can be compared against the annotated type, even when the annotated expression gets removed from the MIR. (#54943)
* Use the inferred type to allow infallible handling of user type projections (#57531)
* Uses revisions for the tests in #56993
* Check the types of `Unevaluated` constants with no annotations (#46702)
* Some drive-by cleanup
Closes#46702Closes#54943Closes#57531Closes#57731
cc #56993 leaving this open to track the underlying issue: we are not running tests with full NLL enabled on CI at the moment
r? @nikomatsakis
Universes
This PR transitions the compiler to use **universes** instead of the **leak-check**. It is marked as [WIP] for a few reasons:
- The diagnostics at present are terrible =)
- This changes the behavior of coherence, regressing some things that used to compile
The goals of this PR at present are:
- To start getting some eyes on the code
- To do a crater run
- To see the full travis results (due to https://github.com/rust-lang/rust/issues/52452, I am not able to run the full test suite locally anymore at present)
The first few commits in the PR are changing how `evaluate` treats regions. We now track whether region comparisons occurred, reverting the "staticized" query approach that @arielb1 put in. The problem with "staticized" queries is that it relied on the leak-check to get higher-ranked things correct, and we are removing the leak-check in this PR series, so that caused problems.
You can see at the end a collection of test updates. Mostly we behave the same but with atrocious diagnostics, but there are a number of cases where we used to error and now no longer do, as well as single case where we used to **not** error but we now do (the coherence-subtyping change).
(Note: it would be possible to do a version of leak-check that propagates universe information and recover the old behavior. I am reluctant to do so because I'd like to leave us room to get more precise -- e.g., I want to eventually handle things like `exists<'a> { for<'b> { if ('a: 'b) { 'a: 'b } } }` which presently the leak-check cannot cope with etc. Also because it seems more consistent to me: most folks I've talked to expect the new behavior and are surprised to learn that binding sites were so significant before when it comes to coherence. One question is, though, to what extent are people relying on this in the wild?)
