Auto merge of rust-lang#104098 - aliemjay:fn-wf, r=<try>

fix fn item implied bounds and wf check

These are two distinct changes:
1. Wf-check all fn item substs.
Fixes rust-lang#104005

2. Use implied bounds from impl header.
Fixes rust-lang#98852
Fixes rust-lang#102611

The first is a breaking change and will likely have big impact without the the second one. See the first commit for how it breaks libstd.

Landing the second one without the first will allow more incorrect code to pass. For example an exploit of rust-lang#104005 would be as simple as:
```rust
use core::fmt::Display;

trait ExtendLt<Witness> {
    fn extend(self) -> Box<dyn Display>;
}

impl<T: Display> ExtendLt<&'static T> for T {
    fn extend(self) -> Box<dyn Display> {
        Box::new(self)
    }
}

fn main() {
    let val = (&String::new()).extend();
    println!("{val}");
}
```

cc `@lcnr`
r? types

Author: bors

compiler/rustc_borrowck/src/type_check/free_region_relations.rs

@@ -1,5 +1,6 @@
 use rustc_data_structures::frozen::Frozen;
 use rustc_data_structures::transitive_relation::{TransitiveRelation, TransitiveRelationBuilder};
+use rustc_hir::def::DefKind;
 use rustc_infer::infer::canonical::QueryRegionConstraints;
 use rustc_infer::infer::outlives;
 use rustc_infer::infer::outlives::env::RegionBoundPairs;
@@ -195,7 +196,9 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> {
 
     #[instrument(level = "debug", skip(self))]
     pub(crate) fn create(mut self) -> CreateResult<'tcx> {
-        let span = self.infcx.tcx.def_span(self.universal_regions.defining_ty.def_id());
+        let tcx = self.infcx.tcx;
+        let defining_ty_def_id = self.universal_regions.defining_ty.def_id().expect_local();
+        let span = tcx.def_span(defining_ty_def_id);
 
         // Insert the facts we know from the predicates. Why? Why not.
         let param_env = self.param_env;
@@ -275,6 +278,26 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> {
             normalized_inputs_and_output.push(norm_ty);
         }
 
+        // Add implied bounds from impl header.
+        if matches!(tcx.def_kind(defining_ty_def_id), DefKind::AssocFn | DefKind::AssocConst) {
+            for &(ty, _) in tcx.assumed_wf_types(tcx.local_parent(defining_ty_def_id)) {
+                let Ok(TypeOpOutput { output: norm_ty, constraints: c, .. }) = self
+                    .param_env
+                    .and(type_op::normalize::Normalize::new(ty))
+                    .fully_perform(self.infcx, span)
+                else {
+                    tcx.sess.delay_span_bug(span, &format!("failed to normalize {ty:?}"));
+                    continue;
+                };
+                constraints.extend(c);
+
+                // We currently add implied bounds from the normalized ty only.
+                // This is more conservative and matches wfcheck behavior.
+                let c = self.add_implied_bounds(norm_ty);
+                constraints.extend(c);
+            }
+        }
+
         for c in constraints {
             self.push_region_constraints(c, span);
         }

compiler/rustc_borrowck/src/type_check/mod.rs

@@ -414,6 +414,16 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'tcx> {
                     instantiated_predicates,
                     locations,
                 );
+
+                assert!(!matches!(
+                    tcx.impl_of_method(def_id).map(|imp| tcx.def_kind(imp)),
+                    Some(DefKind::Impl { of_trait: true })
+                ));
+                self.cx.prove_predicates(
+                    args.types().map(|ty| ty::ClauseKind::WellFormed(ty.into())),
+                    locations,
+                    ConstraintCategory::Boring,
+                );
             }
         }
     }

compiler/rustc_trait_selection/src/traits/query/type_op/ascribe_user_type.rs

@@ -63,13 +63,16 @@ fn relate_mir_and_user_ty<'tcx>(
     user_ty: Ty<'tcx>,
 ) -> Result<(), NoSolution> {
     let cause = ObligationCause::dummy_with_span(span);
+    ocx.register_obligation(Obligation::new(
+        ocx.infcx.tcx,
+        cause.clone(),
+        param_env,
+        ty::ClauseKind::WellFormed(user_ty.into()),
+    ));
+
     let user_ty = ocx.normalize(&cause, param_env, user_ty);
     ocx.eq(&cause, param_env, mir_ty, user_ty)?;
 
-    // FIXME(#104764): We should check well-formedness before normalization.
-    let predicate =
-        ty::Binder::dummy(ty::PredicateKind::Clause(ty::ClauseKind::WellFormed(user_ty.into())));
-    ocx.register_obligation(Obligation::new(ocx.infcx.tcx, cause, param_env, predicate));
     Ok(())
 }
 
@@ -113,31 +116,38 @@ fn relate_mir_and_user_args<'tcx>(
         ocx.register_obligation(Obligation::new(tcx, cause, param_env, instantiated_predicate));
     }
 
+    // Now prove the well-formedness of `def_id` with `substs`.
+    // Note for some items, proving the WF of `ty` is not sufficient because the
+    // well-formedness of an item may depend on the WF of gneneric args not present in the
+    // item's type. Currently this is true for associated consts, e.g.:
+    // ```rust
+    // impl<T> MyTy<T> {
+    //     const CONST: () = { /* arbitrary code that depends on T being WF */ };
+    // }
+    // ```
+    for arg in args {
+        ocx.register_obligation(Obligation::new(
+            tcx,
+            cause.clone(),
+            param_env,
+            ty::ClauseKind::WellFormed(arg),
+        ));
+    }
+
     if let Some(UserSelfTy { impl_def_id, self_ty }) = user_self_ty {
+        ocx.register_obligation(Obligation::new(
+            tcx,
+            cause.clone(),
+            param_env,
+            ty::ClauseKind::WellFormed(self_ty.into()),
+        ));
+
         let self_ty = ocx.normalize(&cause, param_env, self_ty);
         let impl_self_ty = tcx.type_of(impl_def_id).instantiate(tcx, args);
         let impl_self_ty = ocx.normalize(&cause, param_env, impl_self_ty);
 
         ocx.eq(&cause, param_env, self_ty, impl_self_ty)?;
-        let predicate = ty::Binder::dummy(ty::PredicateKind::Clause(ty::ClauseKind::WellFormed(
-            impl_self_ty.into(),
-        )));
-        ocx.register_obligation(Obligation::new(tcx, cause.clone(), param_env, predicate));
     }
 
-    // In addition to proving the predicates, we have to
-    // prove that `ty` is well-formed -- this is because
-    // the WF of `ty` is predicated on the args being
-    // well-formed, and we haven't proven *that*. We don't
-    // want to prove the WF of types from  `args` directly because they
-    // haven't been normalized.
-    //
-    // FIXME(nmatsakis): Well, perhaps we should normalize
-    // them?  This would only be relevant if some input
-    // type were ill-formed but did not appear in `ty`,
-    // which...could happen with normalization...
-    let predicate =
-        ty::Binder::dummy(ty::PredicateKind::Clause(ty::ClauseKind::WellFormed(ty.into())));
-    ocx.register_obligation(Obligation::new(tcx, cause, param_env, predicate));
     Ok(())
 }

tests/ui/borrowck/fn-item-check-trait-ref.rs

@@ -0,0 +1,15 @@
+// The method `assert_static` should be callable only for static values,
+// because the impl has an implied bound `where T: 'static`.
+
+// check-fail
+
+trait AnyStatic<Witness>: Sized {
+    fn assert_static(self) {}
+}
+
+impl<T> AnyStatic<&'static T> for T {}
+
+fn main() {
+    (&String::new()).assert_static();
+    //~^ ERROR temporary value dropped while borrowed
+}

tests/ui/borrowck/fn-item-check-trait-ref.stderr

@@ -0,0 +1,12 @@
+error[E0716]: temporary value dropped while borrowed
+  --> $DIR/fn-item-check-trait-ref.rs:13:7
+   |
+LL |     (&String::new()).assert_static();
+   |     --^^^^^^^^^^^^^------------------ 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 previous error
+
+For more information about this error, try `rustc --explain E0716`.

tests/ui/borrowck/fn-item-check-type-params.rs

@@ -0,0 +1,57 @@
+// Regression test for #104005.
+//
+// Previously, different borrowck implementations used to disagree here.
+// The status of each is documented on `fn test_*`.
+
+// check-fail
+
+use std::fmt::Display;
+
+trait Displayable {
+    fn display(self) -> Box<dyn Display>;
+}
+
+impl<T: Display> Displayable for (T, Option<&'static T>) {
+    fn display(self) -> Box<dyn Display> {
+        Box::new(self.0)
+    }
+}
+
+fn extend_lt<T, U>(val: T) -> Box<dyn Display>
+where
+    (T, Option<U>): Displayable,
+{
+    Displayable::display((val, None))
+}
+
+// AST: fail
+// HIR: pass
+// MIR: pass
+pub fn test_call<'a>(val: &'a str) {
+    extend_lt(val);
+    //~^ ERROR borrowed data escapes outside of function
+}
+
+// AST: fail
+// HIR: fail
+// MIR: pass
+pub fn test_coercion<'a>() {
+    let _: fn(&'a str) -> _ = extend_lt;
+    //~^ ERROR lifetime may not live long enough
+}
+
+// AST: fail
+// HIR: fail
+// MIR: fail
+pub fn test_arg() {
+    fn want<I, O>(_: I, _: impl Fn(I) -> O) {}
+    want(&String::new(), extend_lt);
+    //~^ ERROR temporary value dropped while borrowed
+}
+
+// An exploit of the unsoundness.
+fn main() {
+    let val = extend_lt(&String::from("blah blah blah"));
+    //~^ ERROR temporary value dropped while borrowed
+    println!("{}", val);
+}

tests/ui/borrowck/fn-item-check-type-params.stderr

@@ -0,0 +1,43 @@
+error[E0521]: borrowed data escapes outside of function
+  --> $DIR/fn-item-check-type-params.rs:31:5
+   |
+LL | pub fn test_call<'a>(val: &'a str) {
+   |                  --  --- `val` is a reference that is only valid in the function body
+   |                  |
+   |                  lifetime `'a` defined here
+LL |     extend_lt(val);
+   |     ^^^^^^^^^^^^^^
+   |     |
+   |     `val` escapes the function body here
+   |     argument requires that `'a` must outlive `'static`
+
+error: lifetime may not live long enough
+  --> $DIR/fn-item-check-type-params.rs:39:12
+   |
+LL | pub fn test_coercion<'a>() {
+   |                      -- lifetime `'a` defined here
+LL |     let _: fn(&'a str) -> _ = extend_lt;
+   |            ^^^^^^^^^^^^^^^^ type annotation requires that `'a` must outlive `'static`
+
+error[E0716]: temporary value dropped while borrowed
+  --> $DIR/fn-item-check-type-params.rs:48:11
+   |
+LL |     want(&String::new(), extend_lt);
+   |     ------^^^^^^^^^^^^^------------- 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[E0716]: temporary value dropped while borrowed
+  --> $DIR/fn-item-check-type-params.rs:54:26
+   |
+LL |     let val = extend_lt(&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 4 previous errors
+
+Some errors have detailed explanations: E0521, E0716.
+For more information about an error, try `rustc --explain E0521`.

tests/ui/higher-ranked/trait-bounds/issue-59311.rs

@@ -17,6 +17,7 @@ where
     v.t(|| {});
     //~^ ERROR: higher-ranked lifetime error
     //~| ERROR: higher-ranked lifetime error
+    //~| ERROR: higher-ranked lifetime error
 }
 
 fn main() {}

tests/ui/higher-ranked/trait-bounds/issue-59311.stderr

@@ -6,6 +6,15 @@ LL |     v.t(|| {});
    |
    = note: could not prove `{closure@$DIR/issue-59311.rs:17:9: 17:11} well-formed`
 
+error: higher-ranked lifetime error
+  --> $DIR/issue-59311.rs:17:5
+   |
+LL |     v.t(|| {});
+   |     ^^^^^^^^^^
+   |
+   = note: could not prove `{closure@$DIR/issue-59311.rs:17:9: 17:11} well-formed`
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
 error: higher-ranked lifetime error
   --> $DIR/issue-59311.rs:17:9
    |
@@ -14,5 +23,5 @@ LL |     v.t(|| {});
    |
    = note: could not prove `for<'a> &'a V: 'static`
 
-error: aborting due to 2 previous errors
+error: aborting due to 3 previous errors
 

tests/ui/implied-bounds/implied-bounds-on-trait-hierarchy.rs

@@ -1,9 +1,7 @@
-// check-pass
-// known-bug: #84591
+// issue: #84591
 
-// Should fail. Subtrait can incorrectly extend supertrait lifetimes even when
-// supertrait has weaker implied bounds than subtrait. Strongly related to
-// issue #25860.
+// Subtrait was able to incorrectly extend supertrait lifetimes even when
+// supertrait had weaker implied bounds than subtrait.
 
 trait Subtrait<T>: Supertrait {}
 trait Supertrait {
@@ -34,6 +32,7 @@ fn main() {
     {
         let x = "Hello World".to_string();
         subs_to_soup((x.as_str(), &mut d));
+        //~^ does not live long enough
     }
     println!("{}", d);
 }

tests/ui/lifetimes/lifetime-errors/issue_74400.rs

@@ -11,6 +11,8 @@ fn f<T, S>(data: &[T], key: impl Fn(&T) -> S) {
 fn g<T>(data: &[T]) {
     f(data, identity)
     //~^ ERROR the parameter type
+    //~| ERROR the parameter type
+    //~| ERROR the parameter type
     //~| ERROR mismatched types
     //~| ERROR implementation of `FnOnce` is not general
 }

tests/ui/lifetimes/lifetime-errors/issue_74400.stderr

@@ -12,6 +12,36 @@ help: consider adding an explicit lifetime bound
 LL | fn g<T: 'static>(data: &[T]) {
    |       +++++++++
 
+error[E0310]: the parameter type `T` may not live long enough
+  --> $DIR/issue_74400.rs:12:5
+   |
+LL |     f(data, identity)
+   |     ^^^^^^^^^^^^^^^^^
+   |     |
+   |     the parameter type `T` must be valid for the static lifetime...
+   |     ...so that the type `T` will meet its required lifetime bounds
+   |
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+help: consider adding an explicit lifetime bound
+   |
+LL | fn g<T: 'static>(data: &[T]) {
+   |       +++++++++
+
+error[E0310]: the parameter type `T` may not live long enough
+  --> $DIR/issue_74400.rs:12:5
+   |
+LL |     f(data, identity)
+   |     ^^^^^^^^^^^^^^^^^
+   |     |
+   |     the parameter type `T` must be valid for the static lifetime...
+   |     ...so that the type `T` will meet its required lifetime bounds
+   |
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+help: consider adding an explicit lifetime bound
+   |
+LL | fn g<T: 'static>(data: &[T]) {
+   |       +++++++++
+
 error[E0308]: mismatched types
   --> $DIR/issue_74400.rs:12:5
    |
@@ -35,7 +65,7 @@ LL |     f(data, identity)
    = note: `fn(&'2 T) -> &'2 T {identity::<&'2 T>}` must implement `FnOnce<(&'1 T,)>`, for any lifetime `'1`...
    = note: ...but it actually implements `FnOnce<(&'2 T,)>`, for some specific lifetime `'2`
 
-error: aborting due to 3 previous errors
+error: aborting due to 5 previous errors
 
 Some errors have detailed explanations: E0308, E0310.
 For more information about an error, try `rustc --explain E0308`.

tests/ui/type-alias-impl-trait/wf-nested.fail.stderr

@@ -1,5 +1,5 @@
 error[E0310]: the parameter type `T` may not live long enough
-  --> $DIR/wf-nested.rs:55:27
+  --> $DIR/wf-nested.rs:57:27
    |
 LL |     type InnerOpaque<T> = impl Sized;
    |                           ^^^^^^^^^^