From baf9f0173cd841792230e788b628ea6ff0539603 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sun, 16 Dec 2018 00:00:46 +0200 Subject: [PATCH 1/2] fix trait objects with a Self-having projection va This follows ALT2 in the issue. Fixes #56288. --- src/librustc/traits/object_safety.rs | 21 +++++++++++++- src/librustc_typeck/astconv.rs | 28 +++++++++++++++++-- ...ject-with-self-in-projection-output-bad.rs | 22 +++++++++++++++ ...-with-self-in-projection-output-bad.stderr | 12 ++++++++ ...ect-with-self-in-projection-output-good.rs | 23 +++++++++++++++ 5 files changed, 103 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/traits/trait-object-with-self-in-projection-output-bad.rs create mode 100644 src/test/ui/traits/trait-object-with-self-in-projection-output-bad.stderr create mode 100644 src/test/ui/traits/trait-object-with-self-in-projection-output-good.rs diff --git a/src/librustc/traits/object_safety.rs b/src/librustc/traits/object_safety.rs index 4b2f817cfa91a..fe40141a5e123 100644 --- a/src/librustc/traits/object_safety.rs +++ b/src/librustc/traits/object_safety.rs @@ -190,7 +190,26 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { // In the case of a trait predicate, we can skip the "self" type. data.skip_binder().input_types().skip(1).any(|t| t.has_self_ty()) } - ty::Predicate::Projection(..) | + ty::Predicate::Projection(ref data) => { + // And similarly for projections. This should be redundant with + // the previous check because any projection should have a + // matching `Trait` predicate with the same inputs, but we do + // the check to be safe. + // + // Note that we *do* allow projection *outputs* to contain + // `self` (i.e., `trait Foo: Bar { type Result; }`), + // we just require the user to specify *both* outputs + // in the object type (i.e., `dyn Foo`). + // + // This is ALT2 in issue #56288, see that for discussion of the + // possible alternatives. + data.skip_binder() + .projection_ty + .trait_ref(self) + .input_types() + .skip(1) + .any(|t| t.has_self_ty()) + } ty::Predicate::WellFormed(..) | ty::Predicate::ObjectSafe(..) | ty::Predicate::TypeOutlives(..) | diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 43e7aee1b124e..ab1eeffc6217a 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -1013,6 +1013,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o { let mut associated_types = BTreeSet::default(); for tr in traits::elaborate_trait_ref(tcx, principal) { + debug!("conv_object_ty_poly_trait_ref: observing object predicate `{:?}`", tr); match tr { ty::Predicate::Trait(pred) => { associated_types.extend(tcx.associated_items(pred.def_id()) @@ -1020,8 +1021,31 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o { .map(|item| item.def_id)); } ty::Predicate::Projection(pred) => { - // Include projections defined on supertraits. - projection_bounds.push((pred, DUMMY_SP)) + // A `Self` within the original bound will be substituted with a + // `TRAIT_OBJECT_DUMMY_SELF`, so check for that. + let references_self = + pred.skip_binder().ty.walk().any(|t| t == dummy_self); + + // If the projection output contains `Self`, force the user to + // elaborate it explicitly to avoid a bunch of complexity. + // + // The "classicaly useful" case is the following: + // ``` + // trait MyTrait: FnMut() -> ::MyOutput { + // type MyOutput; + // } + // ``` + // + // Here, the user could theoretically write `dyn MyTrait`, + // but actually supporting that would "expand" to an infinitely-long type + // `fix $ τ → dyn MyTrait::MyOutput`. + // + // Instead, we force the user to write `dyn MyTrait`, + // which is uglier but works. See the discussion in #56288 for alternatives. + if !references_self { + // Include projections defined on supertraits, + projection_bounds.push((pred, DUMMY_SP)) + } } _ => () } diff --git a/src/test/ui/traits/trait-object-with-self-in-projection-output-bad.rs b/src/test/ui/traits/trait-object-with-self-in-projection-output-bad.rs new file mode 100644 index 0000000000000..b157ef6ed5d99 --- /dev/null +++ b/src/test/ui/traits/trait-object-with-self-in-projection-output-bad.rs @@ -0,0 +1,22 @@ +trait Base { + type Output; +} + +trait Helper: Base::Target> { + type Target; +} + +impl Base for u32 +{ + type Output = i32; +} + +impl Helper for u32 +{ + type Target = i32; +} + +fn main() { + let _x: Box> = Box::new(2u32); + //~^ ERROR the value of the associated type `Output` (from the trait `Base`) must be specified +} diff --git a/src/test/ui/traits/trait-object-with-self-in-projection-output-bad.stderr b/src/test/ui/traits/trait-object-with-self-in-projection-output-bad.stderr new file mode 100644 index 0000000000000..c2e274b0245f7 --- /dev/null +++ b/src/test/ui/traits/trait-object-with-self-in-projection-output-bad.stderr @@ -0,0 +1,12 @@ +error[E0191]: the value of the associated type `Output` (from the trait `Base`) must be specified + --> $DIR/trait-object-with-self-in-projection-output-bad.rs:20:17 + | +LL | type Output; + | ------------ `Output` defined here +... +LL | let _x: Box> = Box::new(2u32); + | ^^^^^^^^^^^^^^^^^^^^^^ associated type `Output` must be specified + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0191`. diff --git a/src/test/ui/traits/trait-object-with-self-in-projection-output-good.rs b/src/test/ui/traits/trait-object-with-self-in-projection-output-good.rs new file mode 100644 index 0000000000000..07681086e16ef --- /dev/null +++ b/src/test/ui/traits/trait-object-with-self-in-projection-output-good.rs @@ -0,0 +1,23 @@ +// compile-pass + +trait Base { + type Output; +} + +trait Helper: Base::Target> { + type Target; +} + +impl Base for u32 +{ + type Output = i32; +} + +impl Helper for u32 +{ + type Target = i32; +} + +fn main() { + let _x: Box> = Box::new(2u32); +} From 1fd23f56f372d02ed0b909bc7d2ff9ac8f2a82d1 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Tue, 18 Dec 2018 00:33:21 +0200 Subject: [PATCH 2/2] improve tests as suggested by review comments --- ...ject-with-self-in-projection-output-bad.rs | 28 +++++++++++ ...-with-self-in-projection-output-bad.stderr | 13 ++++- ...ect-with-self-in-projection-output-good.rs | 5 ++ ...n-projection-output-repeated-supertrait.rs | 48 +++++++++++++++++++ 4 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/traits/trait-object-with-self-in-projection-output-repeated-supertrait.rs diff --git a/src/test/ui/traits/trait-object-with-self-in-projection-output-bad.rs b/src/test/ui/traits/trait-object-with-self-in-projection-output-bad.rs index b157ef6ed5d99..766bd147431b4 100644 --- a/src/test/ui/traits/trait-object-with-self-in-projection-output-bad.rs +++ b/src/test/ui/traits/trait-object-with-self-in-projection-output-bad.rs @@ -1,3 +1,7 @@ +// Regression test for #56288. Checks that if a supertrait defines an associated type +// projection that references `Self`, then that associated type must still be explicitly +// specified in the `dyn Trait` variant, since we don't know what `Self` is anymore. + trait Base { type Output; } @@ -16,7 +20,31 @@ impl Helper for u32 type Target = i32; } +trait ConstI32 { + type Out; +} + +impl ConstI32 for T { + type Out = i32; +} + +// Test that you still need to manually give a projection type if the Output type +// is normalizable. +trait NormalizableHelper: + Base::Out> +{ + type Target; +} + +impl NormalizableHelper for u32 +{ + type Target = i32; +} + fn main() { let _x: Box> = Box::new(2u32); //~^ ERROR the value of the associated type `Output` (from the trait `Base`) must be specified + + let _y: Box> = Box::new(2u32); + //~^ ERROR the value of the associated type `Output` (from the trait `Base`) must be specified } diff --git a/src/test/ui/traits/trait-object-with-self-in-projection-output-bad.stderr b/src/test/ui/traits/trait-object-with-self-in-projection-output-bad.stderr index c2e274b0245f7..350f8ea850709 100644 --- a/src/test/ui/traits/trait-object-with-self-in-projection-output-bad.stderr +++ b/src/test/ui/traits/trait-object-with-self-in-projection-output-bad.stderr @@ -1,5 +1,5 @@ error[E0191]: the value of the associated type `Output` (from the trait `Base`) must be specified - --> $DIR/trait-object-with-self-in-projection-output-bad.rs:20:17 + --> $DIR/trait-object-with-self-in-projection-output-bad.rs:45:17 | LL | type Output; | ------------ `Output` defined here @@ -7,6 +7,15 @@ LL | type Output; LL | let _x: Box> = Box::new(2u32); | ^^^^^^^^^^^^^^^^^^^^^^ associated type `Output` must be specified -error: aborting due to previous error +error[E0191]: the value of the associated type `Output` (from the trait `Base`) must be specified + --> $DIR/trait-object-with-self-in-projection-output-bad.rs:48:17 + | +LL | type Output; + | ------------ `Output` defined here +... +LL | let _y: Box> = Box::new(2u32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ associated type `Output` must be specified + +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0191`. diff --git a/src/test/ui/traits/trait-object-with-self-in-projection-output-good.rs b/src/test/ui/traits/trait-object-with-self-in-projection-output-good.rs index 07681086e16ef..793d556d08ca2 100644 --- a/src/test/ui/traits/trait-object-with-self-in-projection-output-good.rs +++ b/src/test/ui/traits/trait-object-with-self-in-projection-output-good.rs @@ -1,5 +1,10 @@ // compile-pass +// Regression test related to #56288. Check that a supertrait projection (of +// `Output`) that references `Self` can be ok if it is referencing a projection (of +// `Self::Target`, in this case). Note that we still require the user to manually +// specify both `Target` and `Output` for now. + trait Base { type Output; } diff --git a/src/test/ui/traits/trait-object-with-self-in-projection-output-repeated-supertrait.rs b/src/test/ui/traits/trait-object-with-self-in-projection-output-repeated-supertrait.rs new file mode 100644 index 0000000000000..46c083f930591 --- /dev/null +++ b/src/test/ui/traits/trait-object-with-self-in-projection-output-repeated-supertrait.rs @@ -0,0 +1,48 @@ +// compile-pass + +// Regression test related to #56288. Check that a supertrait projection (of +// `Output`) that references `Self` is ok if there is another occurence of +// the same supertrait that specifies the projection explicitly, even if +// the projection's associated type is not explicitly specified in the object type. +// +// Note that in order for this to compile, we need the `Self`-referencing projection +// to normalize fairly directly to a concrete type, otherwise the trait resolver +// will hate us. +// +// There is a test in `trait-object-with-self-in-projection-output-bad.rs` that +// having a normalizing, but `Self`-containing projection does not *by itself* +// allow you to avoid writing the projected type (`Output`, in this example) +// explicitly. + +trait ConstI32 { + type Out; +} + +impl ConstI32 for T { + type Out = i32; +} + +trait Base { + type Output; +} + +trait NormalizingHelper: Base::Out> + Base { + type Target; +} + +impl Base for u32 +{ + type Output = i32; +} + +impl NormalizingHelper for u32 +{ + type Target = i32; +} + +fn main() { + // Make sure this works both with and without the associated type + // being specified. + let _x: Box> = Box::new(2u32); + let _y: Box> = Box::new(2u32); +}