Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 0 additions & 38 deletions compiler/rustc_borrowck/src/type_check/canonical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,42 +323,4 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
.and(type_op::ascribe_user_type::AscribeUserType { mir_ty, user_ty }),
);
}

/// *Incorrectly* skips the WF checks we normally do in `ascribe_user_type`.
///
/// FIXME(#104478, #104477): This is a hack for backward-compatibility.
#[instrument(skip(self), level = "debug")]
pub(super) fn ascribe_user_type_skip_wf(
&mut self,
mir_ty: Ty<'tcx>,
user_ty: ty::UserType<'tcx>,
span: Span,
) {
let ty::UserTypeKind::Ty(user_ty) = user_ty.kind else { bug!() };

// A fast path for a common case with closure input/output types.
if let ty::Infer(_) = user_ty.kind() {
self.eq_types(user_ty, mir_ty, Locations::All(span), ConstraintCategory::Boring)
.unwrap();
return;
}

// FIXME: Ideally MIR types are normalized, but this is not always true.
let mir_ty = self.normalize(mir_ty, Locations::All(span));

let cause = ObligationCause::dummy_with_span(span);
let param_env = self.infcx.param_env;
let _: Result<_, ErrorGuaranteed> = self.fully_perform_op(
Locations::All(span),
ConstraintCategory::Boring,
type_op::custom::CustomTypeOp::new(
|ocx| {
let user_ty = ocx.normalize(&cause, param_env, user_ty);
ocx.eq(&cause, param_env, user_ty, mir_ty)?;
Ok(())
},
"ascribe_user_type_skip_wf",
),
);
}
}
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/type_check/input_output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
.skip(1 + if is_coroutine_with_implicit_resume_ty { 1 } else { 0 })
.map(|local| &self.body.local_decls[local]),
) {
self.ascribe_user_type_skip_wf(
self.ascribe_user_type(
arg_decl.ty,
ty::UserType::new(ty::UserTypeKind::Ty(user_ty)),
arg_decl.source_info.span,
Expand All @@ -119,7 +119,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {

// If the user explicitly annotated the output type, enforce it.
let output_decl = &self.body.local_decls[RETURN_PLACE];
self.ascribe_user_type_skip_wf(
self.ascribe_user_type(
output_decl.ty,
ty::UserType::new(ty::UserTypeKind::Ty(user_provided_sig.output())),
output_decl.source_info.span,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
//@ check-pass
struct Foo<'a>(&'a ())
where
(): Trait<'a>;
Expand All @@ -21,7 +20,7 @@ where
}

fn main() {
let bar: for<'a, 'b> fn(Foo<'a>, &'b ()) = |_, _| {};
let bar: for<'a, 'b> fn(Foo<'a>, &'b ()) = |_, _| {}; //~ ERROR: lifetime may not live long enough

// If `could_use_implied_bounds` were to use implied bounds,
// keeping 'a late-bound, then we could assign that function
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/implied-bounds/hrlt-implied-trait-bounds-roundtrip.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error: lifetime may not live long enough
--> $DIR/hrlt-implied-trait-bounds-roundtrip.rs:23:49
|
LL | let bar: for<'a, 'b> fn(Foo<'a>, &'b ()) = |_, _| {};
| ^
| |
| has type `Foo<'1>`
| requires that `'1` must outlive `'static`

error: aborting due to 1 previous error

Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ LL | let mut closure = expect_sig(|p, y| *p = y);
| - - ^^^^^^ assignment requires that `'1` must outlive `'2`
| | |
| | has type `&'1 i32`
| has type `&'?1 mut &'2 i32`
| has type `&'2 mut &'?2 i32`

note: no external requirements
--> $DIR/escape-argument-callee.rs:20:1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ LL | establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y
= note: late-bound region is '?4
= note: number of external vids: 5
= note: where '?1: '?2
= note: where '?1: '?2
= note: where '?1: '?2

note: no external requirements
--> $DIR/propagate-approximated-ref.rs:42:1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ LL | establish_relationships(&cell_a, &cell_b, |_outlives, x, y| {
= note: late-bound region is '?3
= note: number of external vids: 4
= note: where '?1: '?0
= note: where '?1: '?0

note: no external requirements
--> $DIR/propagate-approximated-shorter-to-static-no-bound.rs:31:1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ LL | establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y
= note: late-bound region is '?3
= note: late-bound region is '?4
= note: number of external vids: 5
= note: where '?1: '?2
= note: where '?1: '?2
= note: where '?1: '?0
= note: where '?1: '?0

note: no external requirements
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ error: lifetime may not live long enough
LL | establish_relationships(&cell_a, &cell_b, |_outlives, x, y| {
| --------- - has type `&'?6 Cell<&'1 u32>`
| |
| has type `&'?4 Cell<&'2 &'?1 u32>`
| has type `&'2 Cell<&'?5 &'?1 u32>`
LL | // Only works if 'x: 'y:
LL | demand_y(x, y, x.get())
| ^^^^^^^^^^^^^^^^^^^^^^^ argument requires that `'1` must outlive `'2`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ fn demand_y<'x, 'y>(_cell_x: &Cell<&'x u32>, _cell_y: &Cell<&'y u32>, _y: &'y u3

#[rustc_regions]
fn supply<'a, 'b>(cell_a: Cell<&'a u32>, cell_b: Cell<&'b u32>) {
establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| {
establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| { //~ ERROR: lifetime may not live long enough
// Only works if 'x: 'y:
demand_y(x, y, x.get())
//~^ ERROR
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
note: no external requirements
note: external requirements
--> $DIR/propagate-fail-to-approximate-longer-wrong-bounds.rs:39:47
|
LL | establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| {
Expand All @@ -17,12 +17,16 @@ LL | establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y
= note: late-bound region is '?10
= note: late-bound region is '?3
= note: late-bound region is '?4
= note: number of external vids: 5
= note: where '?1: '?2
= note: where '?1: '?2
= note: where '?1: '?2

error: lifetime may not live long enough
--> $DIR/propagate-fail-to-approximate-longer-wrong-bounds.rs:41:9
|
LL | establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| {
| ---------- ---------- has type `&'?7 Cell<&'2 &'?2 u32>`
| ---------- ---------- has type `&'2 Cell<&'?8 &'?2 u32>`
| |
| has type `&'?5 Cell<&'1 &'?1 u32>`
LL | // Only works if 'x: 'y:
Expand All @@ -41,5 +45,24 @@ LL | fn supply<'a, 'b>(cell_a: Cell<&'a u32>, cell_b: Cell<&'b u32>) {
|
= note: defining type: supply

error: aborting due to 1 previous error
error: lifetime may not live long enough
--> $DIR/propagate-fail-to-approximate-longer-wrong-bounds.rs:39:5
|
LL | fn supply<'a, 'b>(cell_a: Cell<&'a u32>, cell_b: Cell<&'b u32>) {
| -- -- lifetime `'b` defined here
| |
| lifetime `'a` defined here
LL | / establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| {
LL | | // Only works if 'x: 'y:
LL | | demand_y(x, y, x.get())
LL | |
LL | | });
| |______^ argument requires that `'a` must outlive `'b`
|
= help: consider adding the following bound: `'a: 'b`
= note: requirement occurs because of the type `Cell<&'?11 u32>`, which makes the generic argument `&'?11 u32` invariant
= note: the struct `Cell<T>` is invariant over the parameter `T`
= help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance

error: aborting due to 2 previous errors

Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error: lifetime may not live long enough
LL | doit(0, &|x, y| {
| - - has type `&'1 i32`
| |
| has type `&Cell<&'2 i32>`
| has type `&'2 Cell<&i32>`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also looks wrong, is it due to the removed normalization?

LL | x.set(y);
| ^^^^^^^^ argument requires that `'1` must outlive `'2`
|
Expand Down
48 changes: 48 additions & 0 deletions tests/ui/wf/check-wf-of-closure-args-complex.rs
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the name right now, so TODO.

Also, this test, when run with current nightly, indeeds performs a use-after-free. But now we error on it, it can thus be removed to this:

use std::sync::OnceLock;

type Payload = Box<i32>;

static STORAGE: OnceLock<& ()> = OnceLock::new();

trait Store {}
impl Store for &'static Payload {}

struct MyTy<T: Store>(T);

trait IsFn {}
impl IsFn for for<'x> fn(&'x Payload) -> MyTy<&'x Payload> {}

fn bar<F: IsFn>(_: F) {}

fn main() {
    bar::<for<'a> fn(&'a Payload) -> MyTy<&'a Payload>>(|x| unsafe {
        std::mem::transmute::<&Payload, MyTy<&Payload>>(x)
    });
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// The same as check-wf-of-closure-args-complex, but this example causes
// a use-after-free if we don't perform the wf check.
//
#![feature(unboxed_closures)]

use std::sync::OnceLock;

type Payload = Box<i32>;

static STORAGE: OnceLock<&'static Payload> = OnceLock::new();

trait Store {
fn store(&self);
}
impl Store for &'static Payload {
fn store(&self) {
STORAGE.set(*self).unwrap();
}
}

#[repr(transparent)]
struct MyTy<T: Store>(T);
impl<T: Store> Drop for MyTy<T> {
fn drop(&mut self) {
self.0.store();
}
}

trait IsFn: for<'x> Fn<(&'x Payload,)> {}

impl IsFn for for<'x> fn(&'x Payload) -> MyTy<&'x Payload> {}

fn foo(f: impl for<'x> Fn(&'x Payload)) {
let a = Box::new(1);
f(&a);
}
fn bar<F: IsFn>(f: F) {
foo(|x| { f(x); });
}

fn main() {
// If no wf-check is done on this closure given to `bar`, this compiles fine and
// a use-after-free will occur.
bar::<for<'a> fn(&'a Payload) -> MyTy<&'a Payload>>(|x| unsafe { //~ ERROR: lifetime may not live long enough
std::mem::transmute::<&Payload, MyTy<&Payload>>(x)
});
println!("{}", STORAGE.get().unwrap());
}
10 changes: 10 additions & 0 deletions tests/ui/wf/check-wf-of-closure-args-complex.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: lifetime may not live long enough
--> $DIR/check-wf-of-closure-args-complex.rs:44:60
|
LL | bar::<for<'a> fn(&'a Payload) -> MyTy<&'a Payload>>(|x| unsafe {
| - ^ requires that `'1` must outlive `'static`
| |
| has type `&'1 Box<i32>`

error: aborting due to 1 previous error

14 changes: 14 additions & 0 deletions tests/ui/wf/check-wf-of-closure-args.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Checks that we perform WF checks on closure args, regardless of wether they
// are used in the closure it self.
// related to issue #104478

struct MyTy<T: Trait>(T);
trait Trait {}
impl Trait for &'static str {}
fn wf<T>(_: T) {}

fn main() {
let _: for<'x> fn(MyTy<&'x str>) = |_| {}; //~ ERROR: lifetime may not live long enough

let _: for<'x> fn(MyTy<&'x str>) = |x| wf(x); //~ ERROR: lifetime may not live long enough
}
20 changes: 20 additions & 0 deletions tests/ui/wf/check-wf-of-closure-args.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error: lifetime may not live long enough
--> $DIR/check-wf-of-closure-args.rs:11:41
|
LL | let _: for<'x> fn(MyTy<&'x str>) = |_| {};
| ^
| |
| has type `MyTy<&'1 str>`
| requires that `'1` must outlive `'static`

error: lifetime may not live long enough
--> $DIR/check-wf-of-closure-args.rs:13:41
|
LL | let _: for<'x> fn(MyTy<&'x str>) = |x| wf(x);
| ^
| |
| has type `MyTy<&'1 str>`
| requires that `'1` must outlive `'static`

error: aborting due to 2 previous errors
Comment on lines +10 to +19
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find that the error message is a bit worse now, compared to the old:

error[E0521]: borrowed data escapes outside of closure
  --> src/lib.rs:10:44
   |
10 |     let _: for<'x> fn(MyTy<&'x str>) = |x| wf(x); // FAIL
   |                                         -  ^^^^^
   |                                         |  |
   |                                         |  `x` escapes the closure body here
   |                                         |  argument requires that `'1` must outlive `'static`
   |                                         `x` is a reference that is only valid in the closure body
   |                                         has type `MyTy<&'1 str>`


Loading