Use derive attributes for uninteresting traversals#108040
Merged
bors merged 1 commit intorust-lang:masterfrom Feb 15, 2023
Merged
Use derive attributes for uninteresting traversals#108040bors merged 1 commit intorust-lang:masterfrom
bors merged 1 commit intorust-lang:masterfrom
Conversation
oli-obk
requested changes
Feb 14, 2023
Contributor
oli-obk
left a comment
There was a problem hiding this comment.
These attributes should be documented. On the trait maybe? not sure where we document the derives.
Contributor
Author
|
We can document the macro. |
3a83e62 to
f394c13
Compare
Contributor
|
Great thanks! @bors r+ rollup |
Collaborator
|
📌 Commit f394c1332119cef1919795af7f1abe0460f793c0 has been approved by It is now in the queue for this repository. |
f394c13 to
3b510e8
Compare
Contributor
Author
|
Apologies, it failed tidy check. Have re-pushed. @bors r- |
Contributor
|
@bors r+ rollup |
Collaborator
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Feb 14, 2023
…g_traversals, r=oli-obk Use derive attributes for uninteresting traversals It appears that visiting and folding was implemented on `BitMatrix` solely so that the derive macros could be used on `GeneratorLayout`, however such implementation would not necessarily be correct for other uses (if there were any). Adding attributes to the derive macro is more correct and potentially more generally useful. r? `@oli-obk`
This was referenced Feb 14, 2023
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 14, 2023
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#107573 (Update the minimum external LLVM to 14) - rust-lang#107626 (Fix `x fix` on the standard library itself) - rust-lang#107673 (update ICU4X to 1.1.0) - rust-lang#107733 (Store metrics from `metrics.json` to CI PGO timer) - rust-lang#108007 (Use `is_str` instead of string kind comparison) - rust-lang#108033 (add an unstable `#[rustc_coinductive]` attribute) - rust-lang#108039 (Refactor refcounted structural_impls via functors) - rust-lang#108040 (Use derive attributes for uninteresting traversals) - rust-lang#108044 (interpret: rename Pointer::from_addr → from_addr_invalid) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
3 tasks
Jarcho
pushed a commit
to Jarcho/rust
that referenced
this pull request
Feb 26, 2023
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#107573 (Update the minimum external LLVM to 14) - rust-lang#107626 (Fix `x fix` on the standard library itself) - rust-lang#107673 (update ICU4X to 1.1.0) - rust-lang#107733 (Store metrics from `metrics.json` to CI PGO timer) - rust-lang#108007 (Use `is_str` instead of string kind comparison) - rust-lang#108033 (add an unstable `#[rustc_coinductive]` attribute) - rust-lang#108039 (Refactor refcounted structural_impls via functors) - rust-lang#108040 (Use derive attributes for uninteresting traversals) - rust-lang#108044 (interpret: rename Pointer::from_addr → from_addr_invalid) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This was referenced Nov 5, 2023
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
This file contains hidden or 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
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.
It appears that visiting and folding was implemented on
BitMatrixsolely so that the derive macros could be used onGeneratorLayout, however such implementation would not necessarily be correct for other uses (if there were any). Adding attributes to the derive macro is more correct and potentially more generally useful.r? @oli-obk