Automatically make trivial types noop-traversable

[eggyal]

This is a second reincarnation of #108214 (or at least, what that PR ultimately became), the previous reincarnation in #117620 having been closed accidentally. It may be easier to review by commit, albeit there are quite a few of them.

Terminology

I refer to:

• folds and visits as "traversals"
• the types that are folded or visited as "traversables"
• the folders and visitors as "traversers"
• traversables on which traversers can act as "interesting" (there are currently only five: binders, types, consts, regions and predicates) and all others as "trivial" (traversals of which are no-ops)
• the TypeFoldable and TypeVisitable traits (and only these traits) as the "traversable traits"

Content

This PR:

• introduces a macro, rustc_type_ir::noop_if_trivially_traversable that uses auto-deref specialisation to:
  • perform a no-op traversal on values of type T if the interner implements rustc_type_ir::TriviallyTraverses<T> (thereby obviating all need for most trivial types to implement the traversable traits—and indeed such implementations are removed) or
  • delegate to the relevant traversable trait implementation on T otherwise;
• introduces an auto-trait, rustc_middle::ty::TriviallyTraversable, that is then "unimplemented" on the interesting types (and thereby remains implemented only on, but on all, trivial types);
• implements rustc_type_ir::TriviallyTraverses<T> on the TyCtxt<'tcx> interner for all T: rustc_middle::ty::TriviallyTraversable (thus ensuring that, for that interner at least, trivial types do not require implementations of the traversable traits);
• updates the traversable traits' derive macros to:
  • skip fields that reference neither any generic type parameters nor, if present, the 'tcx lifetime parameter
  • use the above specialisation macro when traversing fields
  • if the traversable is not parameterised by a 'tcx lifetime, generate implementations that are generic over the interner;
• replaces those derive macros' distinct helper attributes (introduced in Use derive attributes for uninteresting traversals #108040) with a unified #[skip_traversal] helper attribute that requires a justifying reason to be provided (this does mean that the different types of traversal can no longer be independently skipped, but nowhere is that currently required or ever expected to be);
  • the derive macros by default refuse to be implemented on trivial types as specialisation usually negates any need—but this can be overridden with a #[skip_traversal(impl_despite_trivial_because = "<reason>")] attribute;
• uses those derive macros in place of remaining usages of the TrivialTypeTraversal macros and some explicit implementations; and
• a few other minor relevant refactorings.

Commentary

One limitation of auto-deref specialisation is that it cannot work for unconstrained generic types. Therefore, absent real specialisation, implementations of the traversable traits must constrain their generics:

• by default, the derive macros constrain every field type T that references one or more generic type parameters to implementors of the relevant traversable trait—however this can be modified on a field (or variant) basis with a #[skip_traversal(because_trivial)] attribute so that instead the interner is constrained to implement TriviallyTraverses<T> (and the field is not traversed)
  • the constraints are applied to the field types rather than the generic type parameters to achieve a "perfect derive" that not only avoids having to propagate #[skip_traversal] annotations into wrapping types but also works with associated types and other esoteric situations—however this could result in trait solver cycles if interesting generic field types are one day involved in recursive type definitions;
  • for exceptionally rare cases where traversal of an item/variant/field should be a no-op despite it being (potentially, if generic) interesting, it can be annotated with the explicit and deliberately cumbersome #[skip_traversal(despite_potential_miscompilation_because = "<reason>")] (which produces a no-op without adding any constraint).
• (the few remaining) explicit implementations of the traversable traits over generic types are similarly constrained
  • indeed, for generic tuples all element types must implement the trait—however, since (most) trivial types no longer do, this unfortunately means that the implementations are not applicable to tuples with trivial elements and hence some such tuples have been replaced with more concrete newtypes that have appropriate constraints: c5e9447 is a particularly hefty example of this (where tuples containing Span are replaced with Spanned<T> instead).

r? @lcnr
cc @oli-obk

@rustbot label A-query-system T-compiler WG-trait-system-refactor

[rustbot]

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

Some changes occurred in engine.rs, potentially modifying the public API of ObligationCtxt.

cc @lcnr, @compiler-errors

Some changes occurred in abstract_const.rs

cc @BoxyUwU

These commits modify compiler targets.
(See the Target Tier Policy.)

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

[lcnr]

if there are conflicts, feel free to only rebase once I've reviewed it

[eggyal]

Sure. Will wait until you've reviewed before pushing anything else. You may want to try that perf run now before any conflicts arise?

[lcnr]

@bors try @rust-timer queue

[bors]

⌛ Trying commit 37bab7a with merge 793a8bf...

[bors]

☀️ Try build successful - checks-actions
Build commit: 793a8bf (793a8bf6d82bf779eead11e22a6dad04873c5342)

[lcnr]

@rust-timer build 793a8bf

[rust-timer]

Finished benchmarking commit (793a8bf): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.3%	[0.1%, 0.6%]	14
Regressions ❌ (secondary)	0.6%	[0.2%, 1.0%]	15
Improvements ✅ (primary)	-0.4%	[-0.4%, -0.4%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	[-0.4%, 0.6%]	15

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.4%, 1.8%]	10
Regressions ❌ (secondary)	4.2%	[4.2%, 4.2%]	1
Improvements ✅ (primary)	-1.0%	[-1.6%, -0.4%]	3
Improvements ✅ (secondary)	-3.4%	[-4.5%, -2.3%]	2
All ❌✅ (primary)	0.4%	[-1.6%, 1.8%]	13

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.5%	[0.4%, 0.6%]	5
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.5%	[-0.5%, -0.5%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	[-0.5%, 0.6%]	7

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 664.787s -> 664.957s (0.03%)
Artifact size: 308.65 MiB -> 308.80 MiB (0.05%)

[eggyal]

I'll try and look into the perf regression over the weekend.

One thing that I should perhaps highlight is that, in order to support safe derivation of the traversable traits upstream of rustc_middle (therefore over generic interners, absent the TriviallyTraverses impl from the auto-trait), the derive macro itself now makes an initial determination (on syntax alone) that a type is guaranteed to be trivial if it references neither generic type parameters nor a 'tcx lifetime (on the basis that without some handle on the interner, it won't contain anything of interest).

Also, since auto-deref specialisation doesn't work for generic types (generics must be constrained to use either a trivial or a non-trivial traversal), the only time that specialisation is now needed is when traversing a field that does not reference any generic type parameters but that does reference a 'tcx lifetime. It may be worth exploring how many of these are actually trivial, as if there are very few then perhaps the complexity of the specialisation machinery isn't justified? (The auto-trait would nevertheless remain to constrain impls so that #[skip_traversal] attributes cannot be unintentionally misused).

Accordingly, I think this PR could be split into a number of smaller ones; for example:

• syntactically detect & skip trivial types in the derive macro, use derive macro in trivial cases, remove superfluous impls
• (if still required, add specialisation and remove further superfluous impls)
• add auto-trait and #[skip_traversal] attribute, use derive macros in newly-supported cases
• make derivations generic over interner (where possible) and use upstream of rustc_middle
• other refactorings

This could make review easier and help to identify the source of the perf regression... let me know if you'd like me to take a crack at it.

[lcnr]

the derive macro itself now makes an initial determination (on syntax alone) that a type is guaranteed to be trivial if it references neither generic type parameters nor a 'tcx lifetime (on the basis that without some handle on the interner, it won't contain anything of interest).

People sometimes use non 'tcx lifetimes for interned lifetimes, so I would change the rule to simply be "no type or non-static lifetime params".

I think splitting the work over multiple self-contained PRs is helpful if possible. So feel free to go ahead. Will hold off reviewing this PR in that case.

[eggyal]

Presence of a 'tcx lifetime is already used to determine whether the derived impl is generic over the interner or only for TyCtxt<'tcx>. Indeed, I'm pretty sure all cases where a lifetime other than 'tcx is currently used are indeed trivial (as I don't think a previous iteration would have compiled if not)—but I'll check on that.

Will go ahead and split this monster up.

@rustbot author

[eggyal]

Okay, I have reworked this into 8 upcoming PR's—but since some build on others, I'm not sure how best to post them. Shall I open one at a time, waiting for each to merge before opening the next? Or shall I open them all at once, each based off its predecessor and therefore automatically rolling forward with master but risking confusion/excess review work if considered out of order? Or I could really pull them apart and rebase to resolve the conflicts as they arise...

[lcnr]

feel free to open them all, assigning each to me, as long as the description clearly states which other PR they depend on

[JohnCSimon]

Triage:
@eggyal I guess this PR is still active.

[Dylan-DPC]

Closing this as it is being split into parts with #117896 being the first part