[style] rustfmt `match`es with comments in or-patterns
Using https://github.com/rust-lang/rustfmt/pull/6893, I reformatted the whole codebase. The result is that `match`es that *should have* been formatted under normal circumstances but are getting skipped now got their expected format. These match expressions were being entirely skipped because they contain or-patterns with comments in between patterns, causing rustfmt to bail out entirely. The or-patterns with comments themselves remain untouched, but now the match arm bodies and other patterns without comments do get formatted under that PR.
Because the fix in rustfmt isn't landed yet, I reworked some of the or-patterns with comments so that formatting doesn't regress. Tried doing this only in larger blocks that are more likely to regress in the meantime.
(Introduced and) removed a bunch of stray backticks \` likely left after an editor autoclosed the intended closing \`, resulting in <code>\`name\`\`</code> in comments.
When an unconstrained type or lifetime parameter is detected in an
`impl`, provide more specific help based on its usage:
- If the parameter is entirely unused, suggest removing it.
- If it is used in the `impl` body but not the `Self` type, suggest
including it in the `Self` type and the struct definition.
This also adds a comprehensive UI test for these cases.
Add better default spans for the `Ty` impl of `QueryKey`
This add default spans for `Ty` in its implementation of `QueryKey`. This is useful so we can get spans for the `layout_of` query which shows up in cycle errors.
Improve caching by introducing `TypingMode::ErasedNotCoherence`
r? @lcnr
This introduces `TypingMode::ErasedNotCoherence`. Most typing modes contain a list of opaque types, which are quite often unused during canonicalization. With this change, any time we try canonicalization, we replace whichever typing mode we're currently in with `ErasedNotcoherence`, attempt to canonicalize, and if that fails *retry* in the original typing mode. If erased mode succeeds, this is beneficial because that way the opaque types don't end up in the cache key, allowing more cache reuse.
This seems to have a small (0.5%) slowdown on most programs, but a dramatic (>60%) speedup in specific cases like the rustc-perf `wg-grammar` benchmark. Some more improvements are expected with "eager normalization", which is work that's under way right now.
refactor rustc_on_unimplemented's filtering
Previously when you had a
```rust
pub struct Directive {
pub is_rustc_attr: bool,
pub condition: Option<OnUnimplementedCondition>,
pub subcommands: ThinVec<Directive>,
pub message: Option<(Span, FormatString)>,
...
}
```
that condition would control the emission of the message, label, notes etc. I've changed that to
```rust
pub struct Directive {
pub is_rustc_attr: bool,
pub filters: ThinVec<(Filter, Directive)>,
pub message: Option<(Span, FormatString)>,
...
```
so that the message etc is always emitted, and there's a vec of tuples with (filter, directive) where the filter controls whether that directive is even emitted, which i think is much clearer. That also makes it easier to not have to do the reverse iteration thing and this makes it so that notes are emitted in declaration order (with nonfiltered options always last).
The rename is because I plan on making it available to other diagnostic attributes at some point (very wip) so `OnUnimplementedCondition` and the like would have to be renamed anyway.
Suggest `[const] Trait` bounds in more places
Right now we have some special logic in the const checker for emitting `[const] Trait` suggestions, but I'm trying to handle that similarly to how it is handled for normal `Trait` clauses. This is just a small step in how it will look on the UX side, which should make my follow-up PRs affect tests less and just be a refactoring
Fix pathological performance in trait solver cycles with errors
Fuchsia's Starnix system has had a multi-year long bug where occasionally a typo could cause the rust compiler to take 10+ hours to report an error (see rust-lang/rust#136516 and rust-lang/rust#150907). This was particularly hard to trace down since Starnix's codebase is massive, over 384 thousand lines as of writing.
With the help of treereduce, cargo-minimize, and rustmerge, after about a month of running we reduced it down to a couple [lines of code], which only takes about 35 seconds to report an error on my machine. The bug also appears to happen with `-Z next-solver=no` and `-Z next-solver=coherence`, but does not occur with `-Z next-solver` or `-Z next-solver=globally`.
I used Gemini to help diagnose the problem and proposed solution (which is the one proposed in this patch):
1. The trait solver gets stuck in an exponential loop evaluating auto-trait bounds (like Send and Sync) on cyclic types that contain compilation errors (TyKind::Error).
2. Normally, if the solver detects a cycle, it prevents the result from being stored in the Global Cache because the result depends on the current evaluation stack. However, when an error is involved, the depth tracking gets pinned to a low value, forcing the solver to rely on the short-lived Provisional Cache. Since the provisional cache is cleared between high-level iterations of the fulfillment loop, the solver ends up re-discovering and re-evaluating the same large cycle thousands of times.
3. Allow global caching of results even if they appear stack-dependent, provided that the inference context is already "tainted by errors" (`self.infcx.tainted_by_errors().is_some()`). This violates the strict invariant that global cache entries shouldn't depend on the stack, but it is safe because the compilation is already guaranteed to fail due to the presence of errors. Prioritizing compiler responsiveness and termination over perfect correctness in error states is the correct trade-off here.
I added the reduction as the test case for this. However, I don't see an easy way to catch if this bug comes back. Should we add some way to timeout the test if it takes longer than 10 seconds to compile? That could be a source of flakes though.
I don't have any experience with the trait solver code, but I did try to review the code to the best of my ability. This approach seems a bit of a bandaid to the solution, but I don't see a better solution. We could try to teach the solver to not clear the provisional cache in this circumstance, but I suspect that'd be a pretty invasive change.
I'm guessing if this does cause problems, it might report an incorrect error, but I (and Gemini) were unable to come up with an example that reported a different error with and without this fix.
Resolvesrust-lang/rust#150907
[lines of code]: https://gist.github.com/erickt/255bc4006292cac88de906bd6bd9220a
Change keyword order for `impl` restrictions
Based on rust-lang/rust#155222, this PR reorders keywords in trait definitions to group restrictions with visibility. It changes the order from `pub(...) const unsafe auto impl(...) trait Foo {...}` to `pub(...) impl(...) const unsafe auto trait Foo {...}`.
Tracking issue for restrictions: rust-lang/rust#105077
r? @Urgau
cc @jhpratt
Fix ICE when const closure appears inside a non-const trait method
Fixesrust-lang/rust#153891
`hir_body_const_context()` unconditionally delegated to the parent's const context for const closures, returning `None` when the parent had no const context. This caused `mir_const_qualif()` to hit a `span_bug!`, since `mir_promoted()` had already decided to call it based on the closure's own syntactic constness. Fall back to `ConstContext::ConstFn` when the parent's const context is `None`, so that the const closure body is still properly const-checked rather than triggering an ICE.
Examining [another attempt](https://github.com/rust-lang/rust/pull/153900/changes) at this issue (which has already been closed), I thought that its approach represents a workaround fix to avoid inconsistencies in the caller of `mir_promoted()`, whereas I think the correct behavior is for `hir_body_const_context()` itself to return the proper value.
Rollup of 7 pull requests
Successful merges:
- rust-lang/rust#155589 (Forbid `check-pass`/`build-pass`/`run-pass` directives in incremental tests)
- rust-lang/rust#155610 (Add missing `dyn` keyword to `trait_alias` page of the Unstable Book)
- rust-lang/rust#155615 (test cleanups for `ui/derives` and `ui/deriving`)
- rust-lang/rust#154874 (Fix ICE for inherited const conditions on const closures)
- rust-lang/rust#155605 (std: Update support for `wasm32-wasip3`)
- rust-lang/rust#155613 (c-variadic: tweak `std` docs)
- rust-lang/rust#155619 (Remove a bunch of unnecessary explicit lifetimes from the ast validator)
Fix ICE for inherited const conditions on const closures
Synchronize `evaluate_host_effect_for_fn_goal` with the behavior of `extract_fn_def_from_const_callable` in new solver.
Closesrust-lang/rust#153861 .
Add Sized supertrait for CoerceUnsized and DispatchFromDyn
This is part of rust-lang/rust#149094. I did not include `Receiver` because I think it is correct to allow non-sized receivers.
Fuchsia's Starnix system has had a multi-year long bug where
occasionally a typo could cause the rust compiler to take 10+ hours to
report an error. This was particularly hard to trace down since
Starnix's codebase is massive, over 384 thousand lines as of writing.
With the help of treereduce, cargo-minimize, and rustmerge, after about
a month of running we reduced it down to a couple [lines of code], which
only takes about 35 seconds to report an error on my machine. The bug
also appears to happen with `-Z next-solver=no` and `-Z
next-solver=coherence`, but does not occur with `-Z next-solver` or `-Z
next-solver=globally`.
I used Gemini to help diagnose the problem and proposed solution (which
is the one proposed in this patch):
1. The trait solver gets stuck in an exponential loop evaluating
auto-trait bounds (like Send and Sync) on cyclic types that contain
compilation errors (TyKind::Error).
2. Normally, if the solver detects a cycle, it prevents the result from
being stored in the Global Cache because the result depends on the
current evaluation stack. However, when an error is involved, the
depth tracking gets pinned to a low value, forcing the solver to rely
on the short-lived Provisional Cache. Since the provisional cache is
cleared between high-level iterations of the fulfillment loop, the
solver ends up re-discovering and re-evaluating the same large cycle
thousands of times.
3. Allow global caching of results even if they appear stack-dependent,
provided that the inference context is already "tainted by errors"
(`self.infcx.tainted_by_errors().is_some()`). This violates the
strict invariant that global cache entries shouldn't depend on the
stack, but it is safe because the compilation is already guaranteed
to fail due to the presence of errors. Prioritizing compiler
responsiveness and termination over perfect correctness in error
states is the correct trade-off here.
I added the reduction as the test case for this. However, I don't see an
easy way to catch if this bug comes back. Should we add some way to
timeout the test if it takes longer than 10 seconds to compile? That
could be a source of flakes though.
I don't have any experience with the trait solver code, but I did try to
review the code to the best of my ability. This approach seems a bit of
a bandaid to the solution, but I don't see a better solution. We could
try to teach the solver to not clear the provisional cache in this
circumstance, but I suspect that'd be a pretty invasive change.
I'm guessing if this does cause problems, it might report an incorrect
error, but I (and Gemini) were unable to come up with an example that
reported a different error with and without this fix.
[lines of code]: https://gist.github.com/erickt/255bc4006292cac88de906bd6bd9220a
bors