Automatically make "boring" types noop-traversable #117620
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
|
Some changes occurred in src/tools/clippy cc @rust-lang/clippy The Miri subtree was changed cc @rust-lang/miri Some changes occurred in cc @BoxyUwU These commits modify compiler targets. Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri Some changes occurred in engine.rs, potentially modifying the public API of This PR changes MIR cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt These commits modify the If this was unintentional then you should revert the changes before this PR is merged. |
eggyal
commented
Nov 5, 2023
jackh726
reviewed
Nov 6, 2023
|
☔ The latest upstream changes (presumably #117578) made this pull request unmergeable. Please resolve the merge conflicts. |
|
will review either during the weekend or next week, until then @bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
rustbot
added
the
S-waiting-on-perf
|
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message |
rustbot
added
S-waiting-on-author
[Auto-deref specialisation] is introduced to type library traversals, the public API of which is via the `TriviallyTraverses` trait and the `noop_if_trivially_traversable` macro. Traversals invoked via the macro will then be no-ops if the interner "trivially traverses" the type being traversed *without requiring that type to implement the relevant traversable trait*. A further trait, `rustc_middle::ty::TriviallyTraversable`, is then auto-implemented for types that do not contain anything that may be of interest to traversers. A generic implementation of the former trait is then provided for the `TyCtxt` interner to indicate that it "trivially traverses" all such `TriviallyTraversable` types. This indirection is necessary because auto-traits are unstable, whereas the type library is intended to be stabilised. Finally, the traversable derive macros are updated to utilise this specialisation machinery, thereby avoiding any need for trivially traversable fields (of types for which the traits are derived) to implement the traversable traits. [Auto-deref specialisation]: http://lukaskalbertodt.github.io/2019/12/05/generalized-autoref-based-specialization.html
BindingForm has been a no-op traversable ever since it was first added in cac6126, but it was modified in 0193d1f to (optionally) contain a Place without its implementations being updated to fold/visit such. By using the derive macro to implement the visitable traits, we ensure that all contained types of interest are folded/visited.
Ever since folding was first added in c5754f3, obligations have not folded or visited their causes the ObligationCauseCode potentially containing types that may be of interest to a folder or visitor. By using the derive macro to implement the visitable traits, we ensure that all contained types of interest are folded/visited.
Rather than being concerned with internals of ExternalConstraintsData and PredefinedOpaquesData, we derive traversable impls for those types to which we then delegate.
UnevaluatedConst is not a type of any interest to folders or visitors, and therefore should not be "super visitable" at all.
Previously only enforced via comment, now via negative impls.
eggyal
force-pushed
the
automatically_make_boring_types_noop-traversable
branch
from
a416685
to
419cc6d
Compare
|
meh. Github fail. Will open another. |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Nov 9, 2023
…pes_noop-traversable, r=<try> Automatically make trivial types noop-traversable This is a second reincarnation of rust-lang#108214 (or at least, what that PR ultimately became), the previous reincarnation in rust-lang#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](http://lukaskalbertodt.github.io/2019/12/05/generalized-autoref-based-specialization.html) 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 rust-lang#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](https://smallcultfollowing.com/babysteps/blog/2022/04/12/implied-bounds-and-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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a reincarnation of #108214 (or at least, what that PR ultimately became). It may be easier to review by commit, albeit there are quite a few of them.
Terminology
I refer to:
TypeFoldableandTypeVisitabletraits (and only these traits) as the "traversable traits"Content
This PR:
rustc_type_ir::BoringTraversable, that is then "unimplemented" on the interesting types (and thereby remains implemented only on, but on all, boring types);rustc_type_ir::noop_traversal_if_boringthat uses auto-deref specialisation to:#[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);#[skip_traversal(despite_boring_because = "<reason>")]attribute;'tcxlifetime) generates implementations that are generic over the internerTrivialTypeTraversalmacros and some explicit implementations; andCommentary
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:
#[skip_traversal(because_boring)]attribute so that the types of such fields must instead implement theBoringTraversabletrait (and the field is not traversed)#[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;#[skip_traversal(despite_potential_miscompilation_because = "<reason>")](which produces a no-op without adding any constraint).Spanare replaced withSpanned<T>instead).r? @lcnr
cc @oli-obk
@rustbot label A-query-system T-compiler WG-trait-system-refactor