A couple of tiny polonius things
Here's a couple of tiny things I had ready to go @jackh726
- a tiny cleanup to avoid round-tripping through `Location`s to check for liveness, since we're already working with points
- while I was there, I fixed the doc which wasn't showing up properly (maybe a rustdoc or bootstrap bug when we build locally or during dist, either way, both don't show up correctly linked most of the time) for `PointIndex`es.
- and in the second commit slightly expand test coverage with [an example](https://internals.rust-lang.org/t/get-mut-map-back-from-entry-api/24003) that would have needed stdlib changes (cc author @Darksonn for visibility here)
More soon.
r? @jackh726
now that we need to hold the graph for MIR dumping, and the associated
data to traverse it, there is no difference between the main context and
diagnostics context, so we merge them.
we may need to traverse the lazy graph multiple times:
- to record loan liveness
- to dump the localized outlives constraint in the polonius MIR dump
to do that we extract the previous loan liveness code into an abstract
traversal + visitor handling the liveness-specific parts, while the MIR
dump will be able to record constraints in its own visitor.
now that the analysis is only using the regular liveness shape, we don't
need to store it transposed, and thus don't need the container where it
was stored: the liveness context would be alone in the polonius context.
we thus remove the latter, and rename the former.
Borrowck: simplify diagnostics for placeholders
This folds the call to `region_from_element` into `RegionInferenceContext`, and simplifies the error variant for this case to only talk about regions as opposed to elements. This is the only case where a `RegionElement` leaks out of region inference, so now they can be considered internal to region inference (though that currently isn't expressed). It also clarifies the type information on the methods called to emphasise the fact that they only ever use placeholder regions in the diagnostics completely ignore any other element.
It also adds a bunch of FIXMEs to some fishy statements that conjure universes from what seems like arbitrary integers.
This was lifted from rust-lang/rust#142623.
r? @lcnr
borrowck: suggest `&mut *x` for pattern reborrows
Fixesrust-lang/rust#81059
r? @estebank as you should have some context here, but feel free to re-assign if you don't have time to review right now.
This essentially folds the call to `region_from_element` into `RegionInferenceContext`,
and simplifies the error variant for this case. It also clarifies the type
information on the methods called to emphasise the fact that they only ever use
placeholder regions in the diagnostics, and completely ignore any other element.
Check stalled coroutine obligations eagerly
Fixesrust-lang/rust#151322Fixesrust-lang/rust#151323Fixesrust-lang/rust#137916Fixesrust-lang/rust#138274
The problem is that stalled coroutine obligations can't be satisifed so that they cause normalization to fail in `mir_borrowck`.
Thus, we failed to register any opaque to storage in the next solver.
I fix it by checking these obligations earlier in `mir_borrowck`.
r? @lcnr
Suppress unused_mut lint if mutation fails due to borrowck error
Remedying the borrowck error will likely result in the mut becoming used, and therefore the lint is likely incorrect.
Fixesrust-lang/rust#152024
r? compiler
Remove some unnecessary `try`-related type annotations
I left a few, like
```rust
let result: Result<_, ModError<'_>> = try {
```
where it felt like seeing it might still be useful for the reader.
Feel free to push back on any of these changes if you think they should be left alone.
Provide more context on trait bounds being unmet due to imperfect derive
When encountering a value that has a borrow checker error where the type was previously moved, when suggesting cloning verify that it is not already being derived. If it is, explain why the `derive(Clone)` doesn't apply:
```
note: if `TypedAddress<T>` implemented `Clone`, you could clone the value
--> $DIR/derive-clone-implicit-bound.rs:6:1
|
LL | #[derive(Clone, Copy)]
| ----- derived `Clone` adds implicit bounds on type parameters
LL | pub struct TypedAddress<T>{
| ^^^^^^^^^^^^^^^^^^^^^^^^-^
| | |
| | introduces an implicit `T: Clone` bound
| consider manually implementing `Clone` for this type
...
LL | let old = self.return_value(offset);
| ------ you could clone this value
```
When encountering a bound coming from a derive macro, suggest manual impl of the trait.
Use the span for the specific param when adding bounds in builtin derive macros, so the diagnostic will point at them as well as the derive macro itself.
```
note: required for `Id<SomeNode>` to implement `PartialEq`
--> $DIR/derive-implicit-bound.rs:5:10
|
LL | #[derive(PartialEq, Eq)]
| ^^^^^^^^^
LL | pub struct Id<T>(PhantomData<T>);
| - unsatisfied trait bound introduced in this `derive` macro
= help: consider manually implementing `PartialEq` to avoid undesired bounds
```
Mention that the trait could be manually implemented in E0599.
Fixrust-lang/rust#108894. Address rust-lang/rust#143714. Address #rust-lang/rust#146515 (but ideally would also suggest constraining the fn bound correctly as well).
When encountering a value that has a borrow checker error where the type was previously moved, when suggesting cloning verify that it is not already being derived. If it is, explain why the `derive(Clone)` doesn't apply:
```
note: if `TypedAddress<T>` implemented `Clone`, you could clone the value
--> $DIR/derive-clone-implicit-bound.rs:6:1
|
LL | #[derive(Clone, Copy)]
| ----- derived `Clone` adds implicit bounds on type parameters
LL | pub struct TypedAddress<T>{
| ^^^^^^^^^^^^^^^^^^^^^^^^-^
| | |
| | introduces an implicit `T: Clone` bound
| consider manually implementing `Clone` for this type
...
LL | let old = self.return_value(offset);
| ------ you could clone this value
```
I left a few, like
```rust
let result: Result<_, ModError<'_>> = try {
```
where it felt like seeing it might still be useful for the reader.
Feel free to push back on any of these changes if you think seeing the type would be better.
Improve move error diagnostic for `AsyncFn` closures
When an async closure captures a variable by move but is constrained to `AsyncFn` or `AsyncFnMut`, the error message now explains that the closure kind is the issue and points to the trait bound, similar to the existing diagnostic for `Fn`/`FnMut` closures.
**Before:**
```
error[E0507]: cannot move out of `foos` which is behind a shared reference
--> src/lib.rs:12:20
|
11 | async fn foo(foos: &mut [&mut Foo]) -> Result<(), ()> {
| ---- move occurs because `foos` has type...
12 | in_transaction(async || -> Result<(), ()> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ `foos` is moved here
13 | for foo in foos {
| ---- variable moved due to use in coroutine
```
**After:**
```
error[E0507]: cannot move out of `y`, a captured variable in an `AsyncFn` closure
--> src/lib.rs:9:10
|
LL | let y = vec![format!("World")];
| - captured outer variable
LL | call(async || {
| ^^^^^^^^ captured by this `AsyncFn` closure
...
help: `AsyncFn` and `AsyncFnMut` closures require captured values to be able
to be consumed multiple times, but `AsyncFnOnce` closures may consume
them only once
--> src/lib.rs:5:27
|
LL | fn call<F>(_: F) where F: AsyncFn() {}
| ^^^^^^^^^
```
Fixesrust-lang/rust#150174
When an async closure captures a variable by move but is constrained to
`AsyncFn` or `AsyncFnMut`, the error message now explains that the
closure kind is the issue and points to the trait bound, similar to the
existing diagnostic for `Fn`/`FnMut` closures.
This simplifies the `PlaceholderReachability` `enum` by
replacing the case when no placeholders were reached with
a standard `Option::None`.
It also rewrites the API for `scc::Annotations` to be update-mut
rather than a more Functional programming style. This showed some slight
performance impact in early tests of the PR and definitely makes
the implementation simpler.
inline constant localized typeck constraint computation
This fixes an oversight in the previous PRs, this constraint is local to a point (and liveness does the rest) and so has a fixed direction.
I wasn't planning on trying to improve the impl for perf, versus computing loan liveness without first unifying the cfg and subset graph, but it's like a 20x improvement for typeck constraints on wg-grammar (-15% end-to-end) for a trivial fix.
r? @jackh726
In general, I want to cleanup these edges to avoid off-by-one errors in constraints at effectful statements and ensure the midpoint-avoidance strategy is sound and works well, in particular with respect to edges that flow backwards from the result into its inputs. But I'd like to start from something that passes all tests and is simpler, because the eventual solution may
1. involve localizing these edges differently than *separate* liveness and typeck lowering passes/approaches, which would need to be lowered at the same time for example. I'm already doing the latter in the loan liveness rewrite as part of creating edges on-demand during traversal, and this new structure would be a better fit to verify, or fix, these subtle edges.
2. also require changes in MIR typeck to track the flow across points more precisely, and I don't know how hard that would be. *Computing* the constraint direction is currently a workaround for that.
Therefore, in a future PR, I'll also remove this computation from the terminator constraints, but I can also do that in this PR if you'd prefer.
Remove the diagnostic lints
Removes the `untranslatable_diagnostic` and `diagnostic_outside_of_impl` lints
These lints are allowed for a while already. Per https://github.com/rust-lang/compiler-team/issues/959, we no longer want to enforce struct diagnostics for all usecases, so this is no longer useful.
r? @Kivooeo
I recommend reviewing commit by commit (also feel free to wait with reviewing until the MCP is accepted)
@rustbot +S-blocked
Blocked by https://github.com/rust-lang/compiler-team/issues/959
