Specifically, change `Ty` from this:
```
pub type Ty<'tcx> = &'tcx TyS<'tcx>;
```
to this
```
pub struct Ty<'tcx>(Interned<'tcx, TyS<'tcx>>);
```
There are two benefits to this.
- It's now a first class type, so we can define methods on it. This
means we can move a lot of methods away from `TyS`, leaving `TyS` as a
barely-used type, which is appropriate given that it's not meant to
be used directly.
- The uniqueness requirement is now explicit, via the `Interned` type.
E.g. the pointer-based `Eq` and `Hash` comes from `Interned`, rather
than via `TyS`, which wasn't obvious at all.
Much of this commit is boring churn. The interesting changes are in
these files:
- compiler/rustc_middle/src/arena.rs
- compiler/rustc_middle/src/mir/visit.rs
- compiler/rustc_middle/src/ty/context.rs
- compiler/rustc_middle/src/ty/mod.rs
Specifically:
- Most mentions of `TyS` are removed. It's very much a dumb struct now;
`Ty` has all the smarts.
- `TyS` now has `crate` visibility instead of `pub`.
- `TyS::make_for_test` is removed in favour of the static `BOOL_TY`,
which just works better with the new structure.
- The `Eq`/`Ord`/`Hash` impls are removed from `TyS`. `Interned`s impls
of `Eq`/`Hash` now suffice. `Ord` is now partly on `Interned`
(pointer-based, for the `Equal` case) and partly on `TyS`
(contents-based, for the other cases).
- There are many tedious sigil adjustments, i.e. adding or removing `*`
or `&`. They seem to be unavoidable.
Stabilize `-Z instrument-coverage` as `-C instrument-coverage`
(Tracking issue for `instrument-coverage`: https://github.com/rust-lang/rust/issues/79121)
This PR stabilizes support for instrumentation-based code coverage, previously provided via the `-Z instrument-coverage` option. (Continue supporting `-Z instrument-coverage` for compatibility for now, but show a deprecation warning for it.)
Many, many people have tested this support, and there are numerous reports of it working as expected.
Move the documentation from the unstable book to stable rustc documentation. Update uses and documentation to use the `-C` option.
Addressing questions raised in the tracking issue:
> If/when stabilized, will the compiler flag be updated to -C instrument-coverage? (If so, the -Z variant could also be supported for some time, to ease migrations for existing users and scripts.)
This stabilization PR updates the option to `-C` and keeps the `-Z` variant to ease migration.
> The Rust coverage implementation depends on (and automatically turns on) -Z symbol-mangling-version=v0. Will stabilizing this feature depend on stabilizing v0 symbol-mangling first? If so, what is the current status and timeline?
This stabilization PR depends on https://github.com/rust-lang/rust/pull/90128 , which stabilizes `-C symbol-mangling-version=v0` (but does not change the default symbol-mangling-version).
> The Rust coverage implementation implements the latest version of LLVM's Coverage Mapping Format (version 4), which forces a dependency on LLVM 11 or later. A compiler error is generated if attempting to compile with coverage, and using an older version of LLVM.
Given that LLVM 13 has now been released, requiring LLVM 11 for coverage support seems like a reasonable requirement. If people don't have at least LLVM 11, nothing else breaks; they just can't use coverage support. Given that coverage support currently requires a nightly compiler and LLVM 11 or newer, allowing it on a stable compiler built with LLVM 11 or newer seems like an improvement.
The [tracking issue](https://github.com/rust-lang/rust/issues/79121) and the [issue label A-code-coverage](https://github.com/rust-lang/rust/labels/A-code-coverage) link to a few open issues related to `instrument-coverage`, but none of them seem like showstoppers. All of them seem like improvements and refinements we can make after stabilization.
The original `-Z instrument-coverage` support went through a compiler-team MCP at https://github.com/rust-lang/compiler-team/issues/278 . Based on that, `@pnkfelix` suggested that this needed a stabilization PR and a compiler-team FCP.
Store def_id_to_hir_id as variant in hir_owner.
If hir_owner is Owner(_), the LocalDefId is pointing to an owner, so the ItemLocalId is 0.
If the HIR node does not exist, we store Phantom.
Otherwise, we store the HirId associated to the LocalDefId.
Related to #89278
r? `@oli-obk`
Create `core::fmt::ArgumentV1` with generics instead of fn pointer
Split from (and prerequisite of) #90488, as this seems to have perf implication.
`@rustbot` label: +T-libs
If hir_owner is Owner(_), the LocalDefId is pointing to an owner, so the ItemLocalId is 0.
If the HIR node does not exist, we store Phantom.
Otherwise, we store the HirId associated to the LocalDefId.
Fix the unsoundness in the `early_otherwise_branch` mir opt pass
Closes#78496 .
This change is a significant rewrite of much of the pass. Exactly what it does is documented in the source file (with ascii art!), and all the changes that are made to the MIR that are not trivially sound are carefully documented. That being said, this is my first time touching MIR, so there are probably some invariants I did not know about that I broke.
This version of the optimization is also somewhat more flexible than the original; for example, we do not care how or where the value on which the parent is switching is computed. There is no requirement that any types be the same. This could be made even more flexible in the future by allowing a wider range of statements in the bodies of `BBC, BBD` (as long as they are all the same of course). This should be a good first step though.
Probably needs a perf run.
r? `@oli-obk` who reviewed things the last time this was touched
rustc_mir_itertools: Avoid needless `collect` with itertools
I don't think this should have measurable perf impact (at least not on perf.rlo benchmarks), it's mostly for readability.
Update some rustc dependencies to deduplicate them
This PR updates `rand` and `itertools` in rustc (not the whole workspace) in order to deduplicate them (and hopefully slightly improve compile times).
~~Currently, `object` is still duplicated, but https://github.com/rust-lang/thorin/pull/15 and updating `thorin` in the future will remove the use of version 0.27.~~ Update: Thorin 0.2 has now been released, and this PR updates `rustc_codegen_ssa` to use it and deduplicate the `object` crate.
There's a final tiny rustc dependency, `cfg-if`, which will be left: as both versions 0.1.x and 1.0 looked to be heavily depended on, they will require a few cascading updates to be removed.
Fix variant index / discriminant confusion in uninhabited enum branching
Fix confusion between variant index and variant discriminant. The pass
incorrectly assumed that for `Variants::Single` variant index is the same as
variant discriminant.
r? `@wesleywiser`
Replace `NestedVisitorMap` with generic `NestedFilter`
This is an attempt to make the `intravisit::Visitor` API simpler and "more const" with regard to nested visiting.
With this change, `intravisit::Visitor` does not visit nested things by default, unless you specify `type NestedFilter = nested_filter::OnlyBodies` (or `All`). `nested_visit_map` returns `Self::Map` instead of `NestedVisitorMap<Self::Map>`. It panics by default (unreachable if `type NestedFilter` is omitted).
One somewhat trixty thing here is that `nested_filter::{OnlyBodies, All}` live in `rustc_middle` so that they may have `type Map = map::Map` and so that `impl Visitor`s never need to specify `type Map` - it has a default of `Self::NestedFilter::Map`.
Remove deprecated LLVM-style inline assembly
The `llvm_asm!` was deprecated back in #87590 1.56.0, with intention to remove
it once `asm!` was stabilized, which already happened in #91728 1.59.0. Now it
is time to remove `llvm_asm!` to avoid continued maintenance cost.
Closes#70173.
Closes#92794.
Closes#87612.
Closes#82065.
cc `@rust-lang/wg-inline-asm`
r? `@Amanieu`
[code coverage] Fix missing dead code in modules that are never called
The issue here is that the logic used to determine which CGU to put the dead function stubs in doesn't handle cases where a module is never assigned to a CGU (which is what happens when all of the code in the module is dead).
The partitioning logic also caused issues in #85461 where inline functions were duplicated into multiple CGUs resulting in duplicate symbols.
This commit fixes the issue by removing the complex logic used to assign dead code stubs to CGUs and replaces it with a much simpler model: we pick one CGU to hold all the dead code stubs. We pick a CGU which has exported items which increases the likelihood the linker won't throw away our dead functions and we pick the smallest to minimize the impact on compilation times for crates with very large CGUs.
Fixes#91661Fixes#86177Fixes#85718Fixes#79622
r? ```@tmandry```
cc ```@richkadel```
This PR is not urgent so please don't let it interrupt your holidays! 🎄🎁
Normalize generator-local types with unevaluated constants
Normalize generator-interior types in addition to (i.e. instead of just) erasing regions, since sometimes we collect types with unevaluated const exprs.
Fixes#84737Fixes#88171Fixes#92091Fixes#92634
Probably also fixes#73114, but that one has no code I could test. It looks like it's the same issue, though.
Delay remaining `span_bug`s in drop elaboration
This follows changes from #67967 and converts remaining `span_bug`s into
delayed bugs, since for const items drop elaboration might be executed
on a MIR which failed borrowck.
Fixes#81708.
Fixes#91816.
Remove `NullOp::Box`
Follow up of #89030 and MCP rust-lang/compiler-team#460.
~1 month later nothing seems to be broken, apart from a small regression that #89332 (1aac85bb716c09304b313d69d30d74fe7e8e1a8e) shows could be regained by remvoing the diverging path, so it shall be safe to continue and remove `NullOp::Box` completely.
r? `@jonas-schievink`
`@rustbot` label T-compiler
Continue supporting -Z instrument-coverage for compatibility for now,
but show a deprecation warning for it.
Update uses and documentation to use the -C option.
Move the documentation from the unstable book to stable rustc
documentation.
CTFE eval_fn_call: use FnAbi to determine argument skipping and compatibility
This makes use of the `FnAbi` type in CTFE/Miri, which `@eddyb` has been saying for years is what we should do.^^ `FnAbi` is used to
- determine which arguments to skip (rather than the previous heuristic of skipping ZST arguments with the Rust ABI)
- impose further restrictions on whether caller and callee are consistent in how a given argument is passed
I was hoping it would also simplify the code, but that is not the case -- the previous type compatibility checks are still required (AFAIK), only the ZST skipping is gone and that took barely any code. We also need some hacks because `FnAbi` assumes a certain way of implementing `caller_location` (by passing extra arguments), but Miri can just read the caller location from the call stack so it doesn't need those arguments. (The fact that every backend has to separately implement support for these arguments seems suboptimal -- looks like this might have been better implemented on the MIR level.) To avoid having to implement those unnecessary arguments in Miri, we just compute *whether* the argument is present on the caller/callee side, but don't actually pass that argument around.
I have no idea if this looks the way `@eddyb` thinks it should look... but it makes Miri's test suite pass. ;)
One of rustc's tests fails unfortunately (`ui/const-generics/issues/issue-67739.rs`), some const generic code that is evaluated too early -- I think that should raise `TooGeneric` but instead it ICEs. My assumption is this is some FnAbi code that has not been properly tested on polymorphic code, but it might also be me calling that FnAbi code the wrong way.
r? `@oli-obk` `@eddyb`
Fixes https://github.com/rust-lang/rust/issues/56166
Miri PR at https://github.com/rust-lang/miri/pull/1928
Store a `DefId` instead of an `AdtDef` in `AggregateKind::Adt`
The `AggregateKind` enum ends up in the final mir `Body`. Currently,
any changes to `AdtDef` (regardless of how significant they are)
will legitimately cause the overall result of `optimized_mir` to change,
invalidating any codegen re-use involving that mir.
This will get worse once we start hashing the `Span` inside `FieldDef`
(which is itself contained in `AdtDef`).
To try to reduce these kinds of invalidations, this commit changes
`AggregateKind::Adt` to store just the `DefId`, instead of the full
`AdtDef`. This allows the result of `optimized_mir` to be unchanged
if the `AdtDef` changes in a way that doesn't actually affect any
of the MIR we build.
This follows changes from #67967 and converts remaining `span_bug`s into
delayed bugs, since for const items drop elaboration might be executed
on a MIR which failed borrowck.
The `AggregateKind` enum ends up in the final mir `Body`. Currently,
any changes to `AdtDef` (regardless of how significant they are)
will legitimately cause the overall result of `optimized_mir` to change,
invalidating any codegen re-use involving that mir.
This will get worse once we start hashing the `Span` inside `FieldDef`
(which is itself contained in `AdtDef`).
To try to reduce these kinds of invalidations, this commit changes
`AggregateKind::Adt` to store just the `DefId`, instead of the full
`AdtDef`. This allows the result of `optimized_mir` to be unchanged
if the `AdtDef` changes in a way that doesn't actually affect any
of the MIR we build.
The issue here is that the logic used to determine which CGU to put the
dead function stubs in doesn't handle cases where a module is never
assigned to a CGU.
The partitioning logic also caused issues in #85461 where inline
functions were duplicated into multiple CGUs resulting in duplicate
symbols.
This commit fixes the issue by removing the complex logic used to assign
dead code stubs to CGUs and replaces it with a much simplier model: we
pick one CGU to hold all the dead code stubs. We pick a CGU which has
exported items which increases the likelihood the linker won't throw
away our dead functions and we pick the smallest to minimize the impact
on compilation times for crates with very large CGUs.
Fixes#86177Fixes#85718Fixes#79622
Move generator check earlier in inlining.
Inlining into generator may create references to other generators. For instance, inlining `Pin::<&mut from_generator::GenFuture<[generator1]>>::new_unchecked` into `generator2`. This cross reference can then create cycles when computing inlining for `generator1`.
In order to avoid this kind of surprises, we forbid all inlining into generators, and rely on LLVM to do the right thing. The existing `remove-zst-query-cycle` already ICEs in inline-mir mode, so we use it as test.
Split from #91743.
Previously some code paths would fail to evaluate the rvalue, while
incorrectly indicating success with `Ok`. As a result the previous value
of lhs could have been incorrectly const propagated.
This optimization pass previously made excessive assumptions as to the nature of
the blocks being optimized. We remove those assumptions and make sure to
rigorously justify all changes that are made to the MIR. Details can be found
in the file.
Remove `in_band_lifetimes` from `rustc_mir_transform`
Like #91580, this was inspired by the conversation in #44524 about possibly removing the feature from the compiler. This crate is a heavy `'tcx` user, so is a nice case study.
r? ``@petrochenkov``
Three interesting ones:
This one had the `'tcx` declared on the function, despite the trait taking a `'tcx`:
```diff
-impl Visitor<'_> for UsedLocals {
+impl<'tcx> Visitor<'tcx> for UsedLocals {
fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) {
```
This one use in-band for one, and underscore for the other:
```diff
-pub fn remove_dead_blocks(tcx: TyCtxt<'tcx>, body: &mut Body<'_>) {
+pub fn remove_dead_blocks<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
```
A spurious name, since there's no single-use-lifetime warning:
```diff
-pub fn run_passes(tcx: TyCtxt<'tcx>, body: &'mir mut Body<'tcx>, passes: &[&dyn MirPass<'tcx>]) {
+pub fn run_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, passes: &[&dyn MirPass<'tcx>]) {
```
Address some FIXMEs left over from #91475
This shouldn't change behavior, only clarify what we're currently doing. I filed #91576 to see if the treatment of generator drop shims is intentional.
cc #91475
This one is a heavy `'tcx` user.
Two interesting ones:
This one had the `'tcx` declared on the function, despite the trait taking a `'tcx`:
```diff
-impl Visitor<'_> for UsedLocals {
+impl<'tcx> Visitor<'tcx> for UsedLocals {
fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) {
```
This one use in-band for one, and underscore for the other:
```diff
-pub fn remove_dead_blocks(tcx: TyCtxt<'tcx>, body: &mut Body<'_>) {
+pub fn remove_dead_blocks<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
```
Add a MIR pass manager (Taylor's Version)
The final draft of #91386 and #77665.
While the compile-time constraints in #91386 are cool, I decided on a more minimal approach for now. I want to explore phase constraints and maybe relative-ordering constraints in the future, though. This should preserve existing behavior **exactly** (please let me know if it doesn't) while making the following changes to the way we organize things today:
- Each `MirPhase` now corresponds to a single MIR pass. `run_passes` is not responsible for listing the correct MIR phase.
- `run_passes` no longer silently skips passes if the declared MIR phase is greater than or equal to the body's. This has bitten me multiple times. If you want this behavior, you can always branch on `body.phase` yourself.
- If your pass is solely to emit errors, you can use the `MirLint` interface instead, which gets a shared reference to `Body` instead of a mutable one. By differentiating the two, I hope to make it clearer in the short term where lints belong in the pipeline. In the long term perhaps we could enforce this at compile-time?
- MIR is no longer dumped for passes that aren't enabled, or for lints.
I tried to check that `-Zvalidate` still works correctly, since the MIR phase is now updated as soon as the associated pass is done, instead of at the end of all the passes in `run_passes`. However, it looks like `-Zvalidate` is broken with current nightlies anyways 😢 (it spits out a bunch of errors).
cc `@oli-obk` `@wesleywiser`
r? rust-lang/wg-mir-opt
Looks like Generator drop shims already have `post_borrowck_cleanup` run
on them. That's a bit surprising, since it means they're getting const-
and maybe borrow-checked? This merits further investigation, but for now
just preserve the status quo.
Move `#![feature(const_precise_live_drops)]` checks earlier in the pipeline
Should mitigate the issues found during MCP on #73255.
Once this is done, we should clean up the queries a bit, since I think `mir_drops_elaborated_and_const_checked` can be merged back into `mir_promoted`.
Fixes#90770.
cc ``@rust-lang/wg-const-eval``
r? ``@nikomatsakis`` (since they reviewed #71824)
Instead we run `RemoveFalseEdges` and `RemoveUninitDrops` at the
appropriate time. The extra `SimplifyCfg` avoids visiting unreachable
blocks during `RemoveUninitDrops`.
Otherwise dataflow state will propagate along false edges and cause
things to be marked as maybe init unnecessarily. These should be
separate, since `SimplifyBranches` also makes `if true {} else {}` into
a `goto`, which means we wouldn't lint anything in the `else` block.
Previously for enums using the `Variants::Single` layout, the variant
index was being confused with its discriminant. For example, in the case
of `enum E { A = 1 }`.
Use `discriminant_for_variant` to avoid the issue.
The change is limited to the iteration over indices instead of using
`basic_blocks_mut()` directly, in the case the previous implementation
intentionally avoided invalidating the caches stored in MIR body.
Remove hir::map::blocks and use FnKind instead
The principal tool is `FnLikeNode`, which is not often used and can be easily implemented using `rustc_hir::intravisit::FnKind`.
Adopt let_else across the compiler
This performs a substitution of code following the pattern:
```
let <id> = if let <pat> = ... { identity } else { ... : ! };
```
To simplify it to:
```
let <pat> = ... { identity } else { ... : ! };
```
By adopting the `let_else` feature (cc #87335).
The PR also updates the syn crate because the currently used version of the crate doesn't support `let_else` syntax yet.
Note: Generally I'm the person who *removes* usages of unstable features from the compiler, not adds more usages of them, but in this instance I think it hopefully helps the feature get stabilized sooner and in a better state. I have written a [comment](https://github.com/rust-lang/rust/issues/87335#issuecomment-944846205) on the tracking issue about my experience and what I feel could be improved before stabilization of `let_else`.
Index and hash HIR as part of lowering
Part of https://github.com/rust-lang/rust/pull/88186
~Based on https://github.com/rust-lang/rust/pull/88880 (see merge commit).~
Once HIR is lowered, it is later indexed by the `index_hir` query and hashed for `crate_hash`. This PR moves those post-processing steps to lowering itself. As a side objective, the HIR crate data structure is refactored as an `IndexVec<LocalDefId, Option<OwnerInfo<'hir>>>` where `OwnerInfo` stores all the relevant information for an HIR owner.
r? `@michaelwoerister`
cc `@petrochenkov`
