Auto merge of #113312 - Ddystopia:auto-trait-fun, r=lcnr

discard default auto trait impls if explicit ones exist (rebase of #85048)

Rebase of #85048

compiler/rustc_lint_defs/src/builtin.rs

@@ -4052,12 +4052,12 @@ declare_lint! {
     ///
     /// The compiler disables the automatic implementation if an explicit one
     /// exists for given type constructor. The exact rules governing this
-    /// are currently unsound, quite subtle, and will be modified in the future.
-    /// This change will cause the automatic implementation to be disabled in more
+    /// were previously unsound, quite subtle, and have been recently modified.
+    /// This change caused the automatic implementation to be disabled in more
     /// cases, potentially breaking some code.
     pub SUSPICIOUS_AUTO_TRAIT_IMPLS,
     Warn,
-    "the rules governing auto traits will change in the future",
+    "the rules governing auto traits have recently changed resulting in potential breakage",
     @future_incompatible = FutureIncompatibleInfo {
         reason: FutureIncompatibilityReason::FutureReleaseSemanticsChange,
         reference: "issue #93367 <https://github.com/rust-lang/rust/issues/93367>",

compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs

@@ -124,11 +124,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
 
             self.assemble_candidates_from_projected_tys(obligation, &mut candidates);
             self.assemble_candidates_from_caller_bounds(stack, &mut candidates)?;
-            // Auto implementations have lower priority, so we only
-            // consider triggering a default if there is no other impl that can apply.
-            if candidates.vec.is_empty() {
-                self.assemble_candidates_from_auto_impls(obligation, &mut candidates);
-            }
+            self.assemble_candidates_from_auto_impls(obligation, &mut candidates);
         }
         debug!("candidate list size: {}", candidates.vec.len());
         Ok(candidates)
@@ -513,7 +509,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                     // for an example of a test case that exercises
                     // this path.
                 }
-                ty::Infer(ty::TyVar(_)) => {
+                ty::Infer(ty::TyVar(_) | ty::IntVar(_) | ty::FloatVar(_)) => {
                     // The auto impl might apply; we don't know.
                     candidates.ambiguous = true;
                 }
@@ -533,7 +529,63 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                     }
                 }
 
-                _ => candidates.vec.push(AutoImplCandidate),
+                ty::Infer(ty::FreshTy(_) | ty::FreshIntTy(_) | ty::FreshFloatTy(_)) => {
+                    bug!(
+                        "asked to assemble auto trait candidates of unexpected type: {:?}",
+                        self_ty
+                    );
+                }
+
+                ty::Alias(_, _)
+                    if candidates.vec.iter().any(|c| matches!(c, ProjectionCandidate(..))) =>
+                {
+                    // We do not generate an auto impl candidate for `impl Trait`s which already
+                    // reference our auto trait.
+                    //
+                    // For example during candidate assembly for `impl Send: Send`, we don't have
+                    // to look at the constituent types for this opaque types to figure out that this
+                    // trivially holds.
+                    //
+                    // Note that this is only sound as projection candidates of opaque types
+                    // are always applicable for auto traits.
+                }
+                ty::Alias(_, _) => candidates.vec.push(AutoImplCandidate),
+
+                ty::Bool
+                | ty::Char
+                | ty::Int(_)
+                | ty::Uint(_)
+                | ty::Float(_)
+                | ty::Str
+                | ty::Array(_, _)
+                | ty::Slice(_)
+                | ty::Adt(..)
+                | ty::RawPtr(_)
+                | ty::Ref(..)
+                | ty::FnDef(..)
+                | ty::FnPtr(_)
+                | ty::Closure(_, _)
+                | ty::Generator(..)
+                | ty::Never
+                | ty::Tuple(_)
+                | ty::GeneratorWitness(_)
+                | ty::GeneratorWitnessMIR(..) => {
+                    // Only consider auto impls if there are no manual impls for the root of `self_ty`.
+                    //
+                    // For example, we only consider auto candidates for `&i32: Auto` if no explicit impl
+                    // for `&SomeType: Auto` exists. Due to E0321 the only crate where impls
+                    // for `&SomeType: Auto` can be defined is the crate where `Auto` has been defined.
+                    //
+                    // Generally, we have to guarantee that for all `SimplifiedType`s the only crate
+                    // which may define impls for that type is either the crate defining the type
+                    // or the trait. This should be guaranteed by the orphan check.
+                    let mut has_impl = false;
+                    self.tcx().for_each_relevant_impl(def_id, self_ty, |_| has_impl = true);
+                    if !has_impl {
+                        candidates.vec.push(AutoImplCandidate)
+                    }
+                }
+                ty::Error(_) => {} // do not add an auto trait impl for `ty::Error` for now.
             }
         }
     }

compiler/rustc_trait_selection/src/traits/select/mod.rs

@@ -1783,6 +1783,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
     /// candidates and prefer where-clause candidates.
     ///
     /// See the comment for "SelectionCandidate" for more details.
+    #[instrument(level = "debug", skip(self))]
     fn candidate_should_be_dropped_in_favor_of(
         &mut self,
         victim: &EvaluatedCandidate<'tcx>,
@@ -1806,13 +1807,6 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
         // This is a fix for #53123 and prevents winnowing from accidentally extending the
         // lifetime of a variable.
         match (&other.candidate, &victim.candidate) {
-            (_, AutoImplCandidate) | (AutoImplCandidate, _) => {
-                bug!(
-                    "default implementations shouldn't be recorded \
-                    when there are other valid candidates"
-                );
-            }
-
             // FIXME(@jswrenn): this should probably be more sophisticated
             (TransmutabilityCandidate, _) | (_, TransmutabilityCandidate) => DropVictim::No,
 
@@ -1854,6 +1848,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
             (
                 ParamCandidate(ref other_cand),
                 ImplCandidate(..)
+                | AutoImplCandidate
                 | ClosureCandidate { .. }
                 | GeneratorCandidate
                 | FutureCandidate
@@ -1881,6 +1876,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
             }
             (
                 ImplCandidate(_)
+                | AutoImplCandidate
                 | ClosureCandidate { .. }
                 | GeneratorCandidate
                 | FutureCandidate
@@ -1914,6 +1910,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
             (
                 ObjectCandidate(_) | ProjectionCandidate(..),
                 ImplCandidate(..)
+                | AutoImplCandidate
                 | ClosureCandidate { .. }
                 | GeneratorCandidate
                 | FutureCandidate
@@ -1927,6 +1924,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
 
             (
                 ImplCandidate(..)
+                | AutoImplCandidate
                 | ClosureCandidate { .. }
                 | GeneratorCandidate
                 | FutureCandidate
@@ -2017,6 +2015,19 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
                 }
             }
 
+            (AutoImplCandidate, ImplCandidate(_)) | (ImplCandidate(_), AutoImplCandidate) => {
+                DropVictim::No
+            }
+
+            (AutoImplCandidate, _) | (_, AutoImplCandidate) => {
+                bug!(
+                    "default implementations shouldn't be recorded \
+                    when there are other global candidates: {:?} {:?}",
+                    other,
+                    victim
+                );
+            }
+
             // Everything else is ambiguous
             (
                 ImplCandidate(_)

tests/ui/auto-traits/issue-83857-ub.rs

@@ -0,0 +1,31 @@
+#![allow(suspicious_auto_trait_impls)]
+
+struct Always<T, U>(T, U);
+unsafe impl<T, U> Send for Always<T, U> {}
+struct Foo<T, U>(Always<T, U>);
+
+trait False {}
+unsafe impl<U: False> Send for Foo<u32, U> {}
+
+trait WithAssoc {
+    type Output;
+}
+impl<T: Send> WithAssoc for T {
+    type Output = Self;
+}
+impl WithAssoc for Foo<u32, ()> {
+    type Output = Box<i32>;
+}
+
+fn generic<T, U>(v: Foo<T, U>, f: fn(<Foo<T, U> as WithAssoc>::Output) -> i32) {
+    //~^ ERROR `Foo<T, U>` cannot be sent between threads safely
+    f(foo(v));
+}
+
+fn foo<T: Send>(x: T) -> <T as WithAssoc>::Output {
+    x
+}
+
+fn main() {
+    generic(Foo(Always(0, ())), |b| *b);
+}

tests/ui/auto-traits/issue-83857-ub.stderr

@@ -0,0 +1,22 @@
+error[E0277]: `Foo<T, U>` cannot be sent between threads safely
+  --> $DIR/issue-83857-ub.rs:20:38
+   |
+LL | fn generic<T, U>(v: Foo<T, U>, f: fn(<Foo<T, U> as WithAssoc>::Output) -> i32) {
+   |                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Foo<T, U>` cannot be sent between threads safely
+   |
+   = help: the trait `Send` is not implemented for `Foo<T, U>`
+note: required for `Foo<T, U>` to implement `WithAssoc`
+  --> $DIR/issue-83857-ub.rs:13:15
+   |
+LL | impl<T: Send> WithAssoc for T {
+   |         ----  ^^^^^^^^^     ^
+   |         |
+   |         unsatisfied trait bound introduced here
+help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement
+   |
+LL | fn generic<T, U>(v: Foo<T, U>, f: fn(<Foo<T, U> as WithAssoc>::Output) -> i32) where Foo<T, U>: Send {
+   |                                                                                +++++++++++++++++++++
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0277`.

tests/ui/generator/auto-trait-regions.rs

@@ -26,7 +26,7 @@ fn assert_foo<T: Foo>(f: T) {}
 fn main() {
     // Make sure 'static is erased for generator interiors so we can't match it in trait selection
     let x: &'static _ = &OnlyFooIfStaticRef(No);
-    let gen = || {
+    let gen = move || {
         let x = x;
         yield;
         assert_foo(x);
@@ -36,15 +36,15 @@ fn main() {
 
     // Allow impls which matches any lifetime
     let x = &OnlyFooIfRef(No);
-    let gen = || {
+    let gen = move || {
         let x = x;
         yield;
         assert_foo(x);
     };
     assert_foo(gen); // ok
 
     // Disallow impls which relates lifetimes in the generator interior
-    let gen = || {
+    let gen = move || {
         let a = A(&mut true, &mut true, No);
         //~^ temporary value dropped while borrowed
         //~| temporary value dropped while borrowed

tests/ui/impl-trait/recursive-auto-trait.rs

@@ -0,0 +1,10 @@
+// check-pass
+fn is_send<T: Send>(_: T) {}
+fn foo() -> impl Send {
+    if false {
+        is_send(foo());
+    }
+    ()
+}
+
+fn main() {}