-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tracking issue for 'Allow Self
to appear in the where clause of trait impls'
#38864
Comments
Partially implement RFC 1647 (`Self` in impl headers) The name resolution part is easy, but the typeck part contains an unexpected problem. It turns out that `Self` type *depends* on bounds and `where` clauses, so we need to convert them first to determine what the `Self` type is! If bounds/`where` clauses can refer to `Self` then we have a cyclic dependency. This is required to support impls like this: ``` // Found in libcollections impl<I: IntoIterator> SpecExtend<I> for LinkedList<I::Item> { .... } ^^^^^ associated type `Item` is found using information from bounds ``` I'm not yet sure how to resolve this issue. One possible solution (that feels hacky) is to make two passes over generics - first collect predicates ignoring everything involving `Self`, then determine `Self`, then collect predicates again without ignoring anything. (Some kind of lazy on-demand checking or something looks like a proper solution.) This patch in its current state doesn't solve the problem with `Self` in bounds, so the only observable things it does is improving error messages and supporting `impl Trait<Self> for Type {}`. There's also a question about feature gating. It's non-trivial to *detect* "newly resolved" `Self`s to feature gate them, but it's simple to *enable* the new resolution behavior when the feature gate is already specified. Alternatively this can be considered a bug fix and merged without a feature gate. cc #38864 r? @nikomatsakis cc @eddyb Whitespace ignoring diff https://github.com/rust-lang/rust/pull/38920/files?w=1
[12/12] On-demand type-checking, const-evaluation, MIR building & const-qualification. _This is the last of a series ([prev](#38813)) of patches designed to rework rustc into an out-of-order on-demand pipeline model for both better feature support (e.g. [MIR-based](https://github.com/solson/miri) early constant evaluation) and incremental execution of compiler passes (e.g. type-checking), with beneficial consequences to IDE support as well. If any motivation is unclear, please ask for additional PR description clarifications or code comments._ <hr> As this contains all of the changes that didn't fit neatly into other PRs, I'll be elaborating a bit: ### User-facing changes * when determining whether an `impl Trait` type implements an auto-trait (e.g. `Send` or `Sync`), the function the `impl Trait` came from has to be inferred and type-checking, disallowing cycles * this results from not having an obvious place to put the "deferred obligation" in on-demand atm * while we could model side-effects like that and "post-processing passes" better, it's still more limiting than being able to know the result in the original function (e.g. specialization) *and* there are serious problems around region-checking (if a `Send` impl required `'static`, it wasn't enforced) * early const-eval requires type-checking and const-qualification to be performed first, which means: * you get the intended errors before (if any) constant evaluation error that is simply fallout * associated consts should always work now, and `const fn` type parameters are properly tracked * don't get too excited, array lengths still can't depend on type parameters * #38864 works as intended now, with `Self` being allowed in `impl` bounds * #32205 is largely improved, with associated types being limited to "exact match" `impl`s (as opposed to traversing the specialization graph to resolve unspecified type parameters to their defaults in another `impl` or in the `trait`) *while* checking for overlaps building the specialization graph for that trait - once all the trait impls' have been checked for coherence (including ahead-of-time/on-demand), it's uniform * [crater report](https://gist.github.com/eddyb/bbb869072468c7e08d6d808e75938051) looks clean (aside from `clippy` which broke due to `rustc` internal changes) ### Compiler-internal changes * `ty::Generics` * no longer contains the actual type parameter defaults, instead they're associated with the type parameter's `DefId`, like associated types in a trait definition * this allows computing `ty::Generics` as a leaf (reading only its own HIR) * holds a mapping from `DefIndex` of type parameters to their indices * `ty::AdtDef` * only tracks `#[repr(simd)]` in its `ReprOptions` `repr` field * doesn't contain `enum` discriminant values, but instead each variant either refers to either an explicit value for its discriminant, or the distance from the last explicit discriminant, if any * the `.discriminants(tcx)` method produces an iterator of `ConstInt` values, looking up explicit discriminants in a separate map, if necessary * this allows computing `ty::AdtDef` as a leaf (reading only its own HIR) * Small note: the two above (`Generics`, `AdtDef`), `TraitDef` and `AssociatedItem` should probably end up as part of the HIR, eventually, as they're trivially constructed from it * `ty::FnSig` * now also holds ABI and unsafety, alongside argument types, return type and C variadicity * `&ty::BareFnTy` and `ty::ClosureTy` have been replaced with `PolyFnSig = Binder<FnSig>` * `BareFnTy` was interned and `ClosureTy` was treated as non-trivial to `Clone` because they had a `PolyFnSig` and so used to contain a `Vec<Ty>` (now `&[Ty]`) * `ty::maps` * all the `DepTrackingMap`s have been grouped in a structure available at `tcx.maps` * when creating the `tcx`, a set of `Providers` (one `fn` pointer per map) is required for the local crate, and one for all other crates (i.e. metadata loading), `librustc_driver` plugging the various crates (e.g. `librustc_metadata`, `librustc_typeck`, `librustc_mir`) into it * when a map is queried and the value is missing, the appropriate `fn` pointer from the `Providers` of that crate is called with the `TyCtxt` and the key being queried, to produce the value on-demand * `rustc_const_eval` * demands both `typeck_tables` and `mir_const_qualif` (in preparation for miri) * tracks `Substs` in `ConstVal::Function` for `const fn` calls * returns `TypeckError` if type-checking has failed (or cases that can only be reached if it had) * this error kind is never reported, resulting in less noisy/redundant diagnostics * fixes #39548 (testcase by @larsluthman, taken from #39812, which this supersedes) * on-demand has so far been hooked up to: * `rustc_metadata::cstore_impl`: `ty`, `generics`, `predicates`, `super_predicates`, `trait_def`, `adt_def`, `variances`, `associated_item_def_ids`, `associated_item`, `impl_trait_ref`, `custom_coerce_unsized_kind`, `mir`, `mir_const_qualif`, `typeck_tables`, `closure_kind`, `closure_type` * `rustc_typeck::collect`: `ty`, `generics`, `predicates`, `super_predicates`, `type_param_predicates`, `trait_def`, `adt_def`, `impl_trait_ref` * `rustc_typeck::coherence`: `coherent_trait`, `coherent_inherent_impls` * `rustc_typeck::check`: `typeck_tables`, `closure_type`, `closure_kind` * `rustc_mir::mir_map`: `mir` * `rustc_mir::transform::qualify_consts`: `mir_const_qualif`
[12/12] On-demand type-checking, const-evaluation, MIR building & const-qualification. _This is the last of a series ([prev](#38813)) of patches designed to rework rustc into an out-of-order on-demand pipeline model for both better feature support (e.g. [MIR-based](https://github.com/solson/miri) early constant evaluation) and incremental execution of compiler passes (e.g. type-checking), with beneficial consequences to IDE support as well. If any motivation is unclear, please ask for additional PR description clarifications or code comments._ <hr> As this contains all of the changes that didn't fit neatly into other PRs, I'll be elaborating a bit: ### User-facing changes * when determining whether an `impl Trait` type implements an auto-trait (e.g. `Send` or `Sync`), the function the `impl Trait` came from has to be inferred and type-checking, disallowing cycles * this results from not having an obvious place to put the "deferred obligation" in on-demand atm * while we could model side-effects like that and "post-processing passes" better, it's still more limiting than being able to know the result in the original function (e.g. specialization) *and* there are serious problems around region-checking (if a `Send` impl required `'static`, it wasn't enforced) * early const-eval requires type-checking and const-qualification to be performed first, which means: * you get the intended errors before (if any) constant evaluation error that is simply fallout * associated consts should always work now, and `const fn` type parameters are properly tracked * don't get too excited, array lengths still can't depend on type parameters * #38864 works as intended now, with `Self` being allowed in `impl` bounds * #32205 is largely improved, with associated types being limited to "exact match" `impl`s (as opposed to traversing the specialization graph to resolve unspecified type parameters to their defaults in another `impl` or in the `trait`) *while* checking for overlaps building the specialization graph for that trait - once all the trait impls' have been checked for coherence (including ahead-of-time/on-demand), it's uniform * [crater report](https://gist.github.com/eddyb/bbb869072468c7e08d6d808e75938051) looks clean (aside from `clippy` which broke due to `rustc` internal changes) ### Compiler-internal changes * `ty::Generics` * no longer contains the actual type parameter defaults, instead they're associated with the type parameter's `DefId`, like associated types in a trait definition * this allows computing `ty::Generics` as a leaf (reading only its own HIR) * holds a mapping from `DefIndex` of type parameters to their indices * `ty::AdtDef` * only tracks `#[repr(simd)]` in its `ReprOptions` `repr` field * doesn't contain `enum` discriminant values, but instead each variant either refers to either an explicit value for its discriminant, or the distance from the last explicit discriminant, if any * the `.discriminants(tcx)` method produces an iterator of `ConstInt` values, looking up explicit discriminants in a separate map, if necessary * this allows computing `ty::AdtDef` as a leaf (reading only its own HIR) * Small note: the two above (`Generics`, `AdtDef`), `TraitDef` and `AssociatedItem` should probably end up as part of the HIR, eventually, as they're trivially constructed from it * `ty::FnSig` * now also holds ABI and unsafety, alongside argument types, return type and C variadicity * `&ty::BareFnTy` and `ty::ClosureTy` have been replaced with `PolyFnSig = Binder<FnSig>` * `BareFnTy` was interned and `ClosureTy` was treated as non-trivial to `Clone` because they had a `PolyFnSig` and so used to contain a `Vec<Ty>` (now `&[Ty]`) * `ty::maps` * all the `DepTrackingMap`s have been grouped in a structure available at `tcx.maps` * when creating the `tcx`, a set of `Providers` (one `fn` pointer per map) is required for the local crate, and one for all other crates (i.e. metadata loading), `librustc_driver` plugging the various crates (e.g. `librustc_metadata`, `librustc_typeck`, `librustc_mir`) into it * when a map is queried and the value is missing, the appropriate `fn` pointer from the `Providers` of that crate is called with the `TyCtxt` and the key being queried, to produce the value on-demand * `rustc_const_eval` * demands both `typeck_tables` and `mir_const_qualif` (in preparation for miri) * tracks `Substs` in `ConstVal::Function` for `const fn` calls * returns `TypeckError` if type-checking has failed (or cases that can only be reached if it had) * this error kind is never reported, resulting in less noisy/redundant diagnostics * fixes #39548 (testcase by @larsluthman, taken from #39812, which this supersedes) * on-demand has so far been hooked up to: * `rustc_metadata::cstore_impl`: `ty`, `generics`, `predicates`, `super_predicates`, `trait_def`, `adt_def`, `variances`, `associated_item_def_ids`, `associated_item`, `impl_trait_ref`, `custom_coerce_unsized_kind`, `mir`, `mir_const_qualif`, `typeck_tables`, `closure_kind`, `closure_type` * `rustc_typeck::collect`: `ty`, `generics`, `predicates`, `super_predicates`, `type_param_predicates`, `trait_def`, `adt_def`, `impl_trait_ref` * `rustc_typeck::coherence`: `coherent_trait`, `coherent_inherent_impls` * `rustc_typeck::check`: `typeck_tables`, `closure_type`, `closure_kind` * `rustc_mir::mir_map`: `mir` * `rustc_mir::transform::qualify_consts`: `mir_const_qualif`
[12/12] On-demand type-checking, const-evaluation, MIR building & const-qualification. _This is the last of a series ([prev](#38813)) of patches designed to rework rustc into an out-of-order on-demand pipeline model for both better feature support (e.g. [MIR-based](https://github.com/solson/miri) early constant evaluation) and incremental execution of compiler passes (e.g. type-checking), with beneficial consequences to IDE support as well. If any motivation is unclear, please ask for additional PR description clarifications or code comments._ <hr> As this contains all of the changes that didn't fit neatly into other PRs, I'll be elaborating a bit: ### User-facing changes * when determining whether an `impl Trait` type implements an auto-trait (e.g. `Send` or `Sync`), the function the `impl Trait` came from has to be inferred and type-checking, disallowing cycles * this results from not having an obvious place to put the "deferred obligation" in on-demand atm * while we could model side-effects like that and "post-processing passes" better, it's still more limiting than being able to know the result in the original function (e.g. specialization) *and* there are serious problems around region-checking (if a `Send` impl required `'static`, it wasn't enforced) * early const-eval requires type-checking and const-qualification to be performed first, which means: * you get the intended errors before (if any) constant evaluation error that is simply fallout * associated consts should always work now, and `const fn` type parameters are properly tracked * don't get too excited, array lengths still can't depend on type parameters * #38864 works as intended now, with `Self` being allowed in `impl` bounds * #32205 is largely improved, with associated types being limited to "exact match" `impl`s (as opposed to traversing the specialization graph to resolve unspecified type parameters to their defaults in another `impl` or in the `trait`) *while* checking for overlaps building the specialization graph for that trait - once all the trait impls' have been checked for coherence (including ahead-of-time/on-demand), it's uniform * [crater report](https://gist.github.com/eddyb/bbb869072468c7e08d6d808e75938051) looks clean (aside from `clippy` which broke due to `rustc` internal changes) ### Compiler-internal changes * `ty::Generics` * no longer contains the actual type parameter defaults, instead they're associated with the type parameter's `DefId`, like associated types in a trait definition * this allows computing `ty::Generics` as a leaf (reading only its own HIR) * holds a mapping from `DefIndex` of type parameters to their indices * `ty::AdtDef` * only tracks `#[repr(simd)]` in its `ReprOptions` `repr` field * doesn't contain `enum` discriminant values, but instead each variant either refers to either an explicit value for its discriminant, or the distance from the last explicit discriminant, if any * the `.discriminants(tcx)` method produces an iterator of `ConstInt` values, looking up explicit discriminants in a separate map, if necessary * this allows computing `ty::AdtDef` as a leaf (reading only its own HIR) * Small note: the two above (`Generics`, `AdtDef`), `TraitDef` and `AssociatedItem` should probably end up as part of the HIR, eventually, as they're trivially constructed from it * `ty::FnSig` * now also holds ABI and unsafety, alongside argument types, return type and C variadicity * `&ty::BareFnTy` and `ty::ClosureTy` have been replaced with `PolyFnSig = Binder<FnSig>` * `BareFnTy` was interned and `ClosureTy` was treated as non-trivial to `Clone` because they had a `PolyFnSig` and so used to contain a `Vec<Ty>` (now `&[Ty]`) * `ty::maps` * all the `DepTrackingMap`s have been grouped in a structure available at `tcx.maps` * when creating the `tcx`, a set of `Providers` (one `fn` pointer per map) is required for the local crate, and one for all other crates (i.e. metadata loading), `librustc_driver` plugging the various crates (e.g. `librustc_metadata`, `librustc_typeck`, `librustc_mir`) into it * when a map is queried and the value is missing, the appropriate `fn` pointer from the `Providers` of that crate is called with the `TyCtxt` and the key being queried, to produce the value on-demand * `rustc_const_eval` * demands both `typeck_tables` and `mir_const_qualif` (in preparation for miri) * tracks `Substs` in `ConstVal::Function` for `const fn` calls * returns `TypeckError` if type-checking has failed (or cases that can only be reached if it had) * this error kind is never reported, resulting in less noisy/redundant diagnostics * fixes #39548 (testcase by @larsluthman, taken from #39812, which this supersedes) * on-demand has so far been hooked up to: * `rustc_metadata::cstore_impl`: `ty`, `generics`, `predicates`, `super_predicates`, `trait_def`, `adt_def`, `variances`, `associated_item_def_ids`, `associated_item`, `impl_trait_ref`, `custom_coerce_unsized_kind`, `mir`, `mir_const_qualif`, `typeck_tables`, `closure_kind`, `closure_type` * `rustc_typeck::collect`: `ty`, `generics`, `predicates`, `super_predicates`, `type_param_predicates`, `trait_def`, `adt_def`, `impl_trait_ref` * `rustc_typeck::coherence`: `coherent_trait`, `coherent_inherent_impls` * `rustc_typeck::check`: `typeck_tables`, `closure_type`, `closure_kind` * `rustc_mir::mir_map`: `mir` * `rustc_mir::transform::qualify_consts`: `mir_const_qualif`
[12/12] On-demand type-checking, const-evaluation, MIR building & const-qualification. _This is the last of a series ([prev](#38813)) of patches designed to rework rustc into an out-of-order on-demand pipeline model for both better feature support (e.g. [MIR-based](https://github.com/solson/miri) early constant evaluation) and incremental execution of compiler passes (e.g. type-checking), with beneficial consequences to IDE support as well. If any motivation is unclear, please ask for additional PR description clarifications or code comments._ <hr> As this contains all of the changes that didn't fit neatly into other PRs, I'll be elaborating a bit: ### User-facing changes * when determining whether an `impl Trait` type implements an auto-trait (e.g. `Send` or `Sync`), the function the `impl Trait` came from has to be inferred and type-checking, disallowing cycles * this results from not having an obvious place to put the "deferred obligation" in on-demand atm * while we could model side-effects like that and "post-processing passes" better, it's still more limiting than being able to know the result in the original function (e.g. specialization) *and* there are serious problems around region-checking (if a `Send` impl required `'static`, it wasn't enforced) * early const-eval requires type-checking and const-qualification to be performed first, which means: * you get the intended errors before (if any) constant evaluation error that is simply fallout * associated consts should always work now, and `const fn` type parameters are properly tracked * don't get too excited, array lengths still can't depend on type parameters * #38864 works as intended now, with `Self` being allowed in `impl` bounds * #32205 is largely improved, with associated types being limited to "exact match" `impl`s (as opposed to traversing the specialization graph to resolve unspecified type parameters to their defaults in another `impl` or in the `trait`) *while* checking for overlaps building the specialization graph for that trait - once all the trait impls' have been checked for coherence (including ahead-of-time/on-demand), it's uniform * [crater report](https://gist.github.com/eddyb/bbb869072468c7e08d6d808e75938051) looks clean (aside from `clippy` which broke due to `rustc` internal changes) ### Compiler-internal changes * `ty::Generics` * no longer contains the actual type parameter defaults, instead they're associated with the type parameter's `DefId`, like associated types in a trait definition * this allows computing `ty::Generics` as a leaf (reading only its own HIR) * holds a mapping from `DefIndex` of type parameters to their indices * `ty::AdtDef` * only tracks `#[repr(simd)]` in its `ReprOptions` `repr` field * doesn't contain `enum` discriminant values, but instead each variant either refers to either an explicit value for its discriminant, or the distance from the last explicit discriminant, if any * the `.discriminants(tcx)` method produces an iterator of `ConstInt` values, looking up explicit discriminants in a separate map, if necessary * this allows computing `ty::AdtDef` as a leaf (reading only its own HIR) * Small note: the two above (`Generics`, `AdtDef`), `TraitDef` and `AssociatedItem` should probably end up as part of the HIR, eventually, as they're trivially constructed from it * `ty::FnSig` * now also holds ABI and unsafety, alongside argument types, return type and C variadicity * `&ty::BareFnTy` and `ty::ClosureTy` have been replaced with `PolyFnSig = Binder<FnSig>` * `BareFnTy` was interned and `ClosureTy` was treated as non-trivial to `Clone` because they had a `PolyFnSig` and so used to contain a `Vec<Ty>` (now `&[Ty]`) * `ty::maps` * all the `DepTrackingMap`s have been grouped in a structure available at `tcx.maps` * when creating the `tcx`, a set of `Providers` (one `fn` pointer per map) is required for the local crate, and one for all other crates (i.e. metadata loading), `librustc_driver` plugging the various crates (e.g. `librustc_metadata`, `librustc_typeck`, `librustc_mir`) into it * when a map is queried and the value is missing, the appropriate `fn` pointer from the `Providers` of that crate is called with the `TyCtxt` and the key being queried, to produce the value on-demand * `rustc_const_eval` * demands both `typeck_tables` and `mir_const_qualif` (in preparation for miri) * tracks `Substs` in `ConstVal::Function` for `const fn` calls * returns `TypeckError` if type-checking has failed (or cases that can only be reached if it had) * this error kind is never reported, resulting in less noisy/redundant diagnostics * fixes #39548 (testcase by @larsluthman, taken from #39812, which this supersedes) * on-demand has so far been hooked up to: * `rustc_metadata::cstore_impl`: `ty`, `generics`, `predicates`, `super_predicates`, `trait_def`, `adt_def`, `variances`, `associated_item_def_ids`, `associated_item`, `impl_trait_ref`, `custom_coerce_unsized_kind`, `mir`, `mir_const_qualif`, `typeck_tables`, `closure_kind`, `closure_type` * `rustc_typeck::collect`: `ty`, `generics`, `predicates`, `super_predicates`, `type_param_predicates`, `trait_def`, `adt_def`, `impl_trait_ref` * `rustc_typeck::coherence`: `coherent_trait`, `coherent_inherent_impls` * `rustc_typeck::check`: `typeck_tables`, `closure_type`, `closure_kind` * `rustc_mir::mir_map`: `mir` * `rustc_mir::transform::qualify_consts`: `mir_const_qualif`
The RFC still isn't fully implemented. In particular, this example from the RFC text still fails to compile: impl SomeTrait for SomeType where Self::AssocType: SomeOtherTrait {
type AssocType = SomeOtherType;
} |
@sgrif What's the error? |
struct SomeType;
struct SomeOtherType;
trait SomeTrait {
type AssocType;
}
trait SomeOtherTrait {}
impl SomeTrait for SomeType where Self::AssocType: SomeOtherTrait {
type AssocType = SomeOtherType;
}
fn main() {}
|
Ah that's not part of the actual syntactical support (which works!), but rather the trait system. |
According to the last comment here, it seems like maybe this is finished? |
The issue I raised still exists. I'm not sure how it was overlooked in the RFC, the example given from the RFC itself, and the text explicitly states that we'd like to be able to reference associated types from the trait being implemented. I agree that this is a fix that needs to happen in the trait system, not just syntactic support, but it is something the RFC calls out explicitly multiple times. |
Could you open a separate ticket please, focused on that specific bug? |
RFC: https://github.com/rust-lang/rfcs/blob/master/text/1647-allow-self-in-where-clauses.md
RFC PR: rust-lang/rfcs#1647
Proposes allowing the
Self
type to be used in every position in trait implementations, including where clauses and other parameters to the trait being implemented.The text was updated successfully, but these errors were encountered: