Disallow impl autotrait for trait object

dtolnay · dtolnay · commit 9e1c600f74f9 · 2023-02-03T08:33:40.000-08:00
diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs
@@ -31,8 +31,8 @@ cfg_if! {
         pub auto trait Send {}
         pub auto trait Sync {}
 
-        impl<T: ?Sized> Send for T {}
-        impl<T: ?Sized> Sync for T {}
+        impl<T> Send for T {}
+        impl<T> Sync for T {}
 
         #[macro_export]
         macro_rules! rustc_erase_owner {
diff --git a/compiler/rustc_hir_analysis/src/coherence/orphan.rs b/compiler/rustc_hir_analysis/src/coherence/orphan.rs
@@ -8,7 +8,7 @@ use rustc_hir as hir;
 use rustc_middle::ty::subst::InternalSubsts;
 use rustc_middle::ty::util::IgnoreRegions;
 use rustc_middle::ty::{
-    self, ImplPolarity, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor,
+    self, AliasKind, ImplPolarity, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor,
 };
 use rustc_session::lint;
 use rustc_span::def_id::{DefId, LocalDefId};
@@ -86,7 +86,7 @@ fn do_orphan_check_impl<'tcx>(
     // struct B { }
     // impl Foo for A { }
     // impl Foo for B { }
-    // impl !Send for (A, B) { }
+    // impl !Foo for (A, B) { }
     // ```
     //
     // This final impl is legal according to the orphan
@@ -99,50 +99,171 @@ fn do_orphan_check_impl<'tcx>(
         tcx.trait_is_auto(trait_def_id)
     );
 
-    if tcx.trait_is_auto(trait_def_id) && !trait_def_id.is_local() {
+    if tcx.trait_is_auto(trait_def_id) {
         let self_ty = trait_ref.self_ty();
-        let opt_self_def_id = match *self_ty.kind() {
-            ty::Adt(self_def, _) => Some(self_def.did()),
-            ty::Foreign(did) => Some(did),
-            _ => None,
-        };
 
-        let msg = match opt_self_def_id {
-            // We only want to permit nominal types, but not *all* nominal types.
-            // They must be local to the current crate, so that people
-            // can't do `unsafe impl Send for Rc<SomethingLocal>` or
-            // `impl !Send for Box<SomethingLocalAndSend>`.
-            Some(self_def_id) => {
-                if self_def_id.is_local() {
-                    None
+        // If the impl is in the same crate as the auto-trait, almost anything
+        // goes.
+        //
+        //     impl MyAuto for Rc<Something> {}  // okay
+        //     impl<T> !MyAuto for *const T {}   // okay
+        //     impl<T> MyAuto for T {}           // okay
+        //
+        // But there is one important exception: implementing for a trait object
+        // is not allowed.
+        //
+        //     impl MyAuto for dyn Trait {}      // NOT OKAY
+        //     impl<T: ?Sized> MyAuto for T {}   // NOT OKAY
+        //
+        enum LocalImpl {
+            Allow,
+            Disallow { problematic_kind: &'static str },
+        }
+
+        // If the auto-trait is from a dependency, it must only be getting
+        // implemented for a nominal type, and specifically one local to the
+        // current crate.
+        //
+        //     impl<T> Sync for MyStruct<T> {}   // okay
+        //
+        //     impl Sync for Rc<MyStruct> {}     // NOT OKAY
+        enum NonlocalImpl {
+            Allow,
+            DisallowBecauseNonlocal,
+            DisallowOther,
+        }
+
+        // Exhaustive match considering that this logic is essential for
+        // soundness.
+        let (local_impl, nonlocal_impl) = match self_ty.kind() {
+            // struct Struct<T>;
+            // impl AutoTrait for Struct<Foo> {}
+            ty::Adt(self_def, _) => (
+                LocalImpl::Allow,
+                if self_def.did().is_local() {
+                    NonlocalImpl::Allow
+                } else {
+                    NonlocalImpl::DisallowBecauseNonlocal
+                },
+            ),
+
+            // extern { type OpaqueType; }
+            // impl AutoTrait for OpaqueType {}
+            ty::Foreign(did) => (
+                LocalImpl::Allow,
+                if did.is_local() {
+                    NonlocalImpl::Allow
                 } else {
-                    Some((
-                        format!(
-                            "cross-crate traits with a default impl, like `{}`, \
-                                    can only be implemented for a struct/enum type \
-                                    defined in the current crate",
-                            tcx.def_path_str(trait_def_id)
-                        ),
-                        "can't implement cross-crate trait for type in another crate",
-                    ))
+                    NonlocalImpl::DisallowBecauseNonlocal
+                },
+            ),
+
+            // impl AutoTrait for dyn Trait {}
+            ty::Dynamic(..) => (
+                LocalImpl::Disallow { problematic_kind: "trait object" },
+                NonlocalImpl::DisallowOther,
+            ),
+
+            // impl<T> AutoTrait for T {}
+            // impl<T: ?Sized> AutoTrait for T {}
+            ty::Param(..) => (
+                if self_ty.is_sized(tcx, tcx.param_env(def_id)) {
+                    LocalImpl::Allow
+                } else {
+                    LocalImpl::Disallow { problematic_kind: "generic type" }
+                },
+                NonlocalImpl::DisallowOther,
+            ),
+
+            // trait Id { type This: ?Sized; }
+            // impl<T: ?Sized> Id for T {
+            //     type This = T;
+            // }
+            // impl<T: ?Sized> AutoTrait for <T as Id>::This {}
+            ty::Alias(AliasKind::Projection, _) => (
+                LocalImpl::Disallow { problematic_kind: "associated type" },
+                NonlocalImpl::DisallowOther,
+            ),
+
+            // type Opaque = impl Trait;
+            // impl AutoTrait for Opaque {}
+            ty::Alias(AliasKind::Opaque, _) => (
+                LocalImpl::Disallow { problematic_kind: "opaque type" },
+                NonlocalImpl::DisallowOther,
+            ),
+
+            ty::Bool
+            | ty::Char
+            | ty::Int(..)
+            | ty::Uint(..)
+            | ty::Float(..)
+            | ty::Str
+            | ty::Array(..)
+            | ty::Slice(..)
+            | ty::RawPtr(..)
+            | ty::Ref(..)
+            | ty::FnDef(..)
+            | ty::FnPtr(..)
+            | ty::Never
+            | ty::Tuple(..) => (LocalImpl::Allow, NonlocalImpl::DisallowOther),
+
+            ty::Closure(..)
+            | ty::Generator(..)
+            | ty::GeneratorWitness(..)
+            | ty::GeneratorWitnessMIR(..)
+            | ty::Bound(..)
+            | ty::Placeholder(..)
+            | ty::Infer(..) => span_bug!(sp, "weird self type for autotrait impl"),
+
+            ty::Error(..) => (LocalImpl::Allow, NonlocalImpl::Allow),
+        };
+
+        if trait_def_id.is_local() {
+            match local_impl {
+                LocalImpl::Allow => {}
+                LocalImpl::Disallow { problematic_kind } => {
+                    let msg = format!(
+                        "traits with a default impl, like `{trait}`, \
+                                cannot be implemented for {problematic_kind} `{self_ty}`",
+                        trait = tcx.def_path_str(trait_def_id),
+                    );
+                    let label = format!(
+                        "a trait object implements `{trait}` if and only if `{trait}` \
+                                is one of the trait object's trait bounds",
+                        trait = tcx.def_path_str(trait_def_id),
+                    );
+                    let reported =
+                        struct_span_err!(tcx.sess, sp, E0321, "{}", msg).note(label).emit();
+                    return Err(reported);
                 }
             }
-            _ => Some((
-                format!(
-                    "cross-crate traits with a default impl, like `{}`, can \
+        } else {
+            if let Some((msg, label)) = match nonlocal_impl {
+                NonlocalImpl::Allow => None,
+                NonlocalImpl::DisallowBecauseNonlocal => Some((
+                    format!(
+                        "cross-crate traits with a default impl, like `{}`, \
+                                can only be implemented for a struct/enum type \
+                                defined in the current crate",
+                        tcx.def_path_str(trait_def_id)
+                    ),
+                    "can't implement cross-crate trait for type in another crate",
+                )),
+                NonlocalImpl::DisallowOther => Some((
+                    format!(
+                        "cross-crate traits with a default impl, like `{}`, can \
                                 only be implemented for a struct/enum type, not `{}`",
-                    tcx.def_path_str(trait_def_id),
-                    self_ty
-                ),
-                "can't implement cross-crate trait with a default impl for \
-                        non-struct/enum type",
-            )),
-        };
-
-        if let Some((msg, label)) = msg {
-            let reported =
-                struct_span_err!(tcx.sess, sp, E0321, "{}", msg).span_label(sp, label).emit();
-            return Err(reported);
+                        tcx.def_path_str(trait_def_id),
+                        self_ty
+                    ),
+                    "can't implement cross-crate trait with a default impl for \
+                            non-struct/enum type",
+                )),
+            } {
+                let reported =
+                    struct_span_err!(tcx.sess, sp, E0321, "{}", msg).span_label(sp, label).emit();
+                return Err(reported);
+            }
         }
     }
 
diff --git a/tests/ui/coherence/coherence-impl-trait-for-marker-trait-negative.rs b/tests/ui/coherence/coherence-impl-trait-for-marker-trait-negative.rs
@@ -12,19 +12,26 @@ auto trait Marker2 {}
 trait Object: Marker1 {}
 
 // A supertrait marker is illegal...
-impl !Marker1 for dyn Object + Marker2 { }   //~ ERROR E0371
+impl !Marker1 for dyn Object + Marker2 {} //~ ERROR E0371
+                                          //~^ ERROR 0321
 // ...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
+impl !Marker2 for dyn Object + Marker2 {} //~ ERROR E0371
+                                          //~^ ERROR 0321
 
 // 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.
+// Implementing a marker for a local trait object is forbidden by a special
+// orphan-like rule.
+impl !Marker2 for dyn Object {} //~ ERROR E0321
 impl !Send for dyn Object {} //~ ERROR E0321
 impl !Send for dyn Object + Marker2 {} //~ ERROR E0321
 
-fn main() { }
+// Blanket impl that applies to dyn Object is equally problematic.
+auto trait Marker3 {}
+impl<T: ?Sized> !Marker3 for T {} //~ ERROR E0321
+
+auto trait Marker4 {}
+impl<T> !Marker4 for T {} // okay
+
+fn main() {}
diff --git a/tests/ui/coherence/coherence-impl-trait-for-marker-trait-negative.stderr b/tests/ui/coherence/coherence-impl-trait-for-marker-trait-negative.stderr
@@ -1,17 +1,41 @@
 error[E0371]: the object type `(dyn Object + Marker2 + 'static)` automatically implements the trait `Marker1`
   --> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:15:1
    |
-LL | impl !Marker1 for dyn Object + Marker2 { }
+LL | impl !Marker1 for dyn Object + Marker2 {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `(dyn Object + Marker2 + 'static)` automatically implements trait `Marker1`
 
+error[E0321]: traits with a default impl, like `Marker1`, cannot be implemented for trait object `(dyn Object + Marker2 + 'static)`
+  --> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:15:1
+   |
+LL | impl !Marker1 for dyn Object + Marker2 {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: a trait object implements `Marker1` if and only if `Marker1` is one of the trait object's trait bounds
+
 error[E0371]: the object type `(dyn Object + Marker2 + 'static)` automatically implements the trait `Marker2`
-  --> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:17:1
+  --> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:18:1
    |
-LL | impl !Marker2 for dyn Object + Marker2 { }
+LL | impl !Marker2 for dyn Object + Marker2 {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `(dyn Object + Marker2 + 'static)` automatically implements trait `Marker2`
 
+error[E0321]: traits with a default impl, like `Marker2`, cannot be implemented for trait object `(dyn Object + Marker2 + 'static)`
+  --> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:18:1
+   |
+LL | impl !Marker2 for dyn Object + Marker2 {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: a trait object implements `Marker2` if and only if `Marker2` is one of the trait object's trait bounds
+
+error[E0321]: traits with a default impl, like `Marker2`, cannot be implemented for trait object `(dyn Object + 'static)`
+  --> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:26:1
+   |
+LL | impl !Marker2 for dyn Object {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: a trait object implements `Marker2` if and only if `Marker2` is one of the trait object's trait bounds
+
 error[E0117]: only traits defined in the current crate can be implemented for arbitrary types
-  --> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:23:1
+  --> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:22:1
    |
 LL | impl !Send for dyn Marker2 {}
    | ^^^^^^^^^^^^^^^-----------
@@ -33,7 +57,15 @@ error[E0321]: cross-crate traits with a default impl, like `Send`, can only be i
 LL | impl !Send for dyn Object + Marker2 {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't implement cross-crate trait with a default impl for non-struct/enum type
 
-error: aborting due to 5 previous errors
+error[E0321]: traits with a default impl, like `Marker3`, cannot be implemented for generic type `T`
+  --> $DIR/coherence-impl-trait-for-marker-trait-negative.rs:32:1
+   |
+LL | impl<T: ?Sized> !Marker3 for T {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: a trait object implements `Marker3` if and only if `Marker3` is one of the trait object's trait bounds
+
+error: aborting due to 9 previous errors
 
 Some errors have detailed explanations: E0117, E0321, E0371.
 For more information about an error, try `rustc --explain E0117`.
diff --git a/tests/ui/coherence/coherence-impl-trait-for-marker-trait-positive.rs b/tests/ui/coherence/coherence-impl-trait-for-marker-trait-positive.rs
@@ -12,19 +12,26 @@ auto trait Marker2 {}
 trait Object: Marker1 {}
 
 // A supertrait marker is illegal...
-impl Marker1 for dyn Object + Marker2 { }   //~ ERROR E0371
+impl Marker1 for dyn Object + Marker2 {} //~ ERROR E0371
+                                         //~^ ERROR E0321
 // ...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
+impl Marker2 for dyn Object + Marker2 {} //~ ERROR E0371
+                                         //~^ ERROR E0321
 
 // 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.
+// Implementing a marker for a local trait object is forbidden by a special
+// orphan-like rule.
+impl Marker2 for dyn Object {} //~ ERROR E0321
 unsafe impl Send for dyn Object {} //~ ERROR E0321
 unsafe impl Send for dyn Object + Marker2 {} //~ ERROR E0321
 
-fn main() { }
+// Blanket impl that applies to dyn Object is equally problematic.
+auto trait Marker3 {}
+impl<T: ?Sized> Marker3 for T {} //~ ERROR E0321
+
+auto trait Marker4 {}
+impl<T> Marker4 for T {} // okay
+
+fn main() {}
diff --git a/tests/ui/coherence/coherence-impl-trait-for-marker-trait-positive.stderr b/tests/ui/coherence/coherence-impl-trait-for-marker-trait-positive.stderr
