Cache local DefId-keyed queries without hashing
This caches local DefId-keyed queries using just an IndexVec. This costs ~5% extra max-rss at most but brings significant runtime improvement, up to 13% cycle counts (mean: 4%) on primary benchmarks. It's possible that further tweaks could reduce the memory overhead further but this win seems worth landing despite the increased memory, particularly with regards to eliminating the present set in non-incr or storing it inline (skip list?) with the main data.
We tried applying this scheme to all keys in the [first perf run] but found that it carried a significant memory hit (50%). instructions/cycle counts were also much more mixed, though that may have been due to the lack of the present set optimization (needed for fast iter() calls in incremental scenarios).
Closes https://github.com/rust-lang/rust/issues/45275
[first perf run]: https://perf.rust-lang.org/compare.html?start=30dfb9e046aeb878db04332c74de76e52fb7db10&end=6235575300d8e6e2cc6f449cb9048722ef43f9c7&stat=instructions:u
Simplify `closure_env_ty` and `closure_env_param`
Random cleanup that I found when working on async closures. This makes it easier to separate the latter into a new tykind.
Foreign maps are used to cache external DefIds, typically backed by
metadata decoding. In the future we might skip caching `V` there (since
loading from metadata usually is already cheap enough), but for now this
cuts down on the impact to memory usage and time to None-init a bunch of
memory. Foreign data is usually much sparser, since we're not usually
loading *all* entries from the foreign crate(s).
`OutputTypeParameterMismatch` -> `SignatureMismatch`
I'm probably missing something that made this rename more complicated. What did you end up getting stuck on when renaming this selection error, `@lcnr?`
**also** I renamed the `FulfillmentErrorCode` variants. This is just churn but I wanted to do it forever. I can move it out of this PR if desired.
r? lcnr
Inverting the condition lets us merge the two `Ok(false)` paths. I also
find the inverted condition easier to read: "all the things that must be
true for trimming to occur", instead of "any of the things that must be
true for trimming to not occur".
Suggest Upgrading Compiler for Gated Features
This PR addresses #117318
I have a few questions:
1. Do we want to specify the current version and release date of the compiler? I have added this in via environment variables, which I found in the code for the rustc cli where it handles the `--version` flag
a. How can I handle the changing message in the tests?
3. Do we want to only show this message when the compiler is old?
a. How can we determine when the compiler is old?
I'll wait until we figure out the message to bless the tests
Register even erroneous impls
Otherwise the specialization graph fails to pick it up, even though other code assumes that all impl blocks have an entry in the specialization graph.
also includes an unrelated cleanup of the specialization graph query
fixes #119827
next solver: provisional cache
this adds the cache removed in #115843. However, it should now correctly track whether a provisional result depends on an inductive or coinductive stack.
While working on this, I was using the following doc: https://hackmd.io/VsQPjW3wSTGUSlmgwrDKOA. I don't think it's too helpful to understanding this, but am somewhat hopeful that the inline comments are more useful.
There are quite a few future perf improvements here. Given that this is already very involved I don't believe it is worth it (for now). While working on this PR one of my few attempts to significantly improve perf ended up being unsound again because I was not careful enough ✨
r? `@compiler-errors`
Cleanup things in and around `Diagnostic`
These changes all arose when I was looking closely at how to simplify `DiagCtxtInner::emit_diagnostic`.
r? `@compiler-errors`
`is_force_warn` is only possible for diagnostics with `Level::Warning`,
but it is currently stored in `Diagnostic::code`, which every diagnostic
has.
This commit:
- removes the boolean `DiagnosticId::Lint::is_force_warn` field;
- adds a `ForceWarning` variant to `Level`.
Benefits:
- The common `Level::Warning` case now has no arguments, replacing
lots of `Warning(None)` occurrences.
- `rustc_session::lint::Level` and `rustc_errors::Level` are more
similar, both having `ForceWarning` and `Warning`.
In #119606 I added them and used a `_mv` suffix, but that wasn't great.
A `with_` prefix has three different existing uses.
- Constructors, e.g. `Vec::with_capacity`.
- Wrappers that provide an environment to execute some code, e.g.
`with_session_globals`.
- Consuming chaining methods, e.g. `Span::with_{lo,hi,ctxt}`.
The third case is exactly what we want, so this commit changes
`DiagnosticBuilder::foo_mv` to `DiagnosticBuilder::with_foo`.
Thanks to @compiler-errors for the suggestion.
We have `span_delayed_bug` and often pass it a `DUMMY_SP`. This commit
adds `delayed_bug`, which matches pairs like `err`/`span_err` and
`warn`/`span_warn`.
Because it takes an error code after the span. This avoids the confusing
overlap with the `DiagCtxt::struct_span_err` method, which doesn't take
an error code.
Merge dead bb pruning and unreachable bb deduplication.
Both routines share the same basic structure: iterate on all bbs to identify work, and then renumber bbs.
We can do both at once.
Support async recursive calls (as long as they have indirection)
Before #101692, we stored coroutine witness types directly inside of the coroutine. That means that a coroutine could not contain itself (as a witness field) without creating a cycle in the type representation of the coroutine, which we detected with the `OpaqueTypeExpander`, which is used to detect cycles when expanding opaque types after that are inferred to contain themselves.
After `-Zdrop-tracking-mir` was stabilized, we no longer store these generator witness fields directly, but instead behind a def-id based query. That means there is no technical obstacle in the compiler preventing coroutines from containing themselves per se, other than the fact that for a coroutine to have a non-infinite layout, it must contain itself wrapped in a layer of allocation indirection (like a `Box`).
This means that it should be valid for this code to work:
```
async fn async_fibonacci(i: u32) -> u32 {
if i == 0 || i == 1 {
i
} else {
Box::pin(async_fibonacci(i - 1)).await
+ Box::pin(async_fibonacci(i - 2)).await
}
}
```
Whereas previously, you'd need to coerce the future to `Pin<Box<dyn Future<Output = ...>>` before `await`ing it, to prevent the async's desugared coroutine from containing itself across as await point.
This PR does two things:
1. Only report an error if an opaque expansion cycle is detected *not* through coroutine witness fields.
* Instead, if we find an opaque cycle through coroutine witness fields, we compute the layout of the coroutine. If that results in a cycle error, we report it as a recursive async fn.
4. Reworks the way we report layout errors having to do with coroutines, to make up for the diagnostic regressions introduced by (1.). We actually do even better now, pointing out the call sites of the recursion!
Add helper for when we want to know if an item has a host param
r? ````@fmease```` since you're a good reviewer and no good deed goes unpunished
This helper will see far more usages as built-in traits get constified.
Improved support of collapse_debuginfo attribute for macros.
Added walk_chain_collapsed function to consider collapse_debuginfo attribute in parent macros in call chain.
Fixed collapse_debuginfo attribute processing for cranelift (there was if/else branches error swap).
cc https://github.com/rust-lang/rust/issues/100758
This works for most of its call sites. This is nice, because `emit` very
much makes sense as a consuming operation -- indeed,
`DiagnosticBuilderState` exists to ensure no diagnostic is emitted
twice, but it uses runtime checks.
For the small number of call sites where a consuming emit doesn't work,
the commit adds `DiagnosticBuilder::emit_without_consuming`. (This will
be removed in subsequent commits.)
Likewise, `emit_unless` becomes consuming. And `delay_as_bug` becomes
consuming, while `delay_as_bug_without_consuming` is added (which will
also be removed in subsequent commits.)
All this requires significant changes to `DiagnosticBuilder`'s chaining
methods. Currently `DiagnosticBuilder` method chaining uses a
non-consuming `&mut self -> &mut Self` style, which allows chaining to
be used when the chain ends in `emit()`, like so:
```
struct_err(msg).span(span).emit();
```
But it doesn't work when producing a `DiagnosticBuilder` value,
requiring this:
```
let mut err = self.struct_err(msg);
err.span(span);
err
```
This style of chaining won't work with consuming `emit` though. For
that, we need to use to a `self -> Self` style. That also would allow
`DiagnosticBuilder` production to be chained, e.g.:
```
self.struct_err(msg).span(span)
```
However, removing the `&mut self -> &mut Self` style would require that
individual modifications of a `DiagnosticBuilder` go from this:
```
err.span(span);
```
to this:
```
err = err.span(span);
```
There are *many* such places. I have a high tolerance for tedious
refactorings, but even I gave up after a long time trying to convert
them all.
Instead, this commit has it both ways: the existing `&mut self -> Self`
chaining methods are kept, and new `self -> Self` chaining methods are
added, all of which have a `_mv` suffix (short for "move"). Changes to
the existing `forward!` macro lets this happen with very little
additional boilerplate code. I chose to add the suffix to the new
chaining methods rather than the existing ones, because the number of
changes required is much smaller that way.
This doubled chainging is a bit clumsy, but I think it is worthwhile
because it allows a *lot* of good things to subsequently happen. In this
commit, there are many `mut` qualifiers removed in places where
diagnostics are emitted without being modified. In subsequent commits:
- chaining can be used more, making the code more concise;
- more use of chaining also permits the removal of redundant diagnostic
APIs like `struct_err_with_code`, which can be replaced easily with
`struct_err` + `code_mv`;
- `emit_without_diagnostic` can be removed, which simplifies a lot of
machinery, removing the need for `DiagnosticBuilderState`.
