From de6566ce391478c5c483fed98eaf6e3c37f4dab9 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sat, 5 Jan 2019 16:19:34 +0200 Subject: [PATCH 1/2] forbid manually impl'ing one of an object type's marker traits This shouldn't break compatibility for crates that do not use `feature(optin_builtin_traits)`, because as the test shows, it is only possible to impl a marker trait for a trait object in the crate the marker trait is defined in, which must define `feature(optin_builtin_traits)`. Fixes #56934 --- src/librustc_typeck/coherence/mod.rs | 17 +++++++-- ...ce-impl-trait-for-marker-trait-negative.rs | 29 +++++++++++++++ ...mpl-trait-for-marker-trait-negative.stderr | 37 +++++++++++++++++++ ...ce-impl-trait-for-marker-trait-positive.rs | 29 +++++++++++++++ ...mpl-trait-for-marker-trait-positive.stderr | 37 +++++++++++++++++++ 5 files changed, 145 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/coherence/coherence-impl-trait-for-marker-trait-negative.rs create mode 100644 src/test/ui/coherence/coherence-impl-trait-for-marker-trait-negative.stderr create mode 100644 src/test/ui/coherence/coherence-impl-trait-for-marker-trait-positive.rs create mode 100644 src/test/ui/coherence/coherence-impl-trait-for-marker-trait-positive.stderr diff --git a/src/librustc_typeck/coherence/mod.rs b/src/librustc_typeck/coherence/mod.rs index ce71be07efd42..8053ed130e91b 100644 --- a/src/librustc_typeck/coherence/mod.rs +++ b/src/librustc_typeck/coherence/mod.rs @@ -171,13 +171,23 @@ fn check_impl_overlap<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, node_id: ast::NodeI // This is something like impl Trait1 for Trait2. Illegal // if Trait1 is a supertrait of Trait2 or Trait2 is not object safe. - if let Some(principal_def_id) = data.principal_def_id() { - if !tcx.is_object_safe(principal_def_id) { + let component_def_ids = data.iter().flat_map(|predicate| { + match predicate.skip_binder() { + ty::ExistentialPredicate::Trait(tr) => Some(tr.def_id), + ty::ExistentialPredicate::AutoTrait(def_id) => Some(*def_id), + // An associated type projection necessarily comes with + // an additional `Trait` requirement. + ty::ExistentialPredicate::Projection(..) => None, + } + }); + + for component_def_id in component_def_ids { + if !tcx.is_object_safe(component_def_id) { // This is an error, but it will be reported by wfcheck. Ignore it here. // This is tested by `coherence-impl-trait-for-trait-object-safe.rs`. } else { let mut supertrait_def_ids = - traits::supertrait_def_ids(tcx, principal_def_id); + traits::supertrait_def_ids(tcx, component_def_id); if supertrait_def_ids.any(|d| d == trait_def_id) { let sp = tcx.sess.source_map().def_span(tcx.span_of_impl(impl_def_id).unwrap()); struct_span_err!(tcx.sess, @@ -193,6 +203,5 @@ fn check_impl_overlap<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, node_id: ast::NodeI } } } - // FIXME: also check auto-trait def-ids? (e.g. `impl Sync for Foo+Sync`)? } } diff --git a/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-negative.rs b/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-negative.rs new file mode 100644 index 0000000000000..5ea69190951e1 --- /dev/null +++ b/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-negative.rs @@ -0,0 +1,29 @@ +#![feature(optin_builtin_traits)] + +// Test for issue #56934 - that it is impossible to redundantly +// implement an auto-trait for a trait object type that contains it. + +// Negative impl variant. + +auto trait Marker1 {} +auto trait Marker2 {} + +trait Object: Marker1 {} + +// A supertrait marker is illegal... +impl !Marker1 for dyn Object + Marker2 { } //~ ERROR E0371 +// ...and also a direct component. +impl !Marker2 for dyn Object + Marker2 { } //~ ERROR E0371 + +// But implementing a marker if it is not present is OK. +impl !Marker2 for dyn Object {} // OK + +// A non-principal trait-object type is orphan even in its crate. +impl !Send for dyn Marker2 {} //~ ERROR E0117 + +// And impl'ing a remote marker for a local trait object is forbidden +// by one of these special orphan-like rules. +impl !Send for dyn Object {} //~ ERROR E0321 +impl !Send for dyn Object + Marker2 {} //~ ERROR E0321 + +fn main() { } diff --git a/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-negative.stderr b/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-negative.stderr new file mode 100644 index 0000000000000..6e146760db5e3 --- /dev/null +++ b/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-negative.stderr @@ -0,0 +1,37 @@ +error[E0371]: the object type `(dyn Object + Marker2 + 'static)` automatically implements the trait `Marker1` + --> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:14:1 + | +LL | impl !Marker1 for dyn Object + Marker2 { } //~ ERROR E0371 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `(dyn Object + Marker2 + 'static)` automatically implements trait `Marker1` + +error[E0371]: the object type `(dyn Object + Marker2 + 'static)` automatically implements the trait `Marker2` + --> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:16:1 + | +LL | impl !Marker2 for dyn Object + Marker2 { } //~ ERROR E0371 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `(dyn Object + Marker2 + 'static)` automatically implements trait `Marker2` + +error[E0117]: only traits defined in the current crate can be implemented for arbitrary types + --> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:22:1 + | +LL | impl !Send for dyn Marker2 {} //~ ERROR E0117 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ impl doesn't use types inside crate + | + = note: the impl does not reference any types defined in this crate + = note: define and implement a trait or new type instead + +error[E0321]: cross-crate traits with a default impl, like `std::marker::Send`, can only be implemented for a struct/enum type, not `(dyn Object + 'static)` + --> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:26:1 + | +LL | impl !Send for dyn Object {} //~ ERROR E0321 + | ^^^^^^^^^^^^^^^^^^^^^^^^^ can't implement cross-crate trait with a default impl for non-struct/enum type + +error[E0321]: cross-crate traits with a default impl, like `std::marker::Send`, can only be implemented for a struct/enum type, not `(dyn Object + Marker2 + 'static)` + --> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:27:1 + | +LL | impl !Send for dyn Object + Marker2 {} //~ ERROR E0321 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't implement cross-crate trait with a default impl for non-struct/enum type + +error: aborting due to 5 previous errors + +Some errors occurred: E0117, E0321, E0371. +For more information about an error, try `rustc --explain E0117`. diff --git a/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-positive.rs b/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-positive.rs new file mode 100644 index 0000000000000..6b5689e8260f0 --- /dev/null +++ b/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-positive.rs @@ -0,0 +1,29 @@ +#![feature(optin_builtin_traits)] + +// Test for issue #56934 - that it is impossible to redundantly +// implement an auto-trait for a trait object type that contains it. + +// Positive impl variant. + +auto trait Marker1 {} +auto trait Marker2 {} + +trait Object: Marker1 {} + +// A supertrait marker is illegal... +impl Marker1 for dyn Object + Marker2 { } //~ ERROR E0371 +// ...and also a direct component. +impl Marker2 for dyn Object + Marker2 { } //~ ERROR E0371 + +// But implementing a marker if it is not present is OK. +impl Marker2 for dyn Object {} // OK + +// A non-principal trait-object type is orphan even in its crate. +unsafe impl Send for dyn Marker2 {} //~ ERROR E0117 + +// And impl'ing a remote marker for a local trait object is forbidden +// by one of these special orphan-like rules. +unsafe impl Send for dyn Object {} //~ ERROR E0321 +unsafe impl Send for dyn Object + Marker2 {} //~ ERROR E0321 + +fn main() { } diff --git a/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-positive.stderr b/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-positive.stderr new file mode 100644 index 0000000000000..4a8347613eb13 --- /dev/null +++ b/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-positive.stderr @@ -0,0 +1,37 @@ +error[E0371]: the object type `(dyn Object + Marker2 + 'static)` automatically implements the trait `Marker1` + --> $DIR/coherence-impl-trait-for-marker-trait-positive.rs:14:1 + | +LL | impl Marker1 for dyn Object + Marker2 { } //~ ERROR E0371 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `(dyn Object + Marker2 + 'static)` automatically implements trait `Marker1` + +error[E0371]: the object type `(dyn Object + Marker2 + 'static)` automatically implements the trait `Marker2` + --> $DIR/coherence-impl-trait-for-marker-trait-positive.rs:16:1 + | +LL | impl Marker2 for dyn Object + Marker2 { } //~ ERROR E0371 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `(dyn Object + Marker2 + 'static)` automatically implements trait `Marker2` + +error[E0117]: only traits defined in the current crate can be implemented for arbitrary types + --> $DIR/coherence-impl-trait-for-marker-trait-positive.rs:22:1 + | +LL | unsafe impl Send for dyn Marker2 {} //~ ERROR E0117 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ impl doesn't use types inside crate + | + = note: the impl does not reference any types defined in this crate + = note: define and implement a trait or new type instead + +error[E0321]: cross-crate traits with a default impl, like `std::marker::Send`, can only be implemented for a struct/enum type, not `(dyn Object + 'static)` + --> $DIR/coherence-impl-trait-for-marker-trait-positive.rs:26:1 + | +LL | unsafe impl Send for dyn Object {} //~ ERROR E0321 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't implement cross-crate trait with a default impl for non-struct/enum type + +error[E0321]: cross-crate traits with a default impl, like `std::marker::Send`, can only be implemented for a struct/enum type, not `(dyn Object + Marker2 + 'static)` + --> $DIR/coherence-impl-trait-for-marker-trait-positive.rs:27:1 + | +LL | unsafe impl Send for dyn Object + Marker2 {} //~ ERROR E0321 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't implement cross-crate trait with a default impl for non-struct/enum type + +error: aborting due to 5 previous errors + +Some errors occurred: E0117, E0321, E0371. +For more information about an error, try `rustc --explain E0117`. From d38a59f8b5ad8ddbed5294bb4755618d7c2802aa Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Tue, 15 Jan 2019 00:26:50 +0200 Subject: [PATCH 2/2] fix test output changing in rebase --- .../coherence-impl-trait-for-marker-trait-negative.stderr | 2 +- .../coherence-impl-trait-for-marker-trait-positive.stderr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-negative.stderr b/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-negative.stderr index 6e146760db5e3..c8a146cdd4456 100644 --- a/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-negative.stderr +++ b/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-negative.stderr @@ -16,7 +16,7 @@ error[E0117]: only traits defined in the current crate can be implemented for ar LL | impl !Send for dyn Marker2 {} //~ ERROR E0117 | ^^^^^^^^^^^^^^^^^^^^^^^^^^ impl doesn't use types inside crate | - = note: the impl does not reference any types defined in this crate + = note: the impl does not reference only types defined in this crate = note: define and implement a trait or new type instead error[E0321]: cross-crate traits with a default impl, like `std::marker::Send`, can only be implemented for a struct/enum type, not `(dyn Object + 'static)` diff --git a/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-positive.stderr b/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-positive.stderr index 4a8347613eb13..78ca2f5279d63 100644 --- a/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-positive.stderr +++ b/src/test/ui/coherence/coherence-impl-trait-for-marker-trait-positive.stderr @@ -16,7 +16,7 @@ error[E0117]: only traits defined in the current crate can be implemented for ar LL | unsafe impl Send for dyn Marker2 {} //~ ERROR E0117 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ impl doesn't use types inside crate | - = note: the impl does not reference any types defined in this crate + = note: the impl does not reference only types defined in this crate = note: define and implement a trait or new type instead error[E0321]: cross-crate traits with a default impl, like `std::marker::Send`, can only be implemented for a struct/enum type, not `(dyn Object + 'static)`