Add a tidy check that checks whether the fluent slugs only appear once
As ``````@Nilstrieb`````` said in https://github.com/rust-lang/rust/pull/121828#issuecomment-1972622855:
> Might make sense to have a tidy check that checks whether the fluent slugs only appear once in the source code and lint for that
there's a tidy check already for sorting
We can get the tidy check error:
```
tidy check
tidy error: /path/to/rust/compiler/rustc_const_eval/messages.ftl: message `const_eval_invalid_align` is not used
tidy error: /path/to/rust/compiler/rustc_lint/messages.ftl: message `lint_trivial_untranslatable_diag` is not used
tidy error: /path/to/rust/compiler/rustc_parse/messages.ftl: message `parse_invalid_literal_suffix` is not used
tidy error: /path/to/rust/compiler/rustc_infer/messages.ftl: message `infer_need_type_info_in_coroutine` is not used
tidy error: /path/to/rust/compiler/rustc_passes/messages.ftl: message `passes_expr_not_allowed_in_context` is not used
tidy error: /path/to/rust/compiler/rustc_passes/messages.ftl: message `passes_layout` is not used
tidy error: /path/to/rust/compiler/rustc_parse/messages.ftl: message `parse_not_supported` is not used
```
r? ``````@Nilstrieb``````
Use `ControlFlow` in visitors.
Follow up to #121256
This does have a few small behaviour changes in some diagnostic output where the visitor will now find the first match rather than the last match. The change in `find_anon_types.rs` has the only affected test. I don't see this being an issue as the last occurrence isn't any better of a choice than the first.
silence mismatched types errors for implied projections
Currently, if a trait bound is not satisfied, then we suppress any errors for the trait's supertraits not being satisfied, but still report errors for super projections not being satisfied.
For example:
```rust
trait Super {
type Assoc;
}
trait Sub: Super<Assoc = ()> {}
```
Before this PR, if `T: Sub` is not satisfied, then errors for `T: Super` are suppressed, but errors for `<T as Super>::Assoc == ()` are still shown. This PR makes it so that errors about super projections not being satisfied are also suppressed.
The errors are only suppressed if the span of the trait obligation matches the span of the super predicate obligation to avoid silencing error that are not related. This PR removes some differences between the spans of supertraits and super projections to make the suppression work correctly.
This PR fixes the majority of the diagnostics fallout when making `Thin` a supertrait of `Sized` (in a future PR).
cc https://github.com/rust-lang/rust/pull/120354#issuecomment-1930585382
cc `@lcnr`
stricter hidden type wf-check [based on #115008]
Original work by `@aliemjay` in #115008. A huge thanks to them for originally figuring out this approach ❤️
Fixes https://github.com/rust-lang/rust/issues/114728
Fixes https://github.com/rust-lang/rust/issues/114572
Instead of adding the `WellFormed` obligations when relating opaque types, we now always emit such an obligation when defining the hidden type.
This causes nested opaque types which aren't wf to error, see the comment below for the described impact. I believe this change to be desirable as it significantly reduces complexity by removing special-cases.
It also caused an issue with RPITIT: in defaulted trait methods, we add a `Projection(synthetic_assoc, rpit_of_trait_method)` clause to the `param_env`. This clause is not added to the `ParamEnv` of the nested coroutines. This caused a normalization failure in `fn check_coroutine_obligations` with the new solver. I fixed that by using the env of the typeck root instead.
r? `@oli-obk`
Rollup of 9 pull requests
Successful merges:
- #121065 (Add basic i18n guidance for `Display`)
- #121744 (Stop using Bubble in coherence and instead emulate it with an intercrate check)
- #121829 (Dummy tweaks (attempt 2))
- #121857 (Implement async closure signature deduction)
- #121894 (const_eval_select: make it safe but be careful with what we expose on stable for now)
- #122014 (Change some attributes to only_local.)
- #122016 (will_wake tests fail on Miri and that is expected)
- #122018 (only set noalias on Box with the global allocator)
- #122028 (Remove some dead code)
r? `@ghost`
`@rustbot` modify labels: rollup
Don't grab variances in `TypeRelating` relation if we're invariant
Since `Invariant.xform(var) = Invariant` always, so just copy what the generalizer relation does.
Fixes#110106
Suggest removing superfluous semicolon when statements used as expression
Fixes#105431
- it's not a pure recursive visitor, so I guess there may be some more complex scenarios not covered.
- moved `consider_removing_semicolon` to `compiler/rustc_infer` for reusing this helper function.
Improve error messages for generics with default parameters
Fixes#120785
Issue: Previously, all type parameters with default types were deliberately ignored to simplify error messages. For example, an error message for Box type would display `Box<T>` instead of `Box<T, _>`. But, this resulted in unclear error message when a concrete type was used instead of the default type.
Fix: This PR fixes it by checking if a concrete type is specified after a default type to display the entire type name or the simplified type name.
change equate for binders to not rely on subtyping
*summary by `@spastorino` and `@lcnr*`
### Context
The following code:
```rust
type One = for<'a> fn(&'a (), &'a ());
type Two = for<'a, 'b> fn(&'a (), &'b ());
mod my_api {
use std::any::Any;
use std::marker::PhantomData;
pub struct Foo<T: 'static> {
a: &'static dyn Any,
_p: PhantomData<*mut T>, // invariant, the type of the `dyn Any`
}
impl<T: 'static> Foo<T> {
pub fn deref(&self) -> &'static T {
match self.a.downcast_ref::<T>() {
None => unsafe { std::hint::unreachable_unchecked() },
Some(a) => a,
}
}
pub fn new(a: T) -> Foo<T> {
Foo::<T> {
a: Box::leak(Box::new(a)),
_p: PhantomData,
}
}
}
}
use my_api::*;
fn main() {
let foo = Foo::<One>::new((|_, _| ()) as One);
foo.deref();
let foo: Foo<Two> = foo;
foo.deref();
}
```
has UB from hitting the `unreachable_unchecked`. This happens because `TypeId::of::<One>()` is not the same as `TypeId::of::<Two>()` despite them being considered the same types by the type checker.
Currently the type checker considers binders to be equal if subtyping succeeds in both directions: `for<'a> T<'a> eq for<'b> U<'b>` holds if `for<'a> exists<'b> T<'b> <: T'<a> AND for<'b> exists<'a> T<'a> <: T<'b>` holds. This results in `for<'a> fn(&'a (), &'a ())` and `for<'a, 'b> fn(&'a (), &'b ())` being equal in the type system.
`TypeId` is computed by looking at the *structure* of a type. Even though these types are semantically equal, they have a different *structure* resulting in them having different `TypeId`. This can break invariants of unsafe code at runtime and is unsound when happening at compile time, e.g. when using const generics.
So as seen in `main`, we can assign a value of type `Foo::<One>` to a binding of type `Foo<Two>` given those are considered the same type but then when we call `deref`, it calls `downcast_ref` that relies on `TypeId` and we would hit the `None` arm as these have different `TypeId`s.
As stated in https://github.com/rust-lang/rust/issues/97156#issuecomment-1879030033, this causes the API of existing crates to be unsound.
## What should we do about this
The same type resulting in different `TypeId`s is a significant footgun, breaking a very reasonable assumptions by authors of unsafe code. It will also be unsound by itself once they are usable in generic contexts with const generics.
There are two options going forward here:
- change how the *structure* of a type is computed before relying on it. i.e. continue considering `for<'a> fn(&'a (), &'a ())` and `for<'a, 'b> fn(&'a (), &'b ())` to be equal, but normalize them to a common representation so that their `TypeId` are also the same.
- change how the semantic equality of binders to match the way we compute the structure of types. i.e. `for<'a> fn(&'a (), &'a ())` and `for<'a, 'b> fn(&'a (), &'b ())` still have different `TypeId`s but are now also considered to not be semantically equal.
---
Advantages of the first approach:
- with the second approach some higher ranked types stop being equal, even though they are subtypes of each other
General thoughts:
- changing the approach in the future will be breaking
- going from first to second may break ordinary type checking, as types which were previously equal are now distinct
- going from second to first may break coherence, because previously disjoint impls overlap as the used types are now equal
- both of these are quite unlikely. This PR did not result in any crater failures, so this should not matter too much
Advantages of the second approach:
- the soundness of the first approach requires more non-local reasoning. We have to make sure that changes to subtyping do not cause the representative computation to diverge from semantic equality
- e.g. we intend to consider higher ranked implied bounds when subtyping to [fix] https://github.com/rust-lang/rust/issues/25860, I don't know how this will interact and don't feel confident making any prediction here.
- computing a representative type is non-trivial and soundness critical, therefore adding complexity to the "core type system"
---
This PR goes with the second approach. A crater run did not result in any regressions. I am personally very hesitant about trying the first approach due to the above reasons. It feels like there are more unknowns when going that route.
### Changing the way we equate binders
Relating bound variables from different depths already results in a universe error in equate. We therefore only need to make sure that there is 1-to-1 correspondence between bound variables when relating binders. This results in concrete types being structurally equal after anonymizing their bound variables.
We implement this by instantiating one of the binder with placeholders and the other with inference variables and then equating the instantiated types. We do so in both directions.
More formally, we change the typing rules as follows:
```
for<'r0, .., 'rn> exists<'l0, .., 'ln> LHS<'l0, .., 'ln> <: RHS<'r0, .., 'rn>
for<'l0, .., 'ln> exists<'r0, .., 'rn> RHS<'r0, .., 'rn> <: LHS<'l0, .., 'ln>
--------------------------------------------------------------------------
for<'l0, .., 'ln> LHS<'l0, .., 'ln> eq for<'r0, .., 'rn> RHS<'r0, .., 'rn>
```
to
```
for<'r0, .., 'rn> exists<'l0, .., 'ln> LHS<'l0, .., 'ln> eq RHS<'r0, .., 'rn>
for<'l0, .., 'ln> exists<'r0, .., 'rn> RHS<'r0, .., 'rn> eq LHS<'l0, .., 'ln>
--------------------------------------------------------------------------
for<'l0, .., 'ln> LHS<'l0, .., 'ln> eq for<'r0, .., 'rn> RHS<'r0, .., 'rn>
```
---
Fixes#97156
r? `@lcnr`
Count stashed errors again
Stashed diagnostics are such a pain. Their "might be emitted, might not" semantics messes with lots of things.
#120828 and #121206 made some big changes to how they work, improving some things, but still leaving some problems, as seen by the issues caused by #121206. This PR aims to fix all of them by restricting them in a way that eliminates the "might be emitted, might not" semantics while still allowing 98% of their benefit. Details in the individual commit logs.
r? `@oli-obk`
Stashed errors used to be counted as errors, but could then be
cancelled, leading to `ErrorGuaranteed` soundness holes. #120828 changed
that, closing the soundness hole. But it introduced other difficulties
because you sometimes have to account for pending stashed errors when
making decisions about whether errors have occured/will occur and it's
easy to overlook these.
This commit aims for a middle ground.
- Stashed errors (not warnings) are counted immediately as emitted
errors, avoiding the possibility of forgetting to consider them.
- The ability to cancel (or downgrade) stashed errors is eliminated, by
disallowing the use of `steal_diagnostic` with errors, and introducing
the more restrictive methods `try_steal_{modify,replace}_and_emit_err`
that can be used instead.
Other things:
- `DiagnosticBuilder::stash` and `DiagCtxt::stash_diagnostic` now both
return `Option<ErrorGuaranteed>`, which enables the removal of two
`delayed_bug` calls and one `Ty::new_error_with_message` call. This is
possible because we store error guarantees in
`DiagCtxt::stashed_diagnostics`.
- Storing the guarantees also saves us having to maintain a counter.
- Calls to the `stashed_err_count` method are no longer necessary
alongside calls to `has_errors`, which is a nice simplification, and
eliminates two more `span_delayed_bug` calls and one FIXME comment.
- Tests are added for three of the four fixed PRs mentioned below.
- `issue-121108.rs`'s output improved slightly, omitting a non-useful
error message.
Fixes#121451.
Fixes#121477.
Fixes#121504.
Fixes#121508.
Opportunistically resolve regions when processing region outlives obligations
Due to the matching in `TypeOutlives` being structural, we should attempt to opportunistically resolve regions before processing region obligations. Thanks ``@lcnr`` for finding this.
r? lcnr