Disallow impl autotrait for trait object

dtolnay · dtolnay · commit a2479523c58c · 2023-01-19T13:09:34.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
@@ -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,93 @@ 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 self_ty_kind = self_ty.kind();
 
-        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
-                } 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",
-                    ))
+        if trait_def_id.is_local() {
+            // If the auto-trait is local, 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
+            //
+            let problematic_kind = match self_ty_kind {
+                ty::Dynamic(..) => Some("trait object"),
+                ty::Param(..) if !self_ty.is_sized(tcx, tcx.param_env(def_id)) => {
+                    Some("generic type")
                 }
+                _ => None,
+            };
+
+            if let Some(problematic_kind) = 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 the auto-trait is not local, 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
+            //
+            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 {
+                Some(self_def_id) => {
+                    if self_def_id.is_local() {
+                        None
+                    } 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",
+                        ))
+                    }
+                }
+                None => 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",
-            )),
-        };
+                        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);
+            if let Some((msg, label)) = msg {
+                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
@@ -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-positive.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-positive.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-positive.rs:17:1
+  --> $DIR/coherence-impl-trait-for-marker-trait-positive.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-positive.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-positive.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-positive.rs:23:1
+  --> $DIR/coherence-impl-trait-for-marker-trait-positive.rs:22:1
    |
 LL | unsafe 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 | unsafe 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-positive.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`.
