From e1e25a845c2d190afad0c98029cbe368f5bad427 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 7 Aug 2022 21:03:28 +0200 Subject: [PATCH 1/2] Generalize trait object generic param check to aliases. --- compiler/rustc_typeck/src/astconv/mod.rs | 134 +++++++----------- .../ui/associated-types/issue-22560.stderr | 46 +++--- .../cycle-trait-default-type-trait.rs | 1 - .../cycle-trait-default-type-trait.stderr | 21 +-- src/test/ui/issues/issue-21950.stderr | 18 +-- .../ui/traits/alias/generic-default-in-dyn.rs | 10 ++ .../alias/generic-default-in-dyn.stderr | 39 +++++ .../unspecified-self-in-trait-ref.rs | 0 .../unspecified-self-in-trait-ref.stderr | 0 9 files changed, 136 insertions(+), 133 deletions(-) create mode 100644 src/test/ui/traits/alias/generic-default-in-dyn.rs create mode 100644 src/test/ui/traits/alias/generic-default-in-dyn.stderr rename src/test/ui/{ => traits}/unspecified-self-in-trait-ref.rs (100%) rename src/test/ui/{ => traits}/unspecified-self-in-trait-ref.stderr (100%) diff --git a/compiler/rustc_typeck/src/astconv/mod.rs b/compiler/rustc_typeck/src/astconv/mod.rs index 8a5c7fee697d1..0951713e5db18 100644 --- a/compiler/rustc_typeck/src/astconv/mod.rs +++ b/compiler/rustc_typeck/src/astconv/mod.rs @@ -367,36 +367,13 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { return (tcx.intern_substs(&[]), arg_count); } - let is_object = self_ty.map_or(false, |ty| ty == self.tcx().types.trait_object_dummy_self); - struct SubstsForAstPathCtxt<'a, 'tcx> { astconv: &'a (dyn AstConv<'tcx> + 'a), def_id: DefId, generic_args: &'a GenericArgs<'a>, span: Span, - missing_type_params: Vec, inferred_params: Vec, infer_args: bool, - is_object: bool, - } - - impl<'tcx, 'a> SubstsForAstPathCtxt<'tcx, 'a> { - fn default_needs_object_self(&mut self, param: &ty::GenericParamDef) -> bool { - let tcx = self.astconv.tcx(); - if let GenericParamDefKind::Type { has_default, .. } = param.kind { - if self.is_object && has_default { - let default_ty = tcx.at(self.span).type_of(param.def_id); - let self_param = tcx.types.self_param; - if default_ty.walk().any(|arg| arg == self_param.into()) { - // There is no suitable inference default for a type parameter - // that references self, in an object type. - return true; - } - } - } - - false - } } impl<'a, 'tcx> CreateSubstsForGenericArgsCtxt<'a, 'tcx> for SubstsForAstPathCtxt<'a, 'tcx> { @@ -499,41 +476,23 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { GenericParamDefKind::Type { has_default, .. } => { if !infer_args && has_default { // No type parameter provided, but a default exists. - - // If we are converting an object type, then the - // `Self` parameter is unknown. However, some of the - // other type parameters may reference `Self` in their - // defaults. This will lead to an ICE if we are not - // careful! - if self.default_needs_object_self(param) { - self.missing_type_params.push(param.name); - tcx.ty_error().into() - } else { - // This is a default type parameter. - let substs = substs.unwrap(); - if substs.iter().any(|arg| match arg.unpack() { - GenericArgKind::Type(ty) => ty.references_error(), - _ => false, - }) { - // Avoid ICE #86756 when type error recovery goes awry. - return tcx.ty_error().into(); - } - self.astconv - .normalize_ty( - self.span, - EarlyBinder(tcx.at(self.span).type_of(param.def_id)) - .subst(tcx, substs), - ) - .into() + let substs = substs.unwrap(); + if substs.iter().any(|arg| match arg.unpack() { + GenericArgKind::Type(ty) => ty.references_error(), + _ => false, + }) { + // Avoid ICE #86756 when type error recovery goes awry. + return tcx.ty_error().into(); } + self.astconv + .normalize_ty( + self.span, + EarlyBinder(tcx.at(self.span).type_of(param.def_id)) + .subst(tcx, substs), + ) + .into() } else if infer_args { - // No type parameters were provided, we can infer all. - let param = if !self.default_needs_object_self(param) { - Some(param) - } else { - None - }; - self.astconv.ty_infer(param, self.span).into() + self.astconv.ty_infer(Some(param), self.span).into() } else { // We've already errored above about the mismatch. tcx.ty_error().into() @@ -563,10 +522,8 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { def_id, span, generic_args, - missing_type_params: vec![], inferred_params: vec![], infer_args, - is_object, }; let substs = Self::create_substs_for_generic_args( tcx, @@ -578,13 +535,6 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { &mut substs_ctx, ); - self.complain_about_missing_type_params( - substs_ctx.missing_type_params, - def_id, - span, - generic_args.args.is_empty(), - ); - debug!( "create_substs_for_ast_path(generic_params={:?}, self_ty={:?}) -> {:?}", generics, self_ty, substs @@ -1489,23 +1439,47 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { // Erase the `dummy_self` (`trait_object_dummy_self`) used above. let existential_trait_refs = regular_traits.iter().map(|i| { i.trait_ref().map_bound(|trait_ref: ty::TraitRef<'tcx>| { - if trait_ref.self_ty() != dummy_self { - // FIXME: There appears to be a missing filter on top of `expand_trait_aliases`, - // which picks up non-supertraits where clauses - but also, the object safety - // completely ignores trait aliases, which could be object safety hazards. We - // `delay_span_bug` here to avoid an ICE in stable even when the feature is - // disabled. (#66420) - tcx.sess.delay_span_bug( - DUMMY_SP, - &format!( - "trait_ref_to_existential called on {:?} with non-dummy Self", - trait_ref, - ), - ); - } - ty::ExistentialTraitRef::erase_self_ty(tcx, trait_ref) + assert_eq!(trait_ref.self_ty(), dummy_self); + + // Verify that `dummy_self` did not leak inside default type parameters. This + // could not be done at path creation, since we need to see through trait aliases. + let mut missing_type_params = vec![]; + let generics = tcx.generics_of(trait_ref.def_id); + let substs: Vec<_> = trait_ref + .substs + .iter() + .enumerate() + .skip(1) // Remove `Self` for `ExistentialPredicate`. + .map(|(index, arg)| { + if let ty::GenericArgKind::Type(ty) = arg.unpack() + && ty == dummy_self + { + let param = &generics.params[index]; + missing_type_params.push(param.name); + tcx.ty_error().into() + } else { + arg + } + }) + .collect(); + let substs = tcx.intern_substs(&substs[..]); + + let span = i.bottom().1; + let empty_generic_args = trait_bounds.iter().any(|hir_bound| { + hir_bound.trait_ref.path.res == Res::Def(DefKind::Trait, trait_ref.def_id) + && hir_bound.span.contains(span) + }); + self.complain_about_missing_type_params( + missing_type_params, + trait_ref.def_id, + span, + empty_generic_args, + ); + + ty::ExistentialTraitRef { def_id: trait_ref.def_id, substs } }) }); + let existential_projections = bounds.projection_bounds.iter().map(|(bound, _)| { bound.map_bound(|b| { if b.projection_ty.self_ty() != dummy_self { diff --git a/src/test/ui/associated-types/issue-22560.stderr b/src/test/ui/associated-types/issue-22560.stderr index 700923c1b3f58..2b88cf0b4411e 100644 --- a/src/test/ui/associated-types/issue-22560.stderr +++ b/src/test/ui/associated-types/issue-22560.stderr @@ -1,25 +1,3 @@ -error[E0393]: the type parameter `Rhs` must be explicitly specified - --> $DIR/issue-22560.rs:9:23 - | -LL | trait Sub { - | ------------------- type parameter `Rhs` must be specified for this -... -LL | type Test = dyn Add + Sub; - | ^^^ help: set the type parameter to the desired type: `Sub` - | - = note: because of the default `Self` reference, type parameters must be specified on object types - -error[E0393]: the type parameter `Rhs` must be explicitly specified - --> $DIR/issue-22560.rs:9:17 - | -LL | trait Add { - | ------------------- type parameter `Rhs` must be specified for this -... -LL | type Test = dyn Add + Sub; - | ^^^ help: set the type parameter to the desired type: `Add` - | - = note: because of the default `Self` reference, type parameters must be specified on object types - error[E0225]: only auto traits can be used as additional traits in a trait object --> $DIR/issue-22560.rs:9:23 | @@ -28,7 +6,7 @@ LL | type Test = dyn Add + Sub; | | | first non-auto trait | - = help: consider creating a new trait with all of these as supertraits and using that trait here instead: `trait NewTrait: Add<[type error]> + Sub<[type error]> {}` + = help: consider creating a new trait with all of these as supertraits and using that trait here instead: `trait NewTrait: Add + Sub {}` = note: auto-traits like `Send` and `Sync` are traits that have special properties; for more information on them, visit error[E0191]: the value of the associated types `Output` (from trait `Add`), `Output` (from trait `Sub`) must be specified @@ -50,6 +28,28 @@ help: specify the associated types LL | type Test = dyn Add + Sub; | ~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~ +error[E0393]: the type parameter `Rhs` must be explicitly specified + --> $DIR/issue-22560.rs:9:17 + | +LL | trait Add { + | ------------------- type parameter `Rhs` must be specified for this +... +LL | type Test = dyn Add + Sub; + | ^^^ help: set the type parameter to the desired type: `Add` + | + = note: because of the default `Self` reference, type parameters must be specified on object types + +error[E0393]: the type parameter `Rhs` must be explicitly specified + --> $DIR/issue-22560.rs:9:23 + | +LL | trait Sub { + | ------------------- type parameter `Rhs` must be specified for this +... +LL | type Test = dyn Add + Sub; + | ^^^ help: set the type parameter to the desired type: `Sub` + | + = note: because of the default `Self` reference, type parameters must be specified on object types + error: aborting due to 4 previous errors Some errors have detailed explanations: E0191, E0225, E0393. diff --git a/src/test/ui/cycle-trait/cycle-trait-default-type-trait.rs b/src/test/ui/cycle-trait/cycle-trait-default-type-trait.rs index b2edc1a1f66ca..6175b7df1107a 100644 --- a/src/test/ui/cycle-trait/cycle-trait-default-type-trait.rs +++ b/src/test/ui/cycle-trait/cycle-trait-default-type-trait.rs @@ -3,7 +3,6 @@ trait Foo> { //~^ ERROR cycle detected - //~| ERROR cycle detected } fn main() { } diff --git a/src/test/ui/cycle-trait/cycle-trait-default-type-trait.stderr b/src/test/ui/cycle-trait/cycle-trait-default-type-trait.stderr index d4976a0f9c9cd..9d715f4947146 100644 --- a/src/test/ui/cycle-trait/cycle-trait-default-type-trait.stderr +++ b/src/test/ui/cycle-trait/cycle-trait-default-type-trait.stderr @@ -10,30 +10,11 @@ note: cycle used when collecting item types in top-level module | LL | / trait Foo> { LL | | -LL | | -LL | | } -LL | | -LL | | fn main() { } - | |_____________^ - -error[E0391]: cycle detected when computing type of `Foo::X` - --> $DIR/cycle-trait-default-type-trait.rs:4:23 - | -LL | trait Foo> { - | ^^^ - | - = note: ...which immediately requires computing type of `Foo::X` again -note: cycle used when collecting item types in top-level module - --> $DIR/cycle-trait-default-type-trait.rs:4:1 - | -LL | / trait Foo> { -LL | | -LL | | LL | | } LL | | LL | | fn main() { } | |_____________^ -error: aborting due to 2 previous errors +error: aborting due to previous error For more information about this error, try `rustc --explain E0391`. diff --git a/src/test/ui/issues/issue-21950.stderr b/src/test/ui/issues/issue-21950.stderr index 4909398bb848f..731615a6bd8d4 100644 --- a/src/test/ui/issues/issue-21950.stderr +++ b/src/test/ui/issues/issue-21950.stderr @@ -1,3 +1,12 @@ +error[E0191]: the value of the associated type `Output` (from trait `Add`) must be specified + --> $DIR/issue-21950.rs:10:25 + | +LL | type Output; + | ----------- `Output` defined here +... +LL | let x = &10 as &dyn Add; + | ^^^ help: specify the associated type: `Add` + error[E0393]: the type parameter `Rhs` must be explicitly specified --> $DIR/issue-21950.rs:10:25 | @@ -9,15 +18,6 @@ LL | let x = &10 as &dyn Add; | = note: because of the default `Self` reference, type parameters must be specified on object types -error[E0191]: the value of the associated type `Output` (from trait `Add`) must be specified - --> $DIR/issue-21950.rs:10:25 - | -LL | type Output; - | ----------- `Output` defined here -... -LL | let x = &10 as &dyn Add; - | ^^^ help: specify the associated type: `Add` - error: aborting due to 2 previous errors Some errors have detailed explanations: E0191, E0393. diff --git a/src/test/ui/traits/alias/generic-default-in-dyn.rs b/src/test/ui/traits/alias/generic-default-in-dyn.rs new file mode 100644 index 0000000000000..d44e1c2a97530 --- /dev/null +++ b/src/test/ui/traits/alias/generic-default-in-dyn.rs @@ -0,0 +1,10 @@ +trait SendEqAlias = PartialEq; +//~^ ERROR trait aliases are experimental + +struct Foo(dyn SendEqAlias); +//~^ ERROR the type parameter `Rhs` must be explicitly specified [E0393] + +struct Bar(dyn SendEqAlias, T); +//~^ ERROR the type parameter `Rhs` must be explicitly specified [E0393] + +fn main() {} diff --git a/src/test/ui/traits/alias/generic-default-in-dyn.stderr b/src/test/ui/traits/alias/generic-default-in-dyn.stderr new file mode 100644 index 0000000000000..76a068e864a3c --- /dev/null +++ b/src/test/ui/traits/alias/generic-default-in-dyn.stderr @@ -0,0 +1,39 @@ +error[E0658]: trait aliases are experimental + --> $DIR/generic-default-in-dyn.rs:1:1 + | +LL | trait SendEqAlias = PartialEq; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #41517 for more information + = help: add `#![feature(trait_alias)]` to the crate attributes to enable + +error[E0393]: the type parameter `Rhs` must be explicitly specified + --> $DIR/generic-default-in-dyn.rs:4:19 + | +LL | struct Foo(dyn SendEqAlias); + | ^^^^^^^^^^^^^^ missing reference to `Rhs` + | + ::: $SRC_DIR/core/src/cmp.rs:LL:COL + | +LL | pub trait PartialEq { + | --------------------------------------- type parameter `Rhs` must be specified for this + | + = note: because of the default `Self` reference, type parameters must be specified on object types + +error[E0393]: the type parameter `Rhs` must be explicitly specified + --> $DIR/generic-default-in-dyn.rs:7:19 + | +LL | struct Bar(dyn SendEqAlias, T); + | ^^^^^^^^^^^^^^ missing reference to `Rhs` + | + ::: $SRC_DIR/core/src/cmp.rs:LL:COL + | +LL | pub trait PartialEq { + | --------------------------------------- type parameter `Rhs` must be specified for this + | + = note: because of the default `Self` reference, type parameters must be specified on object types + +error: aborting due to 3 previous errors + +Some errors have detailed explanations: E0393, E0658. +For more information about an error, try `rustc --explain E0393`. diff --git a/src/test/ui/unspecified-self-in-trait-ref.rs b/src/test/ui/traits/unspecified-self-in-trait-ref.rs similarity index 100% rename from src/test/ui/unspecified-self-in-trait-ref.rs rename to src/test/ui/traits/unspecified-self-in-trait-ref.rs diff --git a/src/test/ui/unspecified-self-in-trait-ref.stderr b/src/test/ui/traits/unspecified-self-in-trait-ref.stderr similarity index 100% rename from src/test/ui/unspecified-self-in-trait-ref.stderr rename to src/test/ui/traits/unspecified-self-in-trait-ref.stderr From 0df84ae67c01d44c3d6c0887333bafca1ea7f060 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 7 Aug 2022 22:02:02 +0200 Subject: [PATCH 2/2] Ban indirect references to `Self` too. --- compiler/rustc_typeck/src/astconv/mod.rs | 38 +++++++++++++++---- src/test/ui/traits/alias/self-in-generics.rs | 8 ++++ .../ui/traits/alias/self-in-generics.stderr | 11 ++++++ 3 files changed, 50 insertions(+), 7 deletions(-) create mode 100644 src/test/ui/traits/alias/self-in-generics.rs create mode 100644 src/test/ui/traits/alias/self-in-generics.stderr diff --git a/compiler/rustc_typeck/src/astconv/mod.rs b/compiler/rustc_typeck/src/astconv/mod.rs index 0951713e5db18..9ec3002e3aaf1 100644 --- a/compiler/rustc_typeck/src/astconv/mod.rs +++ b/compiler/rustc_typeck/src/astconv/mod.rs @@ -43,7 +43,7 @@ use rustc_trait_selection::traits::error_reporting::{ }; use rustc_trait_selection::traits::wf::object_region_bounds; -use smallvec::SmallVec; +use smallvec::{smallvec, SmallVec}; use std::collections::BTreeSet; use std::slice; @@ -1444,6 +1444,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { // Verify that `dummy_self` did not leak inside default type parameters. This // could not be done at path creation, since we need to see through trait aliases. let mut missing_type_params = vec![]; + let mut references_self = false; let generics = tcx.generics_of(trait_ref.def_id); let substs: Vec<_> = trait_ref .substs @@ -1451,12 +1452,18 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { .enumerate() .skip(1) // Remove `Self` for `ExistentialPredicate`. .map(|(index, arg)| { - if let ty::GenericArgKind::Type(ty) = arg.unpack() - && ty == dummy_self - { - let param = &generics.params[index]; - missing_type_params.push(param.name); - tcx.ty_error().into() + if let ty::GenericArgKind::Type(ty) = arg.unpack() { + debug!(?ty); + if ty == dummy_self { + let param = &generics.params[index]; + missing_type_params.push(param.name); + tcx.ty_error().into() + } else if ty.walk().any(|arg| arg == dummy_self.into()) { + references_self = true; + tcx.ty_error().into() + } else { + arg + } } else { arg } @@ -1476,6 +1483,23 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { empty_generic_args, ); + if references_self { + let def_id = i.bottom().0.def_id(); + let mut err = struct_span_err!( + tcx.sess, + i.bottom().1, + E0038, + "the {} `{}` cannot be made into an object", + tcx.def_kind(def_id).descr(def_id), + tcx.item_name(def_id), + ); + err.note( + rustc_middle::traits::ObjectSafetyViolation::SupertraitSelf(smallvec![]) + .error_msg(), + ); + err.emit(); + } + ty::ExistentialTraitRef { def_id: trait_ref.def_id, substs } }) }); diff --git a/src/test/ui/traits/alias/self-in-generics.rs b/src/test/ui/traits/alias/self-in-generics.rs new file mode 100644 index 0000000000000..6b99431f5bbcf --- /dev/null +++ b/src/test/ui/traits/alias/self-in-generics.rs @@ -0,0 +1,8 @@ +#![feature(trait_alias)] + +pub trait SelfInput = Fn(&mut Self); + +pub fn f(_f: &dyn SelfInput) {} +//~^ ERROR the trait alias `SelfInput` cannot be made into an object [E0038] + +fn main() {} diff --git a/src/test/ui/traits/alias/self-in-generics.stderr b/src/test/ui/traits/alias/self-in-generics.stderr new file mode 100644 index 0000000000000..a1056872ea641 --- /dev/null +++ b/src/test/ui/traits/alias/self-in-generics.stderr @@ -0,0 +1,11 @@ +error[E0038]: the trait alias `SelfInput` cannot be made into an object + --> $DIR/self-in-generics.rs:5:19 + | +LL | pub fn f(_f: &dyn SelfInput) {} + | ^^^^^^^^^ + | + = note: it cannot use `Self` as a type parameter in a supertrait or `where`-clause + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0038`.