Rework negative coherence to properly consider impls that only partly overlap
This PR implements a modified negative coherence that handles impls that only have partial overlap.
It does this by:
1. taking both impl trait refs, instantiating them with infer vars
2. equating both trait refs
3. taking the equated trait ref (which represents the two impls' intersection), and resolving any vars
4. plugging all remaining infer vars with placeholder types
these placeholder-plugged trait refs can then be used normally with the new trait solver, since we no longer have to worry about the issue with infer vars in param-envs.
We use the **new trait solver** to reason correctly about unnormalized trait refs (due to deferred projection equality), since this avoid having to normalize anything under param-envs with infer vars in them.
This PR then additionally:
* removes the `FnPtr` knowable hack by implementing proper negative `FnPtr` trait bounds for rigid types.
---
An example:
Consider these two partially overlapping impls:
```
impl<T, U> PartialEq<&U> for &T where T: PartialEq<U> {}
impl<F> PartialEq<F> for F where F: FnPtr {}
```
Under the old algorithm, we would take one of these impls and replace it with infer vars, then try unifying it with the other impl under identity substitutions. This is not possible in either direction, since it either sets `T = U`, or tries to equate `F = &?0`.
Under the new algorithm, we try to unify `?0: PartialEq<?0>` with `&?1: PartialEq<&?2>`. This gives us `?0 = &?1 = &?2` and thus `?1 = ?2`. The intersection of these two trait refs therefore looks like: `&?1: PartialEq<&?1>`. After plugging this with placeholders, we get a trait ref that looks like `&!0: PartialEq<&!0>`, with the first impl having substs `?T = ?U = !0` and the second having substs `?F = &!0`[^1].
Then we can take the param-env from the first impl, and try to prove the negated where clause of the second.
We know that `&!0: !FnPtr` never holds, since it's a rigid type that is also not a fn ptr, we successfully detect that these impls may never overlap.
[^1]: For the purposes of this example, I just ignored lifetimes, since it doesn't really matter.
Rename AsyncCoroutineKind to CoroutineSource
pulled out of https://github.com/rust-lang/rust/pull/116447
Also refactors the printing infra of `CoroutineSource` to be ready for easily extending it with a `Gen` variant for `gen` blocks
Uplift `Canonical` to `rustc_type_ir`
I plan on moving the new trait solver's canonicalizer into either `rustc_type_ir` or a child crate. One dependency on this is lifting `Canonical<V>` to `rustc_type_ir` so we can actually name the canonicalized values.
I may also later lift `CanonicalVarInfo` into the new trait solver. I can't really tell what other changes need to be done, but I'm just putting this up sooner than later since I'm almost certain it'll need to be done regardless of other design choices.
There are a couple of warts introduced by this PR, since we no longer can define inherent `Canonical` impls in `rustc_middle` -- see the changes to:
* `compiler/rustc_trait_selection/src/traits/query/normalize.rs`
* `compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs`
r? lcnr
Improve the warning messages for the `#[diagnostic::on_unimplemented]`
This commit improves warnings emitted for malformed on unimplemented attributes by:
* Improving the span of the warnings
* Adding a label message to them
* Separating the messages for missing and unexpected options
* Adding a help message that says which options are supported
r? `@compiler-errors`
I'm happy to work on further improvements, so feel free to make suggestions.
Return multiple object-safety violation errors and code improvements to the object-safety check
See individual commits for more information. Split off of #114260, since it turned out that the main intent of that PR was wrong.
r? oli-obk
Handle `ReErased` in responses in new solver
There are legitimate cases in the compiler where we return `ReErased` for lifetimes that are uncaptured in the hidden type of an opaque. For example, in the test committed below, we ignore ignore the bivariant lifetimes of an opaque when it's inferred as the hidden type of another opaque. This may result in a `type_of(Opaque)` call returning a type that references `ReErased`. Let's handle this gracefully in the new solver.
Also added a `rustc_hidden_type_of_opaques` attr to print hidden types. This seems useful for opaques.
r? lcnr
Rollup of 6 pull requests
Successful merges:
- #107159 (rand use getrandom for freebsd (available since 12.x))
- #116859 (Make `ty::print::Printer` take `&mut self` instead of `self`)
- #117046 (return unfixed len if pat has reported error)
- #117070 (rustdoc: wrap Type with Box instead of Generics)
- #117074 (Remove smir from triage and add me to stablemir)
- #117086 (Update .mailmap to promote my livename)
r? `@ghost`
`@rustbot` modify labels: rollup
Detect if there is a potential typo where the `{` meant to open the
closure body was written before the body.
```
error[E0277]: expected a `FnOnce<({integer},)>` closure, found `Option<usize>`
--> $DIR/ruby_style_closure_successful_parse.rs:3:31
|
LL | let p = Some(45).and_then({|x|
| ______________________--------_^
| | |
| | required by a bound introduced by this call
LL | | 1 + 1;
LL | | Some(x * 2)
| | ----------- this tail expression is of type `Option<usize>`
LL | | });
| |_____^ expected an `FnOnce<({integer},)>` closure, found `Option<usize>`
|
= help: the trait `FnOnce<({integer},)>` is not implemented for `Option<usize>`
note: required by a bound in `Option::<T>::and_then`
--> $SRC_DIR/core/src/option.rs:LL:COL
help: you might have meant to open the closure body instead of placing a closure within a block
|
LL - let p = Some(45).and_then({|x|
LL + let p = Some(45).and_then(|x| {
|
```
Detect the potential typo where the closure header is missing.
```
error[E0277]: expected a `FnOnce<(&bool,)>` closure, found `bool`
--> $DIR/block_instead_of_closure_in_arg.rs:3:23
|
LL | Some(true).filter({
| _________________------_^
| | |
| | required by a bound introduced by this call
LL | |/ if number % 2 == 0 {
LL | || number == 0
LL | || } else {
LL | || number != 0
LL | || }
| ||_________- this tail expression is of type `bool`
LL | | });
| |______^ expected an `FnOnce<(&bool,)>` closure, found `bool`
|
= help: the trait `for<'a> FnOnce<(&'a bool,)>` is not implemented for `bool`
note: required by a bound in `Option::<T>::filter`
--> $SRC_DIR/core/src/option.rs:LL:COL
help: you might have meant to create the closure instead of a block
|
LL | Some(true).filter(|_| {
| +++
```
Partially address #27300.
Make `ty::print::Printer` take `&mut self` instead of `self`
based on #116815
This simplifies the code by removing all the `self` assignments and
makes the flow of data clearer - always into the printer.
Especially in v0 mangling, which already used `&mut self` in some
places, it gets a lot more uniform.
This simplifies the code by removing all the `self` assignments and
makes the flow of data clearer - always into the printer.
Especially in v0 mangling, which already used `&mut self` in some
places, it gets a lot more uniform.
This commit improves warnings emitted for malformed on unimplemented
attributes by:
* Improving the span of the warnings
* Adding a label message to them
* Separating the messages for missing and unexpected options
* Adding a help message that says which options are supported
Remove lots of generics from `ty::print`
All of these generics mostly resolve to the same thing, which means we can remove them, greatly simplifying the types involved in pretty printing and unlocking another simplification (that is not performed in this PR): Using `&mut self` instead of passing `self` through the return type.
cc `@eddyb` you probably know why it's like this, just checking in and making sure I didn't do anything bad
r? oli-obk
Special case iterator chain checks for suggestion
When encountering method call chains of `Iterator`, check for trailing `;` in the body of closures passed into `Iterator::map`, as well as calls to `<T as Clone>::clone` when `T` is a type param and `T: !Clone`.
Fix#9082.
Fix a performance regression in obligation deduplication.
Commit 8378487 from #114611 changed the location of an obligation deduplication step in `opt_normalize_projection_type`. This meant that deduplication stopped happening on one path where it was still necessary, causing a couple of drastic performance regressions.
This commit moves the deduplication back to the old location. The good news is that #114611 had four commits and 8378487 was of minimal importance, so the perf benefits from that PR remain.
Fixes#116780, #116797.
r? `@compiler-errors`
Commit 8378487 from #114611 changed the location of an obligation
deduplication step in `opt_normalize_projection_type`. This meant that
deduplication stopped happening on one path where it was still
necessary, causing a couple of drastic performance regressions.
This commit moves the deduplication back to the old location. The good
news is that #114611 had four commits and 8378487 was of minimal
importance, so the perf benefits from that PR remain.
Fixes#116780, #116797.
Make `rustc_onunimplemented` export path agnostic
This makes it so that all the matchers that match against paths use the definition path instead of the export path. This removes all duplication around `std`/`alloc`/`core`.
This is not necessarily optimal because we now depend on internal implementation details like `core::ops::control_flow::ControlFlow`, which is not very nice and probably not acceptable for a stable `on_unimplemented`.
An alternative would be to just string-replace normalize away `alloc`/`core` to `std` as a special case, keeping the export paths but making it so that we're still fully standard library flavor agnostic.
Looking at the diff, I'm starting to think that some simple string replacement would go a long way towards fixing the problem of duplication while keeping export paths...
What do you prefer?
Also `@weiznich` for your thoughts about the stable version.
r? compiler-errors
This makes it so that all the matchers that match against paths use the
definition path instead of the export path. This removes all duplication
around `std`/`alloc`/`core`.
This is not necessarily optimal because we now depend on internal
implementation details like `core::ops::control_flow::ControlFlow`,
which is not very nice and probably not acceptable for a stable
`on_unimplemented`.
An alternative would be to just string-replace normalize away
`alloc`/`core` to `std` as a special case, keeping the export paths but
making it so that we're still fully standard library flavor agnostic.
Suggest trait bounds for used associated type on type param
Fix#101351.
When an associated type on a type parameter is used, and the type parameter isn't constrained by the correct trait, suggest the appropriate trait bound:
```
error[E0220]: associated type `Associated` not found for `T`
--> file.rs:6:15
|
6 | field: T::Associated,
| ^^^^^^^^^^ there is a similarly named associated type `Associated` in the trait `Foo`
|
help: consider restricting type parameter `T`
|
5 | struct Generic<T: Foo> {
| +++++
```
When an associated type on a type parameter has a typo, suggest fixing
it:
```
error[E0220]: associated type `Baa` not found for `T`
--> $DIR/issue-55673.rs:9:8
|
LL | T::Baa: std::fmt::Debug,
| ^^^ there is a similarly named associated type `Bar` in the trait `Foo`
|
help: change the associated type name to use `Bar` from `Foo`
|
LL | T::Bar: std::fmt::Debug,
| ~~~
```
Format all the let-chains in compiler crates
Since rust-lang/rustfmt#5910 has landed, soon we will have support for formatting let-chains (as soon as rustfmt syncs and beta gets bumped).
This PR applies the changes [from master rustfmt to rust-lang/rust eagerly](https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/out.20formatting.20of.20prs/near/374997516), so that the next beta bump does not have to deal with a 200+ file diff and can remain concerned with other things like `cfg(bootstrap)` -- #113637 was a pain to land, for example, because of let-else.
I will also add this commit to the ignore list after it has landed.
The commands that were run -- I'm not great at bash-foo, but this applies rustfmt to every compiler crate, and then reverts the two crates that should probably be formatted out-of-tree.
```
~/rustfmt $ ls -1d ~/rust/compiler/* | xargs -I@ cargo run --bin rustfmt -- `@/src/lib.rs` --config-path ~/rust --edition=2021 # format all of the compiler crates
~/rust $ git checkout HEAD -- compiler/rustc_codegen_{gcc,cranelift} # revert changes to cg-gcc and cg-clif
```
cc `@rust-lang/rustfmt`
r? `@WaffleLapkin` or `@Nilstrieb` who said they may be able to review this purely mechanical PR :>
cc `@Mark-Simulacrum` and `@petrochenkov,` who had some thoughts on the order of operations with big formatting changes in https://github.com/rust-lang/rust/pull/95262#issue-1178993801. I think the situation has changed since then, given that let-chains support exists on master rustfmt now, and I'm fairly confident that this formatting PR should land even if *bootstrap* rustfmt doesn't yet format let-chains in order to lessen the burden of the next beta bump.
When encountering method call chains of `Iterator`, check for trailing
`;` in the body of closures passed into `Iterator::map`, as well as
calls to `<T as Clone>::clone` when `T` is a type param and `T: !Clone`.
Fix#9082.
Fix#101351.
When an associated type on a type parameter is used, and the type
parameter isn't constrained by the correct trait, suggest the
appropriate trait bound:
```
error[E0220]: associated type `Associated` not found for `T`
--> file.rs:6:15
|
6 | field: T::Associated,
| ^^^^^^^^^^ there is a similarly named associated type `Associated` in the trait `Foo`
|
help: consider restricting type parameter `T`
|
5 | struct Generic<T: Foo> {
| +++++
```
When an associated type on a type parameter has a typo, suggest fixing
it:
```
error[E0220]: associated type `Baa` not found for `T`
--> $DIR/issue-55673.rs:9:8
|
LL | T::Baa: std::fmt::Debug,
| ^^^ there is a similarly named associated type `Bar` in the trait `Foo`
|
help: change the associated type name to use `Bar` from `Foo`
|
LL | T::Bar: std::fmt::Debug,
| ~~~
```
This PR fixes an issues where rustc would ignore subsequent
`#[diagnostic::on_unimplemented]` attributes. The [corresponding
RFC](https://rust-lang.github.io/rfcs/3368-diagnostic-attribute-namespace.html)
specifies that the first matching instance of each option is used.
Invalid attributes are linted and otherwise ignored.
Show more information when multiple `impl`s apply
- When there are `impl`s without type params, show only those (to avoid showing overly generic `impl`s).
```
error[E0283]: type annotations needed
--> $DIR/multiple-impl-apply.rs:34:9
|
LL | let y = x.into();
| ^ ---- type must be known at this point
|
note: multiple `impl`s satisfying `_: From<Baz>` found
--> $DIR/multiple-impl-apply.rs:14:1
|
LL | impl From<Baz> for Bar {
| ^^^^^^^^^^^^^^^^^^^^^^
...
LL | impl From<Baz> for Foo {
| ^^^^^^^^^^^^^^^^^^^^^^
= note: required for `Baz` to implement `Into<_>`
help: consider giving `y` an explicit type
|
LL | let y: /* Type */ = x.into();
| ++++++++++++
```
- Lower the importance of `T: Sized`, `T: WellFormed` and coercion errors, to prioritize more relevant errors. The pre-existing deduplication logic deals with hiding redundant errors better that way, and we show errors with more metadata that is useful to the user.
- Show `<SelfTy as Trait>::assoc_fn` suggestion in more cases.
```
error[E0790]: cannot call associated function on trait without specifying the corresponding `impl` type
--> $DIR/cross-return-site-inference.rs:38:16
|
LL | return Err(From::from("foo"));
| ^^^^^^^^^^ cannot call associated function of trait
|
help: use a fully-qualified path to a specific available implementation
|
LL | return Err(</* self type */ as From>::from("foo"));
| +++++++++++++++++++ +
```
Fix#88284.
Do not assert that hidden types don't have erased regions.
cc https://github.com/rust-lang/rust/issues/116306
`args` can have erased regions.
In the linked issue, this is reached by computing whether a large type is `Freeze` to compute its ABI.
I do not have a minimized test to include.
Remove the `TypedArena::alloc_from_iter` specialization.
It was added in #78569. It's complicated and doesn't actually help
performance.
r? `@cjgillot`
In `report_fullfillment_errors` push back `T: Sized`, `T: WellFormed`
and coercion errors to the end of the list. The pre-existing
deduplication logic eliminates redundant errors better that way, keeping
the resulting output with fewer errors than before, while also having
more detail.
a small wf and clause cleanup
- remove `Clause::from_projection_clause`, instead use `ToPredicate`
- change `predicate_obligations` to directly take a `Clause`
- remove some unnecessary `&`
- use clause in `min_specialization` checks where easily applicable
Rollup of 5 pull requests
Successful merges:
- #115863 (Add check_unused_messages in tidy)
- #116210 (Ensure that `~const` trait bounds on associated functions are in const traits or impls)
- #116358 (Rename both of the `Match` relations)
- #116371 (Remove unused features from `rustc_llvm`.)
- #116374 (Print normalized ty)
r? `@ghost`
`@rustbot` modify labels: rollup
new solver: remove provisional cache
The provisional cache is a performance optimization if there are large, interleaving cycles. Such cycles generally do not exist. It is incredibly complex and unsound in all trait solvers which have one: the old solver, chalk, and the new solver ([link](https://github.com/rust-lang/rust/blob/master/tests/ui/traits/new-solver/cycles/inductive-not-on-stack.rs)).
Given the assumption that it is not perf-critical and also incredibly complex, remove it from the new solver, only checking whether a goal is on the stack. While writing this, I uncovered two additional soundness bugs, see the inline comments for them.
r? `@compiler-errors`
