Auto merge of #124336 - compiler-errors:super-outlives, r=lcnr

Enforce supertrait outlives obligations hold when confirming impl

**TL;DR:** We elaborate super-predicates and apply any outlives obligations when proving an impl holds to fix a mismatch between implied bounds.

Bugs in implied bounds (and implied well-formedness) occur whenever there is a mismatch between the assumptions that some code can assume to hold, and the obligations that a caller/user of that code must prove. If the former is stronger than the latter, then unsoundness occurs.

Take a look at the example unsoundness:

```rust
use std::fmt::Display;

trait Static: 'static {}
impl<T> Static for &'static T {}
fn foo<S: Display>(x: S) -> Box<dyn Display>
where
    &'static S: Static,
{
    Box::new(x)
}

fn main() {
    let s = foo(&String::from("blah blah blah"));
    println!("{}", s);
}
```

This specific example occurs because we elaborate obligations in `fn foo`:

* `&'static S: Static`
    * `&'static S: 'static` <- super predicate
        * `S: 'static` <- elaborating outlives bounds

However, when calling `foo`, we only need to prove the direct set of where clauses. So at the call site for some substitution `S = &'not_static str`, that means only proving `&'static &'not_static str: Static`. To prove this, we apply the impl, which itself holds trivially since it has no where clauses.

This is the mismatch -- `foo` is allowed to assume that `S: 'static` via elaborating supertraits, but callers of `foo` never need to prove that `S: 'static`.

There are several approaches to fixing this, all of which have problems due to current limitations in our type system:
1. proving the elaborated set of predicates always - This leads to issues since we don't have coinductive trait semantics, so we easily hit new cycles.
    * This would fix our issue, since callers of `foo` would have to both prove `&'static &'not_static str: Static` and its elaborated bounds, which would surface the problematic `'not_static: 'static` outlives obligation.
    * However, proving supertraits when proving impls leads to inductive cycles which can't be fixed until we get coinductive trait semantics.
2. Proving that an impl header is WF when applying that impl:
    * This would fix our issue, since when we try to prove `&'static &'not_static str: Static`, we'd need to prove `WF(&'static &'not_static str)`, which would surface the problematic `'not_static: 'static` outlives obligation.
    * However, this leads to issues since we don't have higher-ranked implied bounds. This breaks things when trying to apply impls to higher-ranked trait goals.

To get around these limitations, we apply a subset of (1.), which is to elaborate the supertrait obligations of the impl but filter only the (region/type) outlives out of that set, since those can never participate in an inductive cycle. This is likely not sufficient to fix a pathological example of this issue, but it does clearly fill in a major gap that we're currently overlooking.

This can also result in 'unintended' errors due to missing implied-bounds on binders. We did not encounter this in the crater run and don't expect people to rely on this code in practice:
```rust
trait Outlives<'b>: 'b {}
impl<'b, T> Outlives<'b> for &'b T {}
fn foo<'b>()
where
    // This bound will break due to this PR as we end up proving
    // `&'b &'!a (): 'b` without the implied `'!a: 'b`
    // bound.
    for<'a> &'b &'a (): Outlives<'b>,
{}
```

Fixes #98117

---

Crater: #124336 (comment)
Triaged: #124336 (comment)

All of the fallout is due to generic const exprs, and can be ignored.

Author: bors

Diff for: compiler/rustc_next_trait_solver/src/solve/assembly/structural_traits.rs

@@ -7,7 +7,7 @@ use rustc_type_ir::data_structures::HashMap;
 use rustc_type_ir::fold::{TypeFoldable, TypeFolder, TypeSuperFoldable};
 use rustc_type_ir::inherent::*;
 use rustc_type_ir::lang_items::TraitSolverLangItem;
-use rustc_type_ir::{self as ty, Interner, Upcast as _};
+use rustc_type_ir::{self as ty, elaborate, Interner, Upcast as _};
 use rustc_type_ir_macros::{TypeFoldable_Generic, TypeVisitable_Generic};
 use tracing::instrument;
 
@@ -671,11 +671,19 @@ where
 {
     let cx = ecx.cx();
     let mut requirements = vec![];
-    requirements.extend(
+    // Elaborating all supertrait outlives obligations here is not soundness critical,
+    // since if we just used the unelaborated set, then the transitive supertraits would
+    // be reachable when proving the former. However, since we elaborate all supertrait
+    // outlives obligations when confirming impls, we would end up with a different set
+    // of outlives obligations here if we didn't do the same, leading to ambiguity.
+    // FIXME(-Znext-solver=coinductive): Adding supertraits here can be removed once we
+    // make impls coinductive always, since they'll always need to prove their supertraits.
+    requirements.extend(elaborate::elaborate(
+        cx,
         cx.explicit_super_predicates_of(trait_ref.def_id)
             .iter_instantiated(cx, trait_ref.args)
             .map(|(pred, _)| pred),
-    );
+    ));
 
     // FIXME(associated_const_equality): Also add associated consts to
     // the requirements here.

Diff for: compiler/rustc_next_trait_solver/src/solve/trait_goals.rs

@@ -87,6 +87,19 @@ where
                 .map(|pred| goal.with(cx, pred));
             ecx.add_goals(GoalSource::ImplWhereBound, where_clause_bounds);
 
+            // We currently elaborate all supertrait outlives obligations from impls.
+            // This can be removed when we actually do coinduction correctly, and prove
+            // all supertrait obligations unconditionally.
+            let goal_clause: I::Clause = goal.predicate.upcast(cx);
+            for clause in elaborate::elaborate(cx, [goal_clause]) {
+                if matches!(
+                    clause.kind().skip_binder(),
+                    ty::ClauseKind::TypeOutlives(..) | ty::ClauseKind::RegionOutlives(..)
+                ) {
+                    ecx.add_goal(GoalSource::Misc, goal.with(cx, clause));
+                }
+            }
+
             ecx.evaluate_added_goals_and_make_canonical_response(maximal_certainty)
         })
     }

Diff for: compiler/rustc_trait_selection/src/traits/select/mod.rs

@@ -7,15 +7,17 @@ use std::fmt::{self, Display};
 use std::ops::ControlFlow;
 use std::{cmp, iter};
 
+use hir::def::DefKind;
 use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_errors::{Diag, EmissionGuarantee};
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
 use rustc_hir::LangItem;
 use rustc_infer::infer::relate::TypeRelation;
-use rustc_infer::infer::BoundRegionConversionTime::HigherRankedType;
-use rustc_infer::infer::{BoundRegionConversionTime, DefineOpaqueTypes};
+use rustc_infer::infer::BoundRegionConversionTime::{self, HigherRankedType};
+use rustc_infer::infer::DefineOpaqueTypes;
+use rustc_infer::traits::util::elaborate;
 use rustc_infer::traits::TraitObligation;
 use rustc_middle::bug;
 use rustc_middle::dep_graph::{dep_kinds, DepNodeIndex};
@@ -2798,6 +2800,35 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
             });
         }
 
+        // Register any outlives obligations from the trait here, cc #124336.
+        if matches!(self.tcx().def_kind(def_id), DefKind::Impl { of_trait: true })
+            && let Some(header) = self.tcx().impl_trait_header(def_id)
+        {
+            let trait_clause: ty::Clause<'tcx> =
+                header.trait_ref.instantiate(self.tcx(), args).upcast(self.tcx());
+            for clause in elaborate(self.tcx(), [trait_clause]) {
+                if matches!(
+                    clause.kind().skip_binder(),
+                    ty::ClauseKind::TypeOutlives(..) | ty::ClauseKind::RegionOutlives(..)
+                ) {
+                    let clause = normalize_with_depth_to(
+                        self,
+                        param_env,
+                        cause.clone(),
+                        recursion_depth,
+                        clause,
+                        &mut obligations,
+                    );
+                    obligations.push(Obligation {
+                        cause: cause.clone(),
+                        recursion_depth,
+                        param_env,
+                        predicate: clause.as_predicate(),
+                    });
+                }
+            }
+        }
+
         obligations
     }
 }

Diff for: compiler/rustc_type_ir/src/inherent.rs

@@ -451,6 +451,8 @@ pub trait Clause<I: Interner<Clause = Self>>:
     + UpcastFrom<I, ty::Binder<I, ty::ClauseKind<I>>>
     + UpcastFrom<I, ty::TraitRef<I>>
     + UpcastFrom<I, ty::Binder<I, ty::TraitRef<I>>>
+    + UpcastFrom<I, ty::TraitPredicate<I>>
+    + UpcastFrom<I, ty::Binder<I, ty::TraitPredicate<I>>>
     + UpcastFrom<I, ty::ProjectionPredicate<I>>
     + UpcastFrom<I, ty::Binder<I, ty::ProjectionPredicate<I>>>
     + IntoKind<Kind = ty::Binder<I, ty::ClauseKind<I>>>

Diff for: tests/ui/fn/implied-bounds-unnorm-associated-type-5.rs

@@ -5,6 +5,7 @@ trait Trait<'a>: 'a {
 // if the `T: 'a` bound gets implied we would probably get ub here again
 impl<'a, T> Trait<'a> for T {
     //~^ ERROR the parameter type `T` may not live long enough
+    //~| ERROR the parameter type `T` may not live long enough
     type Type = ();
 }
 

Diff for: tests/ui/fn/implied-bounds-unnorm-associated-type-5.stderr

@@ -16,8 +16,21 @@ help: consider adding an explicit lifetime bound
 LL | impl<'a, T: 'a> Trait<'a> for T {
    |           ++++
 
+error[E0309]: the parameter type `T` may not live long enough
+  --> $DIR/implied-bounds-unnorm-associated-type-5.rs:6:27
+   |
+LL | impl<'a, T> Trait<'a> for T {
+   |      --                   ^ ...so that the type `T` will meet its required lifetime bounds
+   |      |
+   |      the parameter type `T` must be valid for the lifetime `'a` as defined here...
+   |
+help: consider adding an explicit lifetime bound
+   |
+LL | impl<'a, T: 'a> Trait<'a> for T {
+   |           ++++
+
 error[E0505]: cannot move out of `x` because it is borrowed
-  --> $DIR/implied-bounds-unnorm-associated-type-5.rs:21:10
+  --> $DIR/implied-bounds-unnorm-associated-type-5.rs:22:10
    |
 LL |     let x = String::from("Hello World!");
    |         - binding `x` declared here
@@ -33,7 +46,7 @@ help: consider cloning the value if the performance cost is acceptable
 LL |     let y = f(&x.clone(), ());
    |                 ++++++++
 
-error: aborting due to 2 previous errors
+error: aborting due to 3 previous errors
 
 Some errors have detailed explanations: E0309, E0505.
 For more information about an error, try `rustc --explain E0309`.

Diff for: tests/ui/static/static-lifetime.rs

@@ -1,6 +1,7 @@
 pub trait Arbitrary: Sized + 'static {}
 
 impl<'a, A: Clone> Arbitrary for ::std::borrow::Cow<'a, A> {} //~ ERROR lifetime bound
+//~^ ERROR cannot infer an appropriate lifetime for lifetime parameter `'a`
 
 fn main() {
 }

Diff for: tests/ui/static/static-lifetime.stderr

@@ -11,6 +11,32 @@ LL | impl<'a, A: Clone> Arbitrary for ::std::borrow::Cow<'a, A> {}
    |      ^^
    = note: but lifetime parameter must outlive the static lifetime
 
-error: aborting due to 1 previous error
+error[E0495]: cannot infer an appropriate lifetime for lifetime parameter `'a` due to conflicting requirements
+  --> $DIR/static-lifetime.rs:3:34
+   |
+LL | impl<'a, A: Clone> Arbitrary for ::std::borrow::Cow<'a, A> {}
+   |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: first, the lifetime cannot outlive the lifetime `'a` as defined here...
+  --> $DIR/static-lifetime.rs:3:6
+   |
+LL | impl<'a, A: Clone> Arbitrary for ::std::borrow::Cow<'a, A> {}
+   |      ^^
+note: ...so that the types are compatible
+  --> $DIR/static-lifetime.rs:3:34
+   |
+LL | impl<'a, A: Clone> Arbitrary for ::std::borrow::Cow<'a, A> {}
+   |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^
+   = note: expected `<Cow<'a, A> as Arbitrary>`
+              found `<Cow<'_, A> as Arbitrary>`
+   = note: but, the lifetime must be valid for the static lifetime...
+note: ...so that the declared lifetime parameter bounds are satisfied
+  --> $DIR/static-lifetime.rs:3:34
+   |
+LL | impl<'a, A: Clone> Arbitrary for ::std::borrow::Cow<'a, A> {}
+   |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
 
-For more information about this error, try `rustc --explain E0478`.
+Some errors have detailed explanations: E0478, E0495.
+For more information about an error, try `rustc --explain E0478`.

Diff for: tests/ui/wf/wf-in-where-clause-static.current.stderr

@@ -0,0 +1,12 @@
+error[E0716]: temporary value dropped while borrowed
+  --> $DIR/wf-in-where-clause-static.rs:18:18
+   |
+LL |     let s = foo(&String::from("blah blah blah"));
+   |             -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-- temporary value is freed at the end of this statement
+   |             |    |
+   |             |    creates a temporary value which is freed while still in use
+   |             argument requires that borrow lasts for `'static`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0716`.

Diff for: tests/ui/wf/wf-in-where-clause-static.next.stderr

@@ -0,0 +1,12 @@
+error[E0716]: temporary value dropped while borrowed
+  --> $DIR/wf-in-where-clause-static.rs:18:18
+   |
+LL |     let s = foo(&String::from("blah blah blah"));
+   |             -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-- temporary value is freed at the end of this statement
+   |             |    |
+   |             |    creates a temporary value which is freed while still in use
+   |             argument requires that borrow lasts for `'static`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0716`.

Diff for: tests/ui/wf/wf-in-where-clause-static.rs

@@ -1,9 +1,6 @@
-//@ check-pass
-//@ known-bug: #98117
-
-// Should fail. Functions are responsible for checking the well-formedness of
-// their own where clauses, so this should fail and require an explicit bound
-// `T: 'static`.
+//@ revisions: current next
+//@ ignore-compare-mode-next-solver (explicit revisions)
+//@[next] compile-flags: -Znext-solver
 
 use std::fmt::Display;
 
@@ -19,5 +16,6 @@ where
 
 fn main() {
     let s = foo(&String::from("blah blah blah"));
+    //~^ ERROR temporary value dropped while borrowed
     println!("{}", s);
 }