Rollup merge of rust-lang#107082 - dtolnay:autotraits, r=lcnr

Autotrait bounds on dyn-safe trait methods

This PR is a successor to rust-lang#106604 implementing the approach encouraged by rust-lang#106604 (comment).

**I propose making it legal to use autotraits as trait bounds on the `Self` type of trait methods in a trait object.** rust-lang#51443 (comment) justifies why this use case is particularly important in the context of the async-trait crate.

```rust
#![feature(auto_traits)]
#![deny(where_clauses_object_safety)]

auto trait AutoTrait {}

trait MyTrait {
    fn f(&self) where Self: AutoTrait;
}

fn main() {
    let _: &dyn MyTrait;
}
```

Previously this would fail with:

```console
error: the trait `MyTrait` cannot be made into an object
 --> src/main.rs:7:8
  |
7 |     fn f(&self) where Self: AutoTrait;
  |        ^
  |
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue rust-lang#51443 <rust-lang#51443>
note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
 --> src/main.rs:7:8
  |
6 | trait MyTrait {
  |       ------- this trait cannot be made into an object...
7 |     fn f(&self) where Self: AutoTrait;
  |        ^ ...because method `f` references the `Self` type in its `where` clause
  = help: consider moving `f` to another trait
```

In order for this to be sound without hitting rust-lang#50781, **I further propose that we disallow handwritten autotrait impls that apply to trait objects.** Both of the following were previously allowed (_on nightly_) and no longer allowed in my proposal:

```rust
auto trait AutoTrait {}

trait MyTrait {}
impl AutoTrait for dyn MyTrait {}  // NOT ALLOWED

impl<T: ?Sized> AutoTrait for T {}  // NOT ALLOWED
```

(`impl<T> AutoTrait for T {}` remains allowed.)

After this change, traits with a default impl are implemented for a trait object **if and only if** the autotrait is one of the trait object's trait bounds (or a supertrait of a bound). In other words `dyn Trait + AutoTrait` always implements AutoTrait while `dyn Trait` never implements AutoTrait.

Fixes dtolnay/async-trait#228.

r? `@lcnr`

Author: Dylan-DPC

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 {

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,193 @@ 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
+        //
+        // With this restriction, it's guaranteed that an auto-trait is
+        // implemented for a trait object if and only if the auto-trait is one
+        // of the trait object's trait bounds (or a supertrait of a bound). In
+        // other words `dyn Trait + AutoTrait` always implements AutoTrait,
+        // while `dyn Trait` never implements AutoTrait.
+        //
+        // This is necessary in order for autotrait bounds on methods of trait
+        // objects to be sound.
+        //
+        //     auto trait AutoTrait {}
+        //
+        //     trait ObjectSafeTrait {
+        //         fn f(&self) where Self: AutoTrait;
+        //     }
+        //
+        // We can allow f to be called on `dyn ObjectSafeTrait + AutoTrait`.
+        //
+        // If we didn't deny `impl AutoTrait for dyn Trait`, it would be unsound
+        // for the ObjectSafeTrait shown above to be object safe because someone
+        // could take some type implementing ObjectSafeTrait but not AutoTrait,
+        // unsize it to `dyn ObjectSafeTrait`, and call .f() which has no
+        // concrete implementation (issue #50781).
+        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);
+            }
         }
     }
 

compiler/rustc_trait_selection/src/traits/object_safety.rs

@@ -547,16 +547,56 @@ fn virtual_call_violation_for_method<'tcx>(
 
     // NOTE: This check happens last, because it results in a lint, and not a
     // hard error.
-    if tcx
-        .predicates_of(method.def_id)
-        .predicates
-        .iter()
-        // A trait object can't claim to live more than the concrete type,
-        // so outlives predicates will always hold.
-        .cloned()
-        .filter(|(p, _)| p.to_opt_type_outlives().is_none())
-        .any(|pred| contains_illegal_self_type_reference(tcx, trait_def_id, pred))
-    {
+    if tcx.predicates_of(method.def_id).predicates.iter().any(|&(pred, span)| {
+        // dyn Trait is okay:
+        //
+        //     trait Trait {
+        //         fn f(&self) where Self: 'static;
+        //     }
+        //
+        // because a trait object can't claim to live longer than the concrete
+        // type. If the lifetime bound holds on dyn Trait then it's guaranteed
+        // to hold as well on the concrete type.
+        if pred.to_opt_type_outlives().is_some() {
+            return false;
+        }
+
+        // dyn Trait is okay:
+        //
+        //     auto trait AutoTrait {}
+        //
+        //     trait Trait {
+        //         fn f(&self) where Self: AutoTrait;
+        //     }
+        //
+        // because `impl AutoTrait for dyn Trait` is disallowed by coherence.
+        // Traits with a default impl are implemented for a trait object if and
+        // only if the autotrait is one of the trait object's trait bounds, like
+        // in `dyn Trait + AutoTrait`. This guarantees that trait objects only
+        // implement auto traits if the underlying type does as well.
+        if let ty::PredicateKind::Clause(ty::Clause::Trait(ty::TraitPredicate {
+            trait_ref: pred_trait_ref,
+            constness: ty::BoundConstness::NotConst,
+            polarity: ty::ImplPolarity::Positive,
+        })) = pred.kind().skip_binder()
+            && pred_trait_ref.self_ty() == tcx.types.self_param
+            && tcx.trait_is_auto(pred_trait_ref.def_id)
+        {
+            // Consider bounds like `Self: Bound<Self>`. Auto traits are not
+            // allowed to have generic parameters so `auto trait Bound<T> {}`
+            // would already have reported an error at the definition of the
+            // auto trait.
+            if pred_trait_ref.substs.len() != 1 {
+                tcx.sess.diagnostic().delay_span_bug(
+                    span,
+                    "auto traits cannot have generic parameters",
+                );
+            }
+            return false;
+        }
+
+        contains_illegal_self_type_reference(tcx, trait_def_id, pred.clone())
+    }) {
         return Some(MethodViolationCode::WhereClauseReferencesSelf);
     }
 

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() {}