Fix async closures in CTFE
First commit renames `is_coroutine_or_closure` into `is_closure_like`, because `is_coroutine_or_closure_or_coroutine_closure` seems confusing and long.
Second commit fixes some forgotten cases where we want to handle `TyKind::CoroutineClosure` the same as closures and coroutines.
The test exercises the change to `ValidityVisitor::aggregate_field_path_elem` which is the source of #120946, but not the change to `UsedParamsNeedSubstVisitor`, though I feel like it's not that big of a deal. Let me know if you'd like for me to look into constructing a test for the latter, though I have no idea what it'd look like (we can't assert against `TooGeneric` anywhere?).
Fixes#120946
r? oli-obk cc ``@RalfJung``
Warn on references casting to bigger memory layout
This PR extends the [`invalid_reference_casting`](https://doc.rust-lang.org/rustc/lints/listing/deny-by-default.html#invalid-reference-casting) lint (*deny-by-default*) which currently lint on `&T -> &mut T` casting to also lint on `&(mut) A -> &(mut) B` where `size_of::<B>() > size_of::<A>()` (bigger memory layout requirement).
The goal is to detect such cases:
```rust
let u8_ref: &u8 = &0u8;
let u64_ref: &u64 = unsafe { &*(u8_ref as *const u8 as *const u64) };
//~^ ERROR casting references to a bigger memory layout is undefined behavior
let mat3 = Mat3 { a: Vec3(0i32, 0, 0), b: Vec3(0, 0, 0), c: Vec3(0, 0, 0) };
let mat3 = unsafe { &*(&mat3 as *const _ as *const [[i64; 3]; 3]) };
//~^ ERROR casting references to a bigger memory layout is undefined behavior
```
This is added to help people who write unsafe code, especially when people have matrix struct that they cast to simple array of arrays.
EDIT: One caveat, due to the [`&Header`](https://github.com/rust-lang/unsafe-code-guidelines/issues/256) uncertainty the lint only fires when it can find the underline allocation.
~~I have manually tested all the new expressions that warn against Miri, and they all report immediate UB.~~
r? ``@est31``
pattern_analysis: track usefulness without interior mutability
Because of or-patterns, exhaustiveness needs to be able to lint if a sub-pattern is redundant, e.g. in `Some(_) | Some(true)`. So far the only sane solution I had found was interior mutability. This is a bit of an abstraction leak, and would become a footgun if we ever reused the same `DeconstructedPat`. This PR replaces interior mutability with an address-indexed hashmap, which is logically equivalent.
The check within changed from `delay_span_bug` to `delay_good_path_bug`
in #110476, and removing the check altogether was considered. It's a
very weak sanity check and gets in the way of removing good path delayed
bugs altogether, so this PR just removes it.
When encountering a tail expression in the then arm of an `if` expression
without an `else` arm, account for `async fn` and `async` blocks to
suggest `return`ing the value and pointing at the return type of the
`async fn`.
We now also account for AFIT when looking for the return type to point at.
Fix#115405.
Rollup of 11 pull requests
Successful merges:
- #120765 (Reorder diagnostics API)
- #120833 (More internal emit diagnostics cleanups)
- #120899 (Gracefully handle non-WF alias in `assemble_alias_bound_candidates_recur`)
- #120917 (Remove a bunch of dead parameters in functions)
- #120928 (Add test for recently fixed issue)
- #120933 (check_consts: fix duplicate errors, make importance consistent)
- #120936 (improve `btree_cursors` functions documentation)
- #120944 (Check that the ABI of the instance we are inlining is correct)
- #120956 (Clean inlined type alias with correct param-env)
- #120962 (Add myself to library/std review)
- #120972 (fix ICE for deref coercions with type errors)
r? `@ghost`
`@rustbot` modify labels: rollup
fix ICE for deref coercions with type errors
Follow-up to https://github.com/rust-lang/rust/pull/120895, where I made types with errors go through the full coercion code, which is necessary if we want to build MIR for bodies with errors (https://github.com/rust-lang/rust/pull/120550).
The code for coercing `&T` to `&U` currently assumes that autoderef for `&T` will succeed for at least two steps (`&T` and `T`):
b17491c8f6/compiler/rustc_hir_typeck/src/coercion.rs (L339-L464)
But for types with errors, we previously only returned the no-op autoderef step (`&{type error}` -> `&{type error}`) and then stopped early. This PR changes autoderef for types with errors to still go through the built-in derefs (e.g. `&&{type error}` -> `&{type error}` -> `{type error}`) and only stop early when it would have to go looking for `Deref` trait impls.
fixes https://github.com/rust-lang/rust/issues/120945
r? ``@compiler-errors`` or compiler
Check that the ABI of the instance we are inlining is correct
When computing the `CallSite` in the mir inliner, double check that the instance of the function that we are inlining is compatible with the signature from the trait definition that we acquire from the MIR.
Fixes#120940
r? ``@oli-obk`` or ``@cjgillot``
check_consts: fix duplicate errors, make importance consistent
This is stuff I noticed while working on https://github.com/rust-lang/rust/pull/120932, but it's orthogonal to that PR.
r? ``@oli-obk``
Remove a bunch of dead parameters in functions
Found this kind of issue when working on https://github.com/rust-lang/rust/pull/119650
I wrote a trivial toy lint and manual review to find these.
Gracefully handle non-WF alias in `assemble_alias_bound_candidates_recur`
See explanation in test. I think it's fine to delay a bug here -- I don't believe we ever construct a non-wf alias on the good path? If so, then we can just remove the delay.
Fixes#120891
r? lcnr
Reorder diagnostics API
The totally random ordering of diagnostic methods in `DiagCtxt` has been low-key driving me crazy for a while now.
r? ``@compiler-errors``
Lowering unnamed fields and anonymous adt
This implements #49804.
Goals:
- [x] lowering anonymous ADTs from AST to HIR
- [x] generating definitions of anonymous ADTs
- [x] uniqueness check of the unnamed fields
- [x] field projection of anonymous ADTs
- [x] `#[repr(C)]` check of the anonymous ADTs
Non-Goals (will be in the next PRs)
- capturing generic params for the anonymous ADTs from the parent ADT
- pattern matching of anonymous ADTs
- structural expressions of anonymous ADTs
- rustdoc support of anonymous ADTs
fix cycle error when a static and a promoted are mutually recursive
This also now allows promoteds everywhere to point to 'extern static', because why not? We still check that constants cannot transitively reach 'extern static' through references. (We allow it through raw pointers.)
r? `@oli-obk`
Fixes https://github.com/rust-lang/rust/issues/120949
This also now allows promoteds everywhere to point to 'extern static', because why not?
We still check that constants cannot transitively reach 'extern static' through references.
(We allow it through raw pointers.)
Because it also has a `DiagnosticBuilder` arg, which contains a `dcx`
reference.
Also rename some `builder` variables as `diag`, because that's the usual
name.
Now that error counts can't go up and down due to stashing/stealing, we
have a nice property:
(err_count > 0) iff (an ErrorGuaranteed has been produced)
So we can now record `ErrorGuaranteed`s within `DiagCtxt` and use that
in methods like `has_error`, instead of checking that the count is
greater than 0 and calling `unchecked_error_guaranteed` to create the
`ErrorGuaranteed`.
In fact, we can record a `Vec<ErrorGuaranteed>` and use its length to
count the number, instead of maintaining a separate count.
Avoid accessing the HIR in the happy path of `coherent_trait`
Unfortunately the hir is still used in unsafety checks, and we do not have a way to avoid that. An impl's unsafety is not part of any query other than hir.
So this PR does not affect perf, but could still be considered a cleanup
A trait's local impls are trivially coherent if there are no impls.
This avoids creating a dependency edge on the hir or the specialization graph
This may resolve part of the performance issue of https://github.com/rust-lang/rust/pull/120558
- improve diagnostics of field uniqueness check and representation check
- simplify the implementation of field uniqueness check
- remove some useless codes and improvement neatness
Encode `coroutine_for_closure` for foreign crates
Async closures (and "coroutine closures" in general) need to have their child coroutine encoded. This PR does that.
r? oli-obk
- Remove low-value comments about functionality that is obvious.
- Add missing `track_caller` attributes -- every method should have one.
- Adjust `rustc_lint_diagnostic` attributes. Every method involving a
`impl Into<DiagnosticMessage>` or `impl Into<SubdiangnosticMessage>`
argument should have one, except for those producing bugs, which
aren't user-facing.
The current order is almost perfectly random. This commit puts them into
a predictable order in their own impl block, going from the highest
level (`Block`) to the lowest (`Expect`). Within each level this is the
order:
- struct_err, err
- struct_span_err, span_err
- create_err, emit_err
The first one in each pair creates a diagnostic, the second one creates
*and* emits a diagnostic. Not every method is present for every level.
The diff is messy, but other than moving methods around, the only thing
it does is create the new `impl DiagCtxt` block with its own comment.
Assert that params with the same *index* have the same *name*
Found this bug when trying to build libcore with the new solver, since it will canonicalize two params with the same index into *different* placeholders if those params differ by name.
Fold pointer operations in GVN
This PR proposes 2 combinations of cast operations in MIR GVN:
- a chain of `PtrToPtr` or `MutToConstPointer` casts can be folded together into a single `PtrToPtr` cast;
- we attempt to evaluate more ptr ops when there is no provenance.
In particular, this allows to read from static slices.
This is not yet sufficient to see through slice operations that use `PtrComponents` (because that's a union), but still a step forward.
r? `@ghost`
Print kind of coroutine closure
Make sure that we print "async closure" when we have an async closure, rather than calling it generically a ["coroutine-closure"](https://github.com/rust-lang/rust/pull/120361).
Fixes#120886
r? oli-obk
large_assignments: Allow moves into functions
Moves into functions are typically implemented with pointer passing
rather than memcpy's at the llvm-ir level, so allow moves into
functions.
Part of the "Differentiate between Operand::Move and Operand::Copy" step of https://github.com/rust-lang/rust/issues/83518.
r? `@oli-obk` (who I think is still E-mentor?)
Suppress suggestions in derive macro
close#118809
I suppress warnings inside derive macros.
For example, the compiler emits following error by a program described in https://github.com/rust-lang/rust/issues/118809#issuecomment-1852256687 with a suggestion that indicates invalid syntax.
```
error[E0308]: `?` operator has incompatible types
--> src/main.rs:3:17
|
3 | #[derive(Debug, Deserialize)]
| ^^^^^^^^^^^ expected `u32`, found `u64`
|
= note: `?` operator cannot convert from `u64` to `u32`
= note: this error originates in the derive macro `Deserialize` (in Nightly builds, run with -Z macro-backtrace for more info)
help: you can convert a `u64` to a `u32` and panic if the converted value doesn't fit
|
3 | #[derive(Debug, Deserialize.try_into().unwrap())]
| ++++++++++++++++++++
For more information about this error, try `rustc --explain E0308`.
error: could not compile `serde_test` (bin "serde_test") due to 2 previous errors
```
In this PR, suggestions to cast are suppressed.
```
error[E0308]: `?` operator has incompatible types
--> src/main.rs:3:17
|
3 | #[derive(Debug, Deserialize)]
| ^^^^^^^^^^^ expected `u32`, found `u64`
|
= note: `?` operator cannot convert from `u64` to `u32`
= note: this error originates in the derive macro `Deserialize` (in Nightly builds, run with -Z macro-backtrace for more info)
For more information about this error, try `rustc --explain E0308`.
error: could not compile `serde_test` (bin "serde_test") due to 2 previous errors
```
Allow restricted trait impls under `#[allow_internal_unstable(min_specialization)]`
This is a follow-up to #119963 and a companion to #120866, though it can land independently from the latter.
---
We have several compiler crates that only enable `#[feature(min_specialization)]` because it is required by their expansions of `newtype_index!`, in order to implement traits marked with `#[rustc_specialization_trait]`.
This PR allows those traits to be implemented internally by macros with `#[allow_internal_unstable(min_specialization)]`, without needing specialization to be enabled in the enclosing crate.
Remove unnecessary `#![feature(min_specialization)]`
As of #119963 and #120676, we can now rely on `newtype_index!` having `#[allow_internal_unstable(min_specialization)]`, so there are a few compiler crates that no longer need to include min-spec in their own crate features.
---
Some of the expansions of `newtype_index!` still appear to require min-spec in the crate features. I think this is because `#[orderable]` causes the expansion to include an implementation of `TrustedStep`, which is flagged with `#[rustc_specialization_trait]`, and for whatever reason that isn't permitted by allow-internal-unstable. So this PR only touches the crates where that isn't the case.
Turn the "no saved object file in work product" ICE into a translatable fatal error
I don't know if it's fair to say this fixes https://github.com/rust-lang/rust/issues/120854 but it surely makes the error reporting better and should encourage people with good instincts like ```@CinchBlue.```
Loosen an assertion to account for stashed errors.
The meaning of this assertion changed in #120828 when the meaning of `has_errors` changed to exclude stashed errors. Evidently the new meaning is too restrictive.
Fixes#120856.
r? ```@oli-obk```
Remove support for `associated_type_bound` nested in `dyn` types
These necessarily desugar to `impl Trait`, which is inconsistent with the `associated_type_bound` feature after #120584.
This PR keeps the `is_in_dyn_type` hack, which kind of makes me sad. Ideally, we'd be validating that no object types have associated type bounds somewhere else. Unfortunately, we can't do this later during astconv (i think?), nor can we do it earlier during ast validation (i think?) because of the feature gating of ATB being a *warning* rather than an *error*. Let me know if you have thoughts about this.
r? lcnr
static mut: allow mutable reference to arbitrary types, not just slices and arrays
For historical reasons, we allow this:
```rust
static mut ARRAY: &'static mut [isize] = &mut [1];
```
However, we do not allow this:
```rust
static mut INT: &'static mut isize = &mut 1;
```
I think that's terribly inconsistent. I don't care much for `static mut`, but we have to keep it around for backwards compatibility and so we have to keep supporting it properly in the compiler. In recent refactors of how we deal with mutability of data in `static` and `const`, I almost made a fatal mistake since I tested `static mut INT: &'static mut isize = &mut 1` and concluded that we don't allow such `'static` mutable references even inside `static mut`. After all, nobody would expect this to be allowed only for arrays and slices, right?!?? So for the sake of our own sanity, and of whoever else reverse engineers these rules in the future to understand what the Rust compiler accepts or does not accept, I propose that we accept this for all types, not just arrays and slices.
Use `ensure` when the result of the query is not needed beyond its `Result`ness
while I would like to just remove the `tcx` methods for ensure-only queries, that is hard to do without another query annotation or by turning the `define_callbacks` macro into a proc macro to get more control
should fix perf regression of https://github.com/rust-lang/rust/pull/120558
Remove the FIXME and keep `CRATE_HIR_ID` being its own parent.
This scheme turned out to be more practical than having an `Option` on closer inspection.
Also make `hir_owner_parent` more readable.
Harmonize `AsyncFn` implementations, make async closures conditionally impl `Fn*` traits
This PR implements several changes to the built-in and libcore-provided implementations of `Fn*` and `AsyncFn*` to address two problems:
1. async closures do not implement the `Fn*` family traits, leading to breakage: https://crater-reports.s3.amazonaws.com/pr-120361/index.html
2. *references* to async closures do not implement `AsyncFn*`, as a consequence of the existing blanket impls of the shape `AsyncFn for F where F: Fn, F::Output: Future`.
In order to fix (1.), we implement `Fn` traits appropriately for async closures. It turns out that async closures can:
* always implement `FnOnce`, meaning that they're drop-in compatible with `FnOnce`-bound combinators like `Option::map`.
* conditionally implement `Fn`/`FnMut` if they have no captures, which means that existing usages of async closures should *probably* work without breakage (crater checking this: https://github.com/rust-lang/rust/pull/120712#issuecomment-1930587805).
In order to fix (2.), we make all of the built-in callables implement `AsyncFn*` via built-in impls, and instead adjust the blanket impls for `AsyncFn*` provided by libcore to match the blanket impls for `Fn*`.
These crates all needed specialization for `newtype_index!`, which will no
longer be necessary when the current nightly eventually becomes the next
bootstrap compiler.
Implementing traits marked with `#[rustc_specialization_trait]` normally
requires (min-)specialization to be enabled for the enclosing crate.
With this change, that permission can also be granted by an
`allow_internal_unstable` attribute on the macro that generates the impl.
Avoid a collection and iteration on empty passes
Just some mini optimization I saw in the wild. This way, we avoid a `collect` and `map` on an empty `passes`. Honestly, I don't even think this is big enough of a change to make a benchmark, but I'd still like to see results.
Based on [this book](https://nnethercote.github.io/perf-book/iterators.html#collect-and-extend)
For a rigid projection, recursively look at the self type's item bounds to fix the `associated_type_bounds` feature
Given a deeply nested rigid projection like `<<<T as Trait1>::Assoc1 as Trait2>::Assoc2 as Trait3>::Assoc3`, this PR adjusts both trait solvers to look at the item bounds for all of `Assoc3`, `Assoc2`, and `Assoc1` in order to satisfy a goal. We do this because the item bounds for projections may contain relevant bounds for *other* nested projections when the `associated_type_bounds` (ATB) feature is enabled. For example:
```rust
#![feature(associated_type_bounds)]
trait Trait1 {
type Assoc1: Trait2<Assoc2: Foo>;
// Item bounds for `Assoc1` are:
// `<Self as Trait1>::Assoc1: Trait2`
// `<<Self as Trait1>::Assoc1 as Trait2>::Assoc2: Foo`
}
trait Trait2 {
type Assoc2;
}
trait Foo {}
fn hello<T: Trait1>(x: <<T as Trait1>::Assoc1 as Trait2>::Assoc2) {
fn is_foo(_: impl Foo) {}
is_foo(x);
// Currently fails with:
// ERROR the trait bound `<<Self as Trait1>::Assoc1 as Trait2>::Assoc2: Foo` is not satisfied
}
```
This has been a long-standing place of brokenness for ATBs, and is also part of the reason why ATBs currently desugar so differently in various positions (i.e. sometimes desugaring to param-env bounds, sometimes desugaring to RPITs, etc). For example, in RPIT and TAIT position, `impl Foo<Bar: Baz>` currently desugars to `impl Foo<Bar = impl Baz>` because we do not currently take advantage of these nested item bounds if we desugared them into a single set of item bounds on the opaque. This is obviously both strange and unnecessary if we just take advantage of these bounds as we should.
## Approach
This PR repeatedly peels off each projection of a given goal's self type and tries to match its item bounds against a goal, repeating with the self type of the projection. This is pretty straightforward to implement in the new solver, only requiring us to loop on the self type of a rigid projection to discover inner rigid projections, and we also need to introduce an extra probe so we can normalize them.
In the old solver, we can do essentially the same thing, however we rely on the fact that projections *should* be normalized already. This is obviously not always the case -- however, in the case that they are not fully normalized, such as a projection which has both infer vars and, we bail out with ambiguity if we hit an infer var for the self type.
## Caveats
⚠️ In the old solver, this has the side-effect of actually stalling some higher-ranked trait goals of the form `for<'a> <?0 as Tr<'a>>: Tr2`. Because we stall them, they no longer are eagerly treated as error -- this cause some existing `known-bug` tests to go from fail -> pass.
I'm pretty unconvinced that this is a problem since we make code that we expect to pass in the *new* solver also pass in the *old* solver, though this obviously doesn't solve the *full* problem.
## And then also...
We also adjust the desugaring of ATB to always desugar to a regular associated bound, rather than sometimes to an impl Trait **except** for when the ATB is present in a `dyn Trait`. We need to lower `dyn Trait<Assoc: Bar>` to `dyn Trait<Assoc = impl Bar>` because object types need all of their associated types specified.
I would also be in favor of splitting out the ATB feature and/or removing support for object types in order to stabilize just the set of positions for which the ATB feature is consistent (i.e. always elaborates to a bound).
The meaning of this assertion changed in #120828 when the meaning of
`has_errors` changed to exclude stashed errors. Evidently the new
meaning is too restrictive.
Fixes#120856.
Build DebugInfo for async closures
The test is pretty bare, because I don't really know how to write debuginfo tests. I'd like to land this first, and then flesh it out correctly one it's no longer ICEing on master (which breaks people's ability to test using async closures).
r? oli-obk cc `@rust-lang/wg-debugging` (if any of y'all want to help me write a more fleshed out async closures test)
Emit more specific diagnostics when enums fail to cast with `as`
Fixes#120756
Changes this diagnostic reported in the issue:
```
error[E0605]: non-primitive cast: `Bad` as `u32`
--> src/main.rs:18:10
|
18 | dbg!(bad as u32);
| ^^^^^^^^^^ an `as` expression can only be used to convert between primitive types or to coerce to a specific trait object
```
to this:
```
error[E0605]: non-primitive cast: `Bad` as `u32`
--> src/main.rs:18:10
|
18 | dbg!(bad as u32);
| ^^^^^^^^^^ an `as` expression can be used to convert enum types to numeric types only if the enum type is unit-only or field-less
|
= note: see https://doc.rust-lang.org/reference/items/enumerations.html#casting for more information
```
This change is only for enums. The diagnostic remains unchanged for all other cases.
improve normalization of `Pointee::Metadata`
This PR makes it so that `<Wrapper<Tail> as Pointee>::Metadata` is normalized to `<Tail as Pointee>::Metadata` if we don't know `Wrapper<Tail>: Sized`. With that, the trait solver can prove projection predicates like `<Wrapper<Tail> as Pointee>::Metadata == <Tail as Pointee>::Metadata`, which makes it possible to use the metadata APIs to cast between the tail and the wrapper:
```rust
#![feature(ptr_metadata)]
use std::ptr::{self, Pointee};
fn cast_same_meta<T: ?Sized, U: ?Sized>(ptr: *const T) -> *const U
where
T: Pointee<Metadata = <U as Pointee>::Metadata>,
{
let (thin, meta) = ptr.to_raw_parts();
ptr::from_raw_parts(thin, meta)
}
struct Wrapper<T: ?Sized>(T);
fn cast_to_wrapper<T: ?Sized>(ptr: *const T) -> *const Wrapper<T> {
cast_same_meta(ptr)
}
```
Previously, this failed to compile:
```
error[E0271]: type mismatch resolving `<Wrapper<T> as Pointee>::Metadata == <T as Pointee>::Metadata`
--> src/lib.rs:16:5
|
15 | fn cast_to_wrapper<T: ?Sized>(ptr: *const T) -> *const Wrapper<T> {
| - found this type parameter
16 | cast_same_meta(ptr)
| ^^^^^^^^^^^^^^ expected `Wrapper<T>`, found type parameter `T`
|
= note: expected associated type `<Wrapper<T> as Pointee>::Metadata`
found associated type `<T as Pointee>::Metadata`
= note: an associated type was expected, but a different one was found
```
(Yes, you can already do this with `as` casts. But using functions is so much ✨ *safer* ✨, because you can't change the metadata on accident.)
---
This PR essentially changes the built-in impls of `Pointee` from this:
```rust
// before
impl Pointee for u8 {
type Metadata = ();
}
impl Pointee for [u8] {
type Metadata = usize;
}
// ...
impl Pointee for Wrapper<u8> {
type Metadata = ();
}
impl Pointee for Wrapper<[u8]> {
type Metadata = usize;
}
// ...
// This impl is only selected if `T` is a type parameter or unnormalizable projection or opaque type.
fallback impl<T: ?Sized> Pointee for Wrapper<T>
where
Wrapper<T>: Sized
{
type Metadata = ();
}
// This impl is only selected if `T` is a type parameter or unnormalizable projection or opaque type.
fallback impl<T /*: Sized */> Pointee for T {
type Metadata = ();
}
```
to this:
```rust
// after
impl Pointee for u8 {
type Metadata = ();
}
impl Pointee for [u8] {
type Metadata = usize;
}
// ...
impl<T: ?Sized> Pointee for Wrapper<T> {
// in the old solver this will instead project to the "deep" tail directly,
// e.g. `Wrapper<Wrapper<T>>::Metadata = T::Metadata`
type Metadata = <T as Pointee>::Metadata;
}
// ...
// This impl is only selected if `T` is a type parameter or unnormalizable projection or opaque type.
fallback impl<T /*: Sized */> Pointee for T {
type Metadata = ();
}
```
Fix `ErrorGuaranteed` unsoundness with stash/steal.
When you stash an error, the error count is incremented. You can then use the non-zero error count to get an `ErrorGuaranteed`. You can then steal the error, which decrements the error count. You can then cancel the error.
Example code:
```
fn unsound(dcx: &DiagCtxt) -> ErrorGuaranteed {
let sp = rustc_span::DUMMY_SP;
let k = rustc_errors::StashKey::Cycle;
dcx.struct_err("bogus").stash(sp, k); // increment error count on stash
let guar = dcx.has_errors().unwrap(); // ErrorGuaranteed from error count > 0
let err = dcx.steal_diagnostic(sp, k).unwrap(); // decrement error count on steal
err.cancel(); // cancel error
guar // ErrorGuaranteed with no error emitted!
}
```
This commit fixes the problem in the simplest way: by not counting stashed errors in `DiagCtxt::{err_count,has_errors}`.
However, just doing this without any other changes leads to over 40 ui test failures. Mostly because of uninteresting extra errors (many saying "type annotations needed" when type inference fails), and in a few cases, due to delayed bugs causing ICEs when no normal errors are printed.
To fix these, this commit adds `DiagCtxt::stashed_err_count`, and uses it in three places alongside `DiagCtxt::{has_errors,err_count}`. It's dodgy to rely on it, because unlike `DiagCtxt::err_count` it can go up and down. But it's needed to preserve existing behaviour, and at least the three places that need it are now obvious.
r? oli-obk
Fix more `ty::Error` ICEs in MIR passes
Fixes#120791 - Add a check for `ty::Error` in the `ByMove` coroutine pass
Fixes#120816 - Add a check for `ty::Error` in the MIR validator
Also a drive-by fix for a FIXME I had asked oli to add
r? oli-obk
A drive-by rewrite of `give_region_a_name()`
This drive-by rewrite makes the cache-updating nature of the method clearer, using the Entry API into the hash table for region names to capture the update-insert nature of the method. May be marginally more efficient since it only runtime-borrows and indexes the map once, but in this context the performance impact is almost certainly completely negligible.
Note that this commit should preserve all externally visible behaviour. Notably, it preserves the debug logging:
1. printing even in the case of a `None` for the new computed name, and
2. only printing on new values, begin silent on reused values
Invert diagnostic lints.
That is, change `diagnostic_outside_of_impl` and `untranslatable_diagnostic` from `allow` to `deny`, because more than half of the compiler has been converted to use translated diagnostics.
This commit removes more `deny` attributes than it adds `allow` attributes, which proves that this change is warranted.
r? ````@davidtwco````
Make privacy visitor use types more (instead of HIR)
r? ``@petrochenkov``
This is a prerequisite to normalizing projections, as otherwise we have too many invalid bound vars (hir_ty_to_ty is creating types that have bound vars, but no binder).
The commits are still chaotic, I'm gonna clean them up, but I just wanted to let you know about the general direction and wondering if we could land this before adding normalization, as normalization is where behavioral changes happen, and I'd like to keep that part as minimal as possible.
[context can be found on zulip](https://rust-lang.zulipchat.com/#narrow/stream/315482-t-compiler.2Fetc.2Fopaque-types/topic/weak.20type.20aliases.20and.20privacy)
Toggle assert_unsafe_precondition in codegen instead of expansion
The goal of this PR is to make some of the unsafe precondition checks in the standard library available in debug builds. Some UI tests are included to verify that it does that.
The diff is large, but most of it is blessing mir-opt tests and I've also split up this PR so it can be reviewed commit-by-commit.
This PR:
1. Adds a new intrinsic, `debug_assertions` which is lowered to a new MIR NullOp, and only to a constant after monomorphization
2. Rewrites `assume_unsafe_precondition` to check the new intrinsic, and be monomorphic.
3. Skips codegen of the `assume` intrinsic in unoptimized builds, because that was silly before but with these checks it's *very* silly
4. The checks with the most overhead are `ptr::read`/`ptr::write` and `NonNull::new_unchecked`. I've simply added `#[cfg(debug_assertions)]` to the checks for `ptr::read`/`ptr::write` because I was unable to come up with any (good) ideas for decreasing their impact. But for `NonNull::new_unchecked` I found that the majority of callers can use a different function, often a safe one.
Yes, this PR slows down the compile time of some programs. But in our benchmark suite it's never more than 1% icount, and the average icount change in debug-full programs is 0.22%. I think that is acceptable for such an improvement in developer experience.
https://github.com/rust-lang/rust/issues/120539#issuecomment-1922687101
When you stash an error, the error count is incremented. You can then
use the non-zero error count to get an `ErrorGuaranteed`. You can then
steal the error, which decrements the error count. You can then cancel
the error.
Example code:
```
fn unsound(dcx: &DiagCtxt) -> ErrorGuaranteed {
let sp = rustc_span::DUMMY_SP;
let k = rustc_errors::StashKey::Cycle;
dcx.struct_err("bogus").stash(sp, k); // increment error count on stash
let guar = dcx.has_errors().unwrap(); // ErrorGuaranteed from error count > 0
let err = dcx.steal_diagnostic(sp, k).unwrap(); // decrement error count on steal
err.cancel(); // cancel error
guar // ErrorGuaranteed with no error emitted!
}
```
This commit fixes the problem in the simplest way: by not counting
stashed errors in `DiagCtxt::{err_count,has_errors}`.
However, just doing this without any other changes leads to over 40 ui
test failures. Mostly because of uninteresting extra errors (many saying
"type annotations needed" when type inference fails), and in a few
cases, due to delayed bugs causing ICEs when no normal errors are
printed.
To fix these, this commit adds `DiagCtxt::stashed_err_count`, and uses
it in three places alongside `DiagCtxt::{has_errors,err_count}`. It's
dodgy to rely on it, because unlike `DiagCtxt::err_count` it can go up
and down. But it's needed to preserve existing behaviour, and at least
the three places that need it are now obvious.
Fix mir pass ICE in the presence of other errors
fixes#120779
it is impossible to add a ui test for this, because it only reproduces in build-fail, but a test that also has errors in check-fail mode can't be made build-fail 🙃
I would have to add a run-make test or sth, which is overkill for such a tiny thing imo.
Make `min_exhaustive_patterns` match `exhaustive_patterns` better
Split off from https://github.com/rust-lang/rust/pull/120742.
There remained two edge cases where `min_exhaustive_patterns` wasn't behaving like `exhaustive_patterns`. This fixes them, and tests the feature in a bunch more cases. I essentially went through all uses of `exhaustive_patterns` to see which ones would be interesting to compare between the two features.
r? `@compiler-errors`
No need to take `ImplTraitContext` by ref
We used to mutate `ImplTraitContext`, so it used to be `&mut` mutable ref. Then I think it used to have non-`Copy` data in it, so we took it by `&` ref. Now, none of that remains, so just copy it around.
Remove unused args from functions
`#[instrument]` suppresses the unused arguments from a function, *and* suppresses unused methods too! This PR removes things which are only used via `#[instrument]` calls, and fixes some other errors (privacy?) that I will comment inline.
It's possible that some of these arguments were being passed in for the purposes of being instrumented, but I am unconvinced by most of them.
Introduce `enter_forall` to supercede `instantiate_binder_with_placeholders`
r? `@lcnr`
Long term we'd like to experiment with decrementing the universe count after "exiting" binders so that we do not end up creating infer vars in non-root universes even when they logically reside in the root universe. The fact that we dont do this currently results in a number of issues in the new trait solver where we consider goals to be ambiguous because otherwise it would require lowering the universe of an infer var. i.e. the goal `?x.0 eq <T as Trait<?y.1>>::Assoc` where the alias is rigid would not be able to instantiate `?x` with the alias as there would be a universe error.
This PR is the first-ish sort of step towards being able to implement this as eventually we would want to decrement the universe in `enter_forall`. Unfortunately its Difficult to actually implement decrementing universes nicely so this is a separate step which moves us closer to the long term goal ✨
Rollup of 9 pull requests
Successful merges:
- #119592 (resolve: Unload speculatively resolved crates before freezing cstore)
- #120103 (Make it so that async-fn-in-trait is compatible with a concrete future in implementation)
- #120206 (hir: Make sure all `HirId`s have corresponding HIR `Node`s)
- #120214 (match lowering: consistently lower bindings deepest-first)
- #120688 (GVN: also turn moves into copies with projections)
- #120702 (docs: also check the inline stmt during redundant link check)
- #120727 (exhaustiveness: Prefer "`0..MAX` not covered" to "`_` not covered")
- #120734 (Add `SubdiagnosticMessageOp` as a trait alias.)
- #120739 (improve pretty printing for associated items in trait objects)
r? `@ghost`
`@rustbot` modify labels: rollup
improve pretty printing for associated items in trait objects
* Don't print a binder in front of associated items, because it's not valid syntax.
* e.g. print `dyn for<'a> Trait<'a, Assoc = &'a u8>` instead of `dyn for<'a> Trait<'a, for<'a> Assoc = &'a u8>`.
* Don't print associated items that are implied by a supertrait bound.
* e.g. if we have `trait Sub: Super<Assoc = u8> {}`, then just print `dyn Sub` instead of `dyn Sub<Assoc = u8>`.
I've added the test in the first commit, so you can see the diff of the compiler output in the second commit.
exhaustiveness: Prefer "`0..MAX` not covered" to "`_` not covered"
There was an exception when reporting integer ranges as missing, it's been there for as long as I can remember. This PR removes it. I think it's nicer to report "`0..MAX` not covered" than "`_` not covered". This also makes it consistent with enums, where we report individual enum variants in this case (as showcased in the rest of the `empty-match.rs` test).
r? ``@estebank``
match lowering: consistently lower bindings deepest-first
Currently when lowering match expressions to MIR, we do a funny little dance with the order of bindings. I attempt to explain it in the third commit: we handle refutable (i.e. needing a test) patterns differently than irrefutable ones. This leads to inconsistencies, as reported in https://github.com/rust-lang/rust/issues/120210. The reason we need a dance at all is for situations like:
```rust
fn foo1(x: NonCopyStruct) {
let y @ NonCopyStruct { copy_field: z } = x;
// the above should turn into
let z = x.copy_field;
let y = x;
}
```
Here the `y ```````@```````` binding will move out of `x`, so we need to copy the field first.
I believe that the inconsistency came about when we fixed https://github.com/rust-lang/rust/issues/69971, and didn't notice that the fix didn't extend to refutable patterns. My guess then is that ordering bindings by "deepest-first, otherwise source order" is a sound choice. This PR implements that (at least I hope, match lowering is hard to follow 🥲).
Fixes https://github.com/rust-lang/rust/issues/120210
r? ```````@oli-obk``````` since you merged the original fix to https://github.com/rust-lang/rust/issues/69971
cc ```````@matthewjasper```````
Make it so that async-fn-in-trait is compatible with a concrete future in implementation
There's no technical reason why an AFIT like `async fn foo()` cannot be satisfied with an implementation signature like `fn foo() -> Pin<Box<dyn Future<Output = ()> + 'static>>`.
We rejected this previously because we were uncertain about how AFITs worked with refinement, but I don't believe this needs to be a restriction any longer.
r? oli-obk
resolve: Unload speculatively resolved crates before freezing cstore
Name resolution sometimes loads additional crates to improve diagnostics (e.g. suggest imports).
Not all of these diagnostics result in errors, sometimes they are just warnings, like in #117772.
If additional crates loaded speculatively stay and gets listed by things like `query crates` then they may produce further errors like duplicated lang items, because lang items from speculatively loaded crates are as good as from non-speculatively loaded crates.
They can probably do things like adding unintended impls from speculatively loaded crates to method resolution as well.
The extra crates will also get into the crate's metadata as legitimate dependencies.
In this PR I remove the speculative crates from cstore when name resolution is finished and cstore is frozen.
This is better than e.g. filtering away speculative crates in `query crates` because things like `DefId`s referring to these crates and leaking to later compilation stages can produce ICEs much easier, allowing to detect them.
The unloading could potentially be skipped if any errors were reported (to allow using `DefId`s from speculatively loaded crates for recovery), but I didn't do it in this PR because I haven't seen such cases of recovery. We can reconsider later if any relevant ICEs are reported.
Unblocks https://github.com/rust-lang/rust/pull/117772.
Stop bailing out from compilation just because there were incoherent traits
fixes#120343
but also has a lot of "type annotations needed" fallout. Some are fixed in the second commit.
Reconstify `Add`
r? project-const-traits
I'm not happy with the ui test changes (or failures because I did not bless them and include the diffs in this PR). There is at least some bugs I need to look and try fix:
1. A third duplicated diagnostic when a consumer crate that does not have `effects` enabled has a trait selection error for an upstream const_trait trait. See tests/ui/ufcs/ufcs-qpath-self-mismatch.rs.
2. For some reason, making `Add` a const trait would stop us from suggesting `T: Add` when we try to add two `T`s without that bound. See tests/ui/suggestions/issue-97677.rs
```
error: equality constraints are not yet supported in `where` clauses
--> $DIR/equality-bound.rs:50:9
|
LL | IntoIterator::Item = A,
| ^^^^^^^^^^^^^^^^^^^^^^ not supported
|
= note: see issue #20041 <https://github.com/rust-lang/rust/issues/20041> for more information
help: if `IntoIterator::Item` is an associated type you're trying to set, use the associated type binding syntax
|
LL ~ fn from_iter<T: IntoIterator<Item = A>>(_: T) -> Self
LL | where
LL ~
|
error: equality constraints are not yet supported in `where` clauses
--> $DIR/equality-bound.rs:63:9
|
LL | T::Item = A,
| ^^^^^^^^^^^ not supported
|
= note: see issue #20041 <https://github.com/rust-lang/rust/issues/20041> for more information
help: if `IntoIterator::Item` is an associated type you're trying to set, use the associated type binding syntax
|
LL ~ fn from_iter<T: IntoIterator<Item = A>>(_: T) -> Self
LL | where
LL ~
|
```
Fix#68982.
Record coroutine kind in coroutine generics
Oops, added a new substitution (the "kind" ty) to coroutines but forgot to record it in the `generics_of`. I'm surprised I left this out of the coroutine-closure PR -- I thought I made this change; I possibly rebased it out by accident.
Fixes#120732
r? oli-obk
Remove some `unchecked_claim_error_was_emitted` calls
We want to drive the number of these calls down as much as possible. This PR gets rid of a bunch of them.
r? ``@oli-obk``
MirPass: make name more const
Continues #120161, this time applied to `MirPass` instead of `MirLint`, locally shaves few (very few) instructions off.
r? ``@cjgillot``
pattern_analysis: gather up place-relevant info
We track 3 things about each place during exhaustiveness: its type, its (data) validity, and whether it's the scrutinee place. This PR gathers all three into a single struct.
r? `````@compiler-errors`````
coverage: Split out counter increment sites from BCB node/edge counters
This makes it possible for two nodes/edges in the coverage graph to share the same counter, without causing the instrumentor to inject unwanted duplicate counter-increment statements.
---
````@rustbot```` label +A-code-coverage
Suggest turning `if let` into irrefutable `let` if appropriate
When encountering an `if let` tail expression without an `else` arm for an enum with a single variant, suggest writing an irrefutable `let` binding instead.
```
error[E0317]: `if` may be missing an `else` clause
--> $DIR/irrefutable-if-let-without-else.rs:8:5
|
LL | fn foo(x: Enum) -> i32 {
| --- expected `i32` because of this return type
LL | / if let Enum::Variant(value) = x {
LL | | value
LL | | }
| |_____^ expected `i32`, found `()`
|
= note: `if` expressions without `else` evaluate to `()`
= help: consider adding an `else` block that evaluates to the expected type
help: consider using an irrefutable `let` binding instead
|
LL ~ let Enum::Variant(value) = x;
LL ~ value
|
```
Fix#61788.
Mark "unused binding" suggestion as maybe incorrect
Ignoring unused bindings should be a determination made by a human, `rustfix` shouldn't auto-apply the suggested change.
Fix#54196.
various const interning cleanups
After #119044 I noticed that some things can be simplified and refactored.
This is also a requirement for https://github.com/rust-lang/rust/pull/116564 as there we'll need to treat the base allocation differently from the others
r? ````@RalfJung````
- In `emit_producing_error_guaranteed`, only allow `Level::Error`.
- In `emit_diagnostic`, only produce `ErrorGuaranteed` for `Level` and
`DelayedBug`. (Not `Bug` or `Fatal`. They don't need it, because the
relevant `emit` methods abort.)
- Add/update various comments.
When `catch_fatal_errors` catches a `FatalErrorMarker`, it returns an
`ErrorGuaranteed` that is conjured out of thin air with
`unchecked_claim_error_was_emitted`. But that `ErrorGuaranteed` is never
used.
This commit changes it to instead conjure a `FatalError` out of thin
air. (A non-deprecated action!) This makes more sense because
`FatalError` and `FatalErrorMarker` are a natural pairing -- a
`FatalErrorMarker` is created by calling `FatalError::raise`, so this is
effectively getting back the original `FatalError`.
This requires a tiny change in `catch_with_exit_code`. The old result of
the `catch_fatal_errors` call there was
`Result<Result<(), ErrorGuaranteed>, ErrorGuaranteed>` which could be
`flatten`ed into `Result<(), ErrorGuaranteed>`. The new result of the
`catch_fatal_errors` calls is
`Result<Result<(), ErrorGuaranteed>, FatalError>`, which can't be
`flatten`ed but is still easily matched for the success case.
Don't expect early-bound region to be local when reporting errors in RPITIT well-formedness
The implicit lifetime in the example code gets replaced with `ReError`, which fails a `sub_regions` check in the lexical region solver. Error reporting ends up calling `is_suitable_region` on an early bound region in the *trait* definition. This causes an ICE because we `expect_local()`.
This is kind of a bad explanation, but this code just makes diagnostics reporting a bit more gracefully fallible. If the reviewer wants a thorough investigation of exactly where we get this region outlives obligation, I can write one up. Doesn't really seem worth it, though, imo.
Fixes#120638Fixes#120648
Normalize type outlives obligations in NLL for new solver
Normalize the type outlives assumptions and obligations in MIR borrowck. This should fix any of the lazy-norm-related MIR borrowck problems.
Also some cleanups from last PR:
1. Normalize obligations in a loop in lexical region resolution
2. Use `deeply_normalize_with_skipped_universes` in lexical resolution since we may have, e.g. `for<'a> Alias<'a>: 'b`.
r? lcnr
Account for non-overlapping unmet trait bounds in suggestion
When a method not found on a type parameter could have been provided by any
of multiple traits, suggest each trait individually, instead of a single
suggestion to restrict the type parameter with *all* of them.
Before:
```
error[E0599]: the method `cmp` exists for reference `&T`, but its trait bounds were not satisfied
--> $DIR/method-on-unbounded-type-param.rs:5:10
|
LL | (&a).cmp(&b)
| ^^^ method cannot be called on `&T` due to unsatisfied trait bounds
|
= note: the following trait bounds were not satisfied:
`T: Ord`
which is required by `&T: Ord`
`&T: Iterator`
which is required by `&mut &T: Iterator`
`T: Iterator`
which is required by `&mut T: Iterator`
help: consider restricting the type parameters to satisfy the trait bounds
|
LL | fn g<T>(a: T, b: T) -> std::cmp::Ordering where T: Iterator, T: Ord {
| +++++++++++++++++++++++++
```
After:
```
error[E0599]: the method `cmp` exists for reference `&T`, but its trait bounds were not satisfied
--> $DIR/method-on-unbounded-type-param.rs:5:10
|
LL | (&a).cmp(&b)
| ^^^ method cannot be called on `&T` due to unsatisfied trait bounds
|
= note: the following trait bounds were not satisfied:
`T: Ord`
which is required by `&T: Ord`
`&T: Iterator`
which is required by `&mut &T: Iterator`
`T: Iterator`
which is required by `&mut T: Iterator`
= help: items from traits can only be used if the type parameter is bounded by the trait
help: the following traits define an item `cmp`, perhaps you need to restrict type parameter `T` with one of them:
|
LL | fn g<T: Ord>(a: T, b: T) -> std::cmp::Ordering {
| +++++
LL | fn g<T: Iterator>(a: T, b: T) -> std::cmp::Ordering {
| ++++++++++
```
Fix#108428.
Follow up to #120396, only last commit is relevant.
update indirect structural match lints to match RFC and to show up for dependencies
This is a large step towards implementing https://github.com/rust-lang/rfcs/pull/3535.
We currently have five lints related to "the structural match situation":
- nontrivial_structural_match
- indirect_structural_match
- pointer_structural_match
- const_patterns_without_partial_eq
- illegal_floating_point_literal_pattern
This PR concerns the first 3 of them. (The 4th already is set up to show for dependencies, and the 5th is removed by https://github.com/rust-lang/rust/pull/116284.) nontrivial_structural_match is being removed as per the RFC; the other two are enabled to show up in dependencies.
Fixes https://github.com/rust-lang/rust/issues/73448 by removing the affected analysis.
Account for unbounded type param receiver in suggestions
When encountering
```rust
fn f<T>(a: T, b: T) -> std::cmp::Ordering {
a.cmp(&b) //~ ERROR E0599
}
```
output
```
error[E0599]: no method named `cmp` found for type parameter `T` in the current scope
--> $DIR/method-on-unbounded-type-param.rs:2:7
|
LL | fn f<T>(a: T, b: T) -> std::cmp::Ordering {
| - method `cmp` not found for this type parameter
LL | a.cmp(&b)
| ^^^ method cannot be called on `T` due to unsatisfied trait bounds
|
= help: items from traits can only be used if the type parameter is bounded by the trait
help: the following traits define an item `cmp`, perhaps you need to restrict type parameter `T` with one of them:
|
LL | fn f<T: Ord>(a: T, b: T) -> std::cmp::Ordering {
| +++++
LL | fn f<T: Iterator>(a: T, b: T) -> std::cmp::Ordering {
| ++++++++++
```
Fix#120186.
pattern_analysis: use a plain `Vec` in `DeconstructedPat`
The use of an arena-allocated slice in `DeconstructedPat` dates to when we needed the arena anyway for lifetime reasons. Now that we don't, I'm thinking that if `thir::Pat` can use plain old `Vec`s, maybe so can I.
r? ```@ghost```
