If we have a cause containing `ValuePairs::PolyTraitRefs` but neither
TraitRef has any escaping bound regions then we report the same error as
for `ValuePairs::TraitRefs`.
Add expected error
Add comment
Tweak comment wording
Fix after rebase to updated master
Fix after rebase to updated master
Distinguish mutation in normal and move closures
Tweak error message
Fix error message for nested closures
Refactor code showing mutated upvar in closure
Remove debug assert
B
Rework diagnostics for wrong number of generic args (fixes#66228 and #71924)
This PR reworks the `wrong number of {} arguments` message, so that it provides more details and contextual hints.
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 is a re-attempt of #72389 (which was reverted in #73594)
Instead of using `ExpnKind::Desugaring` to represent operators, this PR
checks the lang item directly.
move leak-check to during coherence, candidate eval
Implementation of MCP https://github.com/rust-lang/compiler-team/issues/295.
I'd like to do a crater run on this.
Note to @rust-lang/lang: This PR is a breaking change (bugfix). It causes tests like the following to go from a future-compatibility warning #56105 to a hard error:
```rust
trait Trait {}
impl Trait for for<'a, 'b> fn(&'a u32, &'b u32) {}
impl Trait for for<'c> fn(&'c u32, &'c u32) {} // now rejected, used to warn
```
I am not aware of any instances of this code in the wild, but that is why we are doing a crater run. The reason for this change is that those two types are, in fact, the same type, and hence the two impls are overlapping.
There will still be impls that trigger #56105 after this lands, however -- I hope that we will eventually just accept those impls without warning, for the most part. One example of such an impl is this pattern, which is used by wasm-bindgen and other crates as well:
```rust
trait Trait {}
impl<T> Trait for fn(&T) { }
impl<T> Trait for fn(T) { } // still accepted, but warns
```
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
This still doesn't handle the case entirely correctly, requiring a more
targeted approach with a better suggestion, but at least now the
suggested syntax makes *some* sense.
* `Place` is no longer recursive.
* The `cmt` type alias is removed
* `Upvar` places no longer include the dereferences of the environment
closure or of by reference captures.
* All non-dereference projections are combined to a single variant.
* Various unnecessary types and methods have been removed.
