Depending on if upvar_tys inferred or not, we were returning either an
inference variable which later resolves to a tuple or else the upvar tys
themselves
Co-authored-by: Roxane Fruytier <roxane.fruytier@hotmail.com>
This commit allows us to decide the number of captures required after
completing capture ananysis, which is required as part of implementing
RFC-2229.
Co-authored-by: Aman Arora <me@aman-arora.com>
Co-authored-by: Jenny Wills <wills.jenniferg@gmail.com>
If a symbol name can only be imported from one place for a type, and
as long as it was not glob-imported anywhere in the current crate, we
can trim its printed path and print only the name.
This has wide implications on error messages with types, for example,
shortening `std::vec::Vec` to just `Vec`, as long as there is no other
`Vec` importable anywhere.
This adds a new '-Z trim-diagnostic-paths=false' option to control this
feature.
On the good path, with no diagnosis printed, we should try to avoid
issuing this query, so we need to prevent trimmed_def_paths query on
several cases.
This change also relies on a previous commit that differentiates
between `Debug` and `Display` on various rustc types, where the latter
is trimmed and presented to the user and the former is not.
Warn about unused expressions with closure or generator type. This follows
existing precedence of must use annotations present on `FnOnce`, `FnMut`, `Fn`
traits, which already indirectly apply to closures in some cases, e.g.,:
```rust
fn f() -> impl FnOnce() {
|| {}
}
fn main() {
// an existing warning: unused implementer of `std::ops::FnOnce` that must be used:
f();
// a new warning: unused closure that must be used:
|| {};
}
```
This now reuses `fn discriminant_ty` in project, removing
some code duplication. Doing so made me realize that
we previously had a disagreement about the discriminant
type of generators, with MIR using `u32` and codegen and
trait selection using `i32`.
We now always use `u32`.
The bug was revealed by the behavior of the old-lub-glb-hr-noteq1.rs
test. The old-lub-glb-hr-noteq2 test shows the current 'order dependent'
behavior of coercions around higher-ranked functions, at least when
running with `-Zborrowck=mir`.
Also, run compare-mode=nll.
In particular, it no longer occurs during the subtyping check. This is
important for enabling lazy normalization, because the subtyping check
will be producing sub-obligations that could affect its results.
Consider an example like
for<'a> fn(<&'a as Mirror>::Item) =
fn(&'b u8)
where `<T as Mirror>::Item = T` for all `T`. We will wish to produce a
new subobligation like
<'!1 as Mirror>::Item = &'b u8
This will, after being solved, ultimately yield a constraint that `'!1
= 'b` which will fail. But with the leak-check being performed on
subtyping, there is no opportunity to normalize `<'!1 as
Mirror>::Item` (unless we invoke that normalization directly from
within subtyping, and I would prefer that subtyping and unification
are distinct operations rather than part of the trait solving stack).
The reason to keep the leak check during coherence and trait
evaluation is partly for backwards compatibility. The coherence change
permits impls for `fn(T)` and `fn(&T)` to co-exist, and the trait
evaluation change means that we can distinguish those two cases
without ambiguity errors. It also avoids recreating #57639, where we
were incorrectly choosing a where clause that would have failed the
leak check over the impl which succeeds.
The other reason to keep the leak check in those places is that I
think it is actually close to the model we want. To the point, I think
the trait solver ought to have the job of "breaking down"
higher-ranked region obligation like ``!1: '2` into into region
obligations that operate on things in the root universe, at which
point they should be handed off to polonius. The leak check isn't
*really* doing that -- these obligations are still handed to the
region solver to process -- but if/when we do adopt that model, the
decision to pass/fail would be happening in roughly this part of the
code.
This change had somewhat more side-effects than I anticipated. It
seems like there are cases where the leak-check was not being enforced
during method proving and trait selection. I haven't quite tracked
this down but I think it ought to be documented, so that we know what
precisely we are committing to.
One surprising test was `issue-30786.rs`. The behavior there seems a
bit "fishy" to me, but the problem is not related to the leak check
change as far as I can tell, but more to do with the closure signature
inference code and perhaps the associated type projection, which
together seem to be conspiring to produce an unexpected
signature. Nonetheless, it is an example of where changing the
leak-check can have some unexpected consequences: we're now failing to
resolve a method earlier than we were, which suggests we might change
some method resolutions that would have been ambiguous to be
successful.
TODO:
* figure out remainig test failures
* add new coherence tests for the patterns we ARE disallowing
Clean up logic around live locals in generator analysis
Resolves#69902. Requires #71893.
I've found it difficult to make changes in the logic around live locals in `generator/transform.rs`. It uses a custom dataflow analysis, `MaybeRequiresStorage`, that AFAICT computes whether a local is either initialized or borrowed. That analysis is using `before` effects, which we should try to avoid if possible because they are harder to reason about than ones only using the unprefixed effects. @pnkfelix has suggested removing "before" effects entirely to simplify the dataflow framework, which I might pursue someday.
This PR replaces `MaybeRequiresStorage` with a combination of the existing `MaybeBorrowedLocals` and a new `MaybeInitializedLocals`. `MaybeInitializedLocals` is just `MaybeInitializedPlaces` with a coarser resolution: it works on whole locals instead of move paths. As a result, I was able to simplify the logic in `compute_storage_conflicts` and `locals_live_across_suspend_points`.
This is not exactly equivalent to the old logic; some generators are now smaller than before. I believe this was because the old logic was too conservative, but I'm not as familiar with the constraints as the original implementers were, so I could be wrong. For example, I don't see a reason the size of the `mixed_sizes` future couldn't be 5K. It went from 7K to 6K in this PR.
r? @jonas-schievink @tmandry
Make `needs_drop` less pessimistic on generators
Generators only have non-trivial drop logic when they may store (in upvars or across yields) a type that does.
This prevents generation of some unnecessary MIR in simple generators. There might be some impact on compile times, but this is probably limited in real-world applications.
~~This builds off of https://github.com/rust-lang/rust/pull/69814 since that contains some fixes that are made relevant by *this* PR (see https://github.com/rust-lang/rust/pull/69814#issuecomment-599147269).~~ (this has been merged)
They used to be covered by `optin_builtin_traits` but negative impls
are now applicable to all traits, not just auto traits.
This also adds docs in the unstable book for the current state of auto traits.
Use smaller discriminants for generators
Closes https://github.com/rust-lang/rust/issues/69815
I'm not yet sure about the runtime performance impact of this, so I'll try running this on some benchmarks (if I can find any). (Update: No impact on the benchmarks I've measured on)
* [x] Add test with a generator that has exactly 256 total states
* [x] Add test with a generator that has more than 256 states so that it needs to use a u16 discriminant
* [x] Add tests for the size of `Option<[generator]>`
* [x] Add tests for the `discriminant_value` intrinsic in all cases
Don't store locals in generators that are immediately overwritten with the resume argument
This fixes https://github.com/rust-lang/rust/issues/69672 and makes https://github.com/rust-lang/rust/pull/69033 pass the async fn size tests again (in other words, there will be no size regression of async fn if both this and https://github.com/rust-lang/rust/pull/69033 land).
~~This is a small botch and I'd rather have a more precise analysis, but that seems much harder to pull off, so this special-cases `Yield` terminators that store the resume argument into a simple local (ie. without any field projections) and explicitly marks that local as "not live" in the suspend point of that yield. We know that this local does not need to be stored in the generator for this suspend point because the next resume would immediately overwrite it with the passed-in resume argument anyways. The local might still end up in the state if it is used across another yield.~~ (this now properly updates the dataflow framework to handle this case)
