LLVM 18 x86 data layout update
With https://reviews.llvm.org/D86310 LLVM now has i128 aligned to 16-bytes on x86 based platforms. This will be in LLVM-18. This patch updates all our spec targets to be 16-byte aligned, and removes the alignment when speaking to older LLVM.
This results in Rust overaligning things relative to LLVM on older LLVMs.
This implements MCP https://github.com/rust-lang/compiler-team/issues/683.
See #54341
With https://reviews.llvm.org/D86310 LLVM now has i128 aligned to
16-bytes on x86 based platforms. This will be in LLVM-18. This patch
updates all our spec targets to be 16-byte aligned, and removes the
alignment when speaking to older LLVM.
This results in Rust overaligning things relative to LLVM on older LLVMs.
This alignment change was discussed in rust-lang/compiler-team#683
See #54341 for additional information about why this is happening and
where this will be useful in the future.
This *does not* stabilize `i128`/`u128` for FFI.
Get rid of the hir_owner query.
This query was meant as a firewall between `hir_owner_nodes` which is supposed to change often, and the queries that only depend on the item signature. That firewall was inefficient, leaking the contents of the HIR body through `HirId`s.
`hir_owner` incurs a significant cost, as we need to hash HIR twice in multiple modes. This PR proposes to remove it, and simplify the hashing scheme.
For the future, `def_kind`, `def_span`... are much more efficient for incremental decoupling, and should be preferred.
change `.unwrap()` to `?` on write where `fmt::Result` is returned
Fixes#120090 which points out that some of the `.unwrap()`s in `rustc_middle/src/mir/pretty.rs` are likely meant to be `?`s
Improved collapse_debuginfo attribute, added command-line flag
Improved attribute collapse_debuginfo with variants: `#[collapse_debuginfo=(no|external|yes)]`.
Added command-line flag for default behaviour.
Work-in-progress: will add more tests.
cc https://github.com/rust-lang/rust/issues/100758
Rollup of 8 pull requests
Successful merges:
- #119172 (Detect `NulInCStr` error earlier.)
- #119833 (Make tcx optional from StableMIR run macro and extend it to accept closures)
- #119967 (Add `PatKind::Err` to AST/HIR)
- #119978 (Move async closure parameters into the resultant closure's future eagerly)
- #120021 (don't store const var origins for known vars)
- #120038 (Don't create a separate "basename" when naming and opening a MIR dump file)
- #120057 (Don't ICE when deducing future output if other errors already occurred)
- #120073 (Remove spastorino from users_on_vacation)
r? `@ghost`
`@rustbot` modify labels: rollup
error on incorrect implied bounds in wfcheck except for Bevy dependents
Rebase of #109763
Additionally, special cases Bevy `ParamSet` types to not trigger the lint. This is tracked in #119956.
Fixes#109628
Don't create a separate "basename" when naming and opening a MIR dump file
These functions were split up by #77080, in order to support passing the dump file's “basename” (filename without extension) to the implementation of `-Zdump-mir-spanview`, so that it could be used as a page title.
That flag has since been removed (#119566), so now there's no particular reason for this code to handle the basename separately from the filename or full path.
This PR therefore restores things to (roughly) how they were before #77080.
Rework how diagnostic lints are stored.
`Diagnostic::code` has the type `DiagnosticId`, which has `Error` and
`Lint` variants. Plus `Diagnostic::is_lint` is a bool, which should be
redundant w.r.t. `Diagnostic::code`.
Seems simple. Except it's possible for a lint to have an error code, in
which case its `code` field is recorded as `Error`, and `is_lint` is
required to indicate that it's a lint. This is what happens with
`derive(LintDiagnostic)` lints. Which means those lints don't have a
lint name or a `has_future_breakage` field because those are stored in
the `DiagnosticId::Lint`.
It's all a bit messy and confused and seems unintentional.
This commit:
- removes `DiagnosticId`;
- changes `Diagnostic::code` to `Option<String>`, which means both
errors and lints can straightforwardly have an error code;
- changes `Diagnostic::is_lint` to `Option<IsLint>`, where `IsLint` is a
new type containing a lint name and a `has_future_breakage` bool, so
all lints can have those, error code or not.
r? `@oli-obk`
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".
`Diagnostic::code` has the type `DiagnosticId`, which has `Error` and
`Lint` variants. Plus `Diagnostic::is_lint` is a bool, which should be
redundant w.r.t. `Diagnostic::code`.
Seems simple. Except it's possible for a lint to have an error code, in
which case its `code` field is recorded as `Error`, and `is_lint` is
required to indicate that it's a lint. This is what happens with
`derive(LintDiagnostic)` lints. Which means those lints don't have a
lint name or a `has_future_breakage` field because those are stored in
the `DiagnosticId::Lint`.
It's all a bit messy and confused and seems unintentional.
This commit:
- removes `DiagnosticId`;
- changes `Diagnostic::code` to `Option<String>`, which means both
errors and lints can straightforwardly have an error code;
- changes `Diagnostic::is_lint` to `Option<IsLint>`, where `IsLint` is a
new type containing a lint name and a `has_future_breakage` bool, so
all lints can have those, error code or not.
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`.
Avoid specialization in the metadata serialization code
With the exception of a perf-only specialization for byte slices and byte vectors.
This uses the same trick of introducing a new trait and having the Encodable and Decodable derives add a bound to it as used for TyEncoder/TyDecoder. The new code is clearer about which encoder/decoder uses which impl and it reduces the dependency of rustc on specialization, making it easier to remove support for specialization entirely or turn it into a construct that is only allowed for perf optimizations if we decide to do this.
Rollup of 9 pull requests
Successful merges:
- #119208 (coverage: Hoist some complex code out of the main span refinement loop)
- #119216 (Use diagnostic namespace in stdlib)
- #119414 (bootstrap: Move -Clto= setting from Rustc::run to rustc_cargo)
- #119420 (Handle ForeignItem as TAIT scope.)
- #119468 (rustdoc-search: tighter encoding for f index)
- #119628 (remove duplicate test)
- #119638 (fix cyle error when suggesting to use associated function instead of constructor)
- #119640 (library: Fix warnings in rtstartup)
- #119642 (library: Fix a symlink test failing on Windows)
r? `@ghost`
`@rustbot` modify labels: rollup
fix cyle error when suggesting to use associated function instead of constructor
Fixes https://github.com/rust-lang/rust/issues/119625.
The first commit fixes the infinite recursion and makes the cycle error actually show up. We do this by making the `Display` for `ty::Instance` impl respect `with_no_queries` so that it can be used in query descriptions.
The second commit fixes the cycle error `resolver_for_lowering` -> `normalize` -> `resolve_instance` (for evaluating const) -> `lang_items` (for `drop_in_place`) -> `resolver_for_lowering` (for collecting lang items). We do this by simply skipping the suggestion when encountering an unnormalized type.
Rollup of 8 pull requests
Successful merges:
- #119151 (Hide foreign `#[doc(hidden)]` paths in import suggestions)
- #119350 (Imply outlives-bounds on lazy type aliases)
- #119354 (Make `negative_bounds` internal & fix some of its issues)
- #119506 (Use `resolutions(()).effective_visiblities` to avoid cycle errors in `report_object_error`)
- #119554 (Fix scoping for let chains in match guards)
- #119563 (Check yield terminator's resume type in borrowck)
- #119589 (cstore: Remove unnecessary locking from `CrateMetadata`)
- #119622 (never patterns: Document behavior of never patterns with macros-by-example)
r? `@ghost`
`@rustbot` modify labels: rollup
Check yield terminator's resume type in borrowck
In borrowck, we didn't check that the lifetimes of the `TerminatorKind::Yield`'s `resume_place` were actually compatible with the coroutine's signature. That means that the lifetimes were totally going unchecked. Whoops!
This PR implements this checking.
Fixes#119564
r? types
Fix scoping for let chains in match guards
If let guards were previously represented as a different type of guard in HIR and THIR. This meant that let chains in match guards were not handled correctly because they were treated exactly like normal guards.
- Remove `hir::Guard` and `thir::Guard`.
- Make the scoping different between normal guards and if let guards also check for let chains.
closes#118593
Hide foreign `#[doc(hidden)]` paths in import suggestions
Stops the compiler from suggesting to import foreign `#[doc(hidden)]` paths.
```@rustbot``` label A-suggestion-diagnostics
Replace a number of FxHashMaps/Sets with stable-iteration-order alternatives
This PR replaces almost all of the remaining `FxHashMap`s in query results with either `FxIndexMap` or `UnordMap`. The only case that is missing is the `EffectiveVisibilities` struct which turned out to not be straightforward to transform. Once that is done too, we can remove the `HashStable` implementation from `HashMap`.
The first commit adds the `StableCompare` trait which is a companion trait to `StableOrd`. Some types like `Symbol` can be compared in a cross-session stable way, but their `Ord` implementation is not stable. In such cases, a `StableCompare` implementation can be provided to offer a lightweight way for stable sorting. The more heavyweight option is to sort via `ToStableHashKey`, but then sorting needs to have access to a stable hashing context and `ToStableHashKey` can also be expensive as in the case of `Symbol` where it has to allocate a `String`.
The rest of the commits are rather mechanical and don't overlap, so they are best reviewed individually.
Part of [MCP 533](https://github.com/rust-lang/compiler-team/issues/533).
Remove `-Zdump-mir-spanview`
The `-Zdump-mir-spanview` flag was added back in #76074, as a development/debugging aid for the initial work on what would eventually become `-Cinstrument-coverage`. It causes the compiler to emit an HTML file containing a function's source code, with various spans highlighted based on the contents of MIR.
When the suggestion was made to [triage and remove unnecessary `-Z` flags (Zulip)](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/.60-Z.60.20option.20triage), I noted that this flag could potentially be worth removing, but I wanted to keep it around to see whether I found it useful for my own coverage work.
But when I actually tried to use it, I ran into various issues (e.g. it crashes on `tests/coverage/closure.rs`). If I can't trust it to work properly without a full overhaul, then instead of diving down a rabbit hole of trying to fix arcane span-handling bugs, it seems better to just remove this obscure old code entirely.
---
````@rustbot```` label +A-code-coverage
Tweak suggestions for bare trait used as a type
```
error[E0782]: trait objects must include the `dyn` keyword
--> $DIR/not-on-bare-trait-2021.rs:11:11
|
LL | fn bar(x: Foo) -> Foo {
| ^^^
|
help: use a generic type parameter, constrained by the trait `Foo`
|
LL | fn bar<T: Foo>(x: T) -> Foo {
| ++++++++ ~
help: you can also use `impl Foo`, but users won't be able to specify the type paramer when calling the `fn`, having to rely exclusively on type inference
|
LL | fn bar(x: impl Foo) -> Foo {
| ++++
help: alternatively, use a trait object to accept any type that implements `Foo`, accessing its methods at runtime using dynamic dispatch
|
LL | fn bar(x: &dyn Foo) -> Foo {
| ++++
error[E0782]: trait objects must include the `dyn` keyword
--> $DIR/not-on-bare-trait-2021.rs:11:19
|
LL | fn bar(x: Foo) -> Foo {
| ^^^
|
help: use `impl Foo` to return an opaque type, as long as you return a single underlying type
|
LL | fn bar(x: Foo) -> impl Foo {
| ++++
help: alternatively, you can return an owned trait object
|
LL | fn bar(x: Foo) -> Box<dyn Foo> {
| +++++++ +
```
Fix#119525:
```
error[E0038]: the trait `Ord` cannot be made into an object
--> $DIR/bare-trait-dont-suggest-dyn.rs:3:33
|
LL | fn ord_prefer_dot(s: String) -> Ord {
| ^^^ `Ord` cannot be made into an object
|
note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
--> $SRC_DIR/core/src/cmp.rs:LL:COL
|
= note: the trait cannot be made into an object because it uses `Self` as a type parameter
::: $SRC_DIR/core/src/cmp.rs:LL:COL
|
= note: the trait cannot be made into an object because it uses `Self` as a type parameter
help: consider using an opaque type instead
|
LL | fn ord_prefer_dot(s: String) -> impl Ord {
| ++++
```
Reorder check_item_type diagnostics so they occur next to the corresponding `check_well_formed` diagnostics
The first commit is just a cleanup.
The second commit moves most checks from `check_mod_item_types` into `check_well_formed`, invoking the checks in lockstep per-item instead of iterating over all items twice.
StableCompare is a companion trait to `StableOrd`. Some types like `Symbol` can be compared in a cross-session stable way, but their `Ord` implementation is not stable. In such cases, a `StableOrd` implementation can be provided to offer a lightweight way for stable sorting. (The more heavyweight option is to sort via `ToStableHashKey`, but then sorting needs to have access to a stable hashing context and `ToStableHashKey` can also be expensive as in the case of `Symbol` where it has to allocate a `String`.)
Because it's redundant w.r.t. `Diagnostic::is_lint`, which is present
for every diagnostic level.
`struct_lint_level_impl` was the only place that set the `Error` field
to `true`, and it's also the only place that calls
`Diagnostic::is_lint()` to set the `is_lint` field.
`Diagnostic` has 40 methods that return `&mut Self` and could be
considered setters. Four of them have a `set_` prefix. This doesn't seem
necessary for a type that implements the builder pattern. This commit
removes the `set_` prefixes on those four methods.
Fix `<BoundConstness as Display>`
There was infinite recursion, which is not very good. I'm not sure what the best way to implement this is, I just did something that felt right.
r? `@fmease`
This involves lots of breaking changes. There are two big changes that
force changes. The first is that the bitflag types now don't
automatically implement normal derive traits, so we need to derive them
manually.
Additionally, bitflags now have a hidden inner type by default, which
breaks our custom derives. The bitflags docs recommend using the impl
form in these cases, which I did.
Introduce `const Trait` (always-const trait bounds)
Feature `const_trait_impl` currently lacks a way to express “always const” trait bounds. This makes it impossible to define generic items like fns or structs which contain types that depend on const method calls (\*). While the final design and esp. the syntax of effects / keyword generics isn't set in stone, some version of “always const” trait bounds will very likely form a part of it. Further, their implementation is trivial thanks to the `effects` backbone.
Not sure if this needs t-lang sign-off though.
(\*):
```rs
#![feature(const_trait_impl, effects, generic_const_exprs)]
fn compute<T: const Trait>() -> Type<{ T::generate() }> { /*…*/ }
struct Store<T: const Trait>
where
Type<{ T::generate() }>:,
{
field: Type<{ T::generate() }>,
}
```
Lastly, “always const” trait bounds are a perfect fit for `generic_const_items`.
```rs
#![feature(const_trait_impl, effects, generic_const_items)]
const DEFAULT<T: const Default>: T = T::default();
```
Previously, we (oli, fee1-dead and I) wanted to reinterpret `~const Trait` as `const Trait` in generic const items which would've been quite surprising and not very generalizable.
Supersedes #117530.
---
cc `@oli-obk`
As discussed
r? fee1-dead (or compiler)
Clean up some lifetimes in `rustc_pattern_analysis`
This PR removes some redundant lifetimes. I figured out that we were shortening the lifetime of an arena-allocated `&'p DeconstructedPat<'p>` to `'a DeconstructedPat<'p>`, which forced us to carry both lifetimes when we could otherwise carry just one.
This PR also removes and elides some unnecessary lifetimes.
I also cherry-picked 0292eb9bb9b897f5c0926c6a8530877f67e7cc9b, and then simplified more lifetimes in `MatchVisitor`, which should make #119233 a very simple PR!
r? Nadrieril
rework `-Zverbose`
implements the changes described in https://github.com/rust-lang/compiler-team/issues/706
the first commit is only a name change from `-Zverbose` to `-Zverbose-internals` and does not change behavior. the second commit changes diagnostics.
possible follow up work:
- `ty::pretty` could print more info with `--verbose` than it does currently. `-Z verbose-internals` shows too much info in a way that's not helpful to users. michael had ideas about this i didn't fully understand: https://rust-lang.zulipchat.com/#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/uplift.20some.20-Zverbose.20calls.20and.20rename.20to.E2.80.A6.20compiler-team.23706/near/408984200
- `--verbose` should imply `-Z write-long-types-to-disk=no`. the code in `ty_string_with_limit` should take `--verbose` into account (apparently this affects `Ty::sort_string`, i'm not familiar with this code). writing a file to disk should suggest passing `--verbose`.
r? `@compiler-errors` cc `@estebank`
Make closures carry their own ClosureKind
Right now, we use the "`movability`" field of `hir::Closure` to distinguish a closure and a coroutine. This is paired together with the `CoroutineKind`, which is located not in the `hir::Closure`, but the `hir::Body`. This is strange and redundant.
This PR introduces `ClosureKind` with two variants -- `Closure` and `Coroutine`, which is put into `hir::Closure`. The `CoroutineKind` is thus removed from `hir::Body`, and `Option<Movability>` no longer needs to be a stand-in for "is this a closure or a coroutine".
r? eholk
Remove `DiagCtxt` API duplication
`DiagCtxt` defines the internal API for creating and emitting diagnostics: methods like `struct_err`, `struct_span_warn`, `note`, `create_fatal`, `emit_bug`. There are over 50 methods.
Some of these methods are then duplicated across several other types: `Session`, `ParseSess`, `Parser`, `ExtCtxt`, and `MirBorrowckCtxt`. `Session` duplicates the most, though half the ones it does are unused. Each duplicated method just calls forward to the corresponding method in `DiagCtxt`. So this duplication exists to (in the best case) shorten chains like `ecx.tcx.sess.parse_sess.dcx.emit_err()` to `ecx.emit_err()`.
This API duplication is ugly and has been bugging me for a while. And it's inconsistent: there's no real logic about which methods are duplicated, and the use of `#[rustc_lint_diagnostic]` and `#[track_caller]` attributes vary across the duplicates.
This PR removes the duplicated API methods and makes all diagnostic creation and emission go through `DiagCtxt`. It also adds `dcx` getter methods to several types to shorten chains. This approach scales *much* better than API duplication; indeed, the PR adds `dcx()` to numerous types that didn't have API duplication: `TyCtxt`, `LoweringCtxt`, `ConstCx`, `FnCtxt`, `TypeErrCtxt`, `InferCtxt`, `CrateLoader`, `CheckAttrVisitor`, and `Resolver`. These result in a lot of changes from `foo.tcx.sess.emit_err()` to `foo.dcx().emit_err()`. (You could do this with more types, but it gets into diminishing returns territory for types that don't emit many diagnostics.)
After all these changes, some call sites are more verbose, some are less verbose, and many are the same. The total number of lines is reduced, mostly because of the removed API duplication. And consistency is increased, because calls to `emit_err` and friends are always preceded with `.dcx()` or `.dcx`.
r? `@compiler-errors`
Unify SourceFile::name_hash and StableSourceFileId
This PR adapts the existing `StableSourceFileId` type so that it can be used instead of the `name_hash` field of `SourceFile`. This simplifies a few things that were kind of duplicated before.
The PR should also fix issues https://github.com/rust-lang/rust/issues/112700 and https://github.com/rust-lang/rust/issues/115835, but I was not able to reproduce these issues in a regression test. As far as I can tell, the root cause of these issues is that the id of the originating crate is not hashed in the `HashStable` impl of `Span` and thus cache entries that should have been considered invalidated were loaded. After this PR, the `stable_id` field of `SourceFile` includes information about the originating crate, so that ICE should not occur anymore.
Split coroutine desugaring kind from source
What a coroutine is desugared from (gen/async gen/async) should be separate from where it comes (fn/block/closure).
`IntoDiagnostic` defaults to `ErrorGuaranteed`, because errors are the
most common diagnostic level. It makes sense to do likewise for the
closely-related (and much more widely used) `DiagnosticBuilder` type,
letting us write `DiagnosticBuilder<'a, ErrorGuaranteed>` as just
`DiagnosticBuilder<'a>`. This cuts over 200 lines of code due to many
multi-line things becoming single line things.
We can just get the error level in the `match` and then use
`DiagnosticBuilder::new`. This then means a number of `DiagCtxt`
functions are no longer needed, because this was the one place that used
them.
Note: the commit changes the treatment of spans for `Expect`, which was
different to all the other cases, but this has no apparent effect.
-Znext-solver: adapt overflow rules to avoid breakage
Do not erase overflow constraints if they are from equating the impl header when normalizing[^1].
This should be the minimal change to not break crates depending on the old project behavior of "apply impl constraints while only lazily evaluating any nested goals".
Fixes https://github.com/rust-lang/trait-system-refactor-initiative/issues/70, see https://hackmd.io/ATf4hN0NRY-w2LIVgeFsVg for the reasoning behind this.
Only keeping constraints on overflow for `normalize-to` goals as that's the only thing needed for backcompat. It also allows us to not track the origin of root obligations. The issue with root goals would be something like the following:
```rust
trait Foo {}
trait Bar {}
trait FooBar {}
impl<T: Foo + Bar> FooBar for T {}
// These two should behave the same, rn we can drop constraints for both,
// but if we don't drop `Misc` goals we would only drop the constraints for
// `FooBar` unless we track origins of root obligations.
fn func1<T: Foo + Bar>() {}
fn func2<T: FooBaz>() {}
```
[^1]: mostly, the actual rules are slightly different
r? ``@compiler-errors``
Replace some instances of FxHashMap/FxHashSet with stable alternatives (mostly in rustc_hir and rustc_ast_lowering)
Part of https://github.com/rust-lang/compiler-team/issues/533. We should be getting close to being able to remove the HashStable impl of HashMap.
And make all hand-written `IntoDiagnostic` impls generic, by using
`DiagnosticBuilder::new(dcx, level, ...)` instead of e.g.
`dcx.struct_err(...)`.
This means the `create_*` functions are the source of the error level.
This change will let us remove `struct_diagnostic`.
Note: `#[rustc_lint_diagnostics]` is added to `DiagnosticBuilder::new`,
it's necessary to pass diagnostics tests now that it's used in
`into_diagnostic` functions.
Don't pass lint back out of lint decorator
Change the decorator function in the signature of the `emit_lint`/`span_lint`/etc family of methods from `impl for<'a, 'b> FnOnce(&'b mut DiagnosticBuilder<'a, ()>) -> &'b mut DiagnosticBuilder<'a, ()>` to `impl for<'a, 'b> FnOnce(&'b mut DiagnosticBuilder<'a, ()>)`. I consider it easier to read this way, especially when there's control flow involved.
r? nnethercote though feel free to reassign
Uplift `TypeAndMut` and `ClosureKind` to `rustc_type_ir`
Uplifts `TypeAndMut` and `ClosureKind`
I know I said I was just going to get rid of `TypeAndMut` (https://github.com/rust-lang/types-team/issues/124) but I think this is much simpler, lol
r? `@jackh726` or `@lcnr`
cache param env canonicalization
Canonicalize ParamEnv only once and store it. Then whenever we try to canonicalize `ParamEnvAnd<'tcx, T>` we only have to canonicalize `T` and then merge the results.
Prelimiary results show ~3-4% savings in diesel and serde benchmarks.
Best to review commits individually. Some commits have a short description.
Initial implementation had a soundness bug (https://github.com/rust-lang/rust/pull/117749#issuecomment-1840453387) due to cache invalidation:
- When canonicalizing `Ty<'?0>` we first try to resolve region variables in the current InferCtxt which may have a constraint `?0 == 'static`. This means that we register `Ty<'?0> => Canonical<Ty<'static>>` in the cache, which is obviously incorrect in another inference context.
- This is fixed by not doing region resolution when canonicalizing the query *input* (vs. response), which is the only place where ParamEnv is used, and then in a later commit we *statically* guard against any form of inference variable resolution of the cached canonical ParamEnv's.
r? `@ghost`
This doesn't change behavior.
It should prevent unintentional resolution of inference variables
during canonicalization, which previously caused a soundness bug.
See PR description for more.
