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
18 changes: 18 additions & 0 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2500,6 +2500,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
DefKind::InlineConst => args.as_inline_const().parent_args(),
other => bug!("unexpected item {:?}", other),
};

let parent_args = tcx.mk_args(parent_args);

assert_eq!(typeck_root_args.len(), parent_args.len());
Expand All @@ -2518,6 +2519,23 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
);
}

// Closures assume that their argument types are well-formed. However, this
// is not guaranteed: a closure can be defined with non–well-formed arguments.
// In most cases this is harmless, as callers checks the well-formedness when
// calling the closure. However, if the closure is never called within the
// body that defines it, these checks may be bypassed, potentially leading to
// unsoundness. To avoid this, we explicitly check the well-formedness of the
// closure's arguments here.
// See #153027.
self.prove_predicates(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

needs comment

also can you write a short FCP proposal here? The change does feel simple enough that I don't think we need to explain too much here just

  • closures assume that their arguments are well-formed
  • this is currently not checked if the closure is not called
  • results in an unsound builtin Fn-trait impl: example
  • some quick section exploring alternatives and why this we ended up with this one

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Okay, I'll write one

args.iter().skip(parent_args.len()).filter_map(|arg| {
let term = arg.as_term()?;
Some(ty::Binder::dummy(ty::PredicateKind::Clause(ty::ClauseKind::WellFormed(term))))
}),
location.to_locations(),
ConstraintCategory::Boring,
);

tcx.predicates_of(def_id).instantiate(tcx, args)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ impl Foo {
//~| ERROR the parameter type `impl for<'a> Fn(&'a usize) -> Box<I>` may not live long enough
//~| ERROR the parameter type `impl for<'a> Fn(&'a usize) -> Box<I>` may not live long enough
//~| ERROR the parameter type `impl for<'a> Fn(&'a usize) -> Box<I>` may not live long enough
//~| ERROR the parameter type `impl for<'a> Fn(&'a usize) -> Box<I>` may not live long enough
//~| ERROR the parameter type `I` may not live long enough
//~| ERROR the parameter type `I` may not live long enough
//~| ERROR the parameter type `I` may not live long enough
Expand Down
16 changes: 15 additions & 1 deletion tests/ui/borrowck/unconstrained-closure-lifetime-generic.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,20 @@ help: consider adding an explicit lifetime bound
LL | pub fn ack<I>(&mut self, f: impl for<'a> Fn(&'a usize) -> Box<I> + 'static) {
| +++++++++

error[E0310]: the parameter type `impl for<'a> Fn(&'a usize) -> Box<I>` may not live long enough
--> $DIR/unconstrained-closure-lifetime-generic.rs:10:29
|
LL | self.bar = Box::new(|baz| Box::new(f(baz)));
| ^^^^^^^^^^^^^^^^^^^^^^
| |
| the parameter type `impl for<'a> Fn(&'a usize) -> Box<I>` must be valid for the static lifetime...
| ...so that the type `impl for<'a> Fn(&'a usize) -> Box<I>` will meet its required lifetime bounds
|
help: consider adding an explicit lifetime bound
|
LL | pub fn ack<I>(&mut self, f: impl for<'a> Fn(&'a usize) -> Box<I> + 'static) {
| +++++++++

error[E0310]: the parameter type `I` may not live long enough
--> $DIR/unconstrained-closure-lifetime-generic.rs:10:35
|
Expand Down Expand Up @@ -113,7 +127,7 @@ LL | }
|
= note: due to object lifetime defaults, `Box<dyn for<'a> Fn(&'a usize) -> Box<(dyn Any + 'a)>>` actually means `Box<(dyn for<'a> Fn(&'a usize) -> Box<(dyn Any + 'a)> + 'static)>`

error: aborting due to 8 previous errors
error: aborting due to 9 previous errors

Some errors have detailed explanations: E0310, E0311, E0597.
For more information about an error, try `rustc --explain E0310`.
29 changes: 29 additions & 0 deletions tests/ui/implied-bounds/implied-bounds-on-external-params-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// A regression test for https://github.com/rust-lang/rust/issues/151637.

struct Wrap<T: 'static>(T);

fn error1<T>(x: T) {
Wrap(x);
//~^ ERROR: the parameter type `T` may not live long enough
}

// We used to add implied bound `T: 'static` from the closure's return type
// when borrow checking the closure, which resulted in allowing non-wf return
// type. Implied bounds which contains only the external regions or type params
// from the parents should not be implied. That would make those bounds in needs
// propagated as closure requirements and either proved or make borrowck error
// from the parent's body.
fn error2<T>(x: T) {
|| Wrap(x);
//~^ ERROR: the parameter type `T` may not live long enough
}

fn no_error1<T: 'static>(x: T) {
|| Wrap(x);
}

fn no_error2<T>(x: T, _: &'static T) {
|| Wrap(x);
}

fn main() {}
31 changes: 31 additions & 0 deletions tests/ui/implied-bounds/implied-bounds-on-external-params-1.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
error[E0310]: the parameter type `T` may not live long enough
--> $DIR/implied-bounds-on-external-params-1.rs:6:5
|
LL | Wrap(x);
| ^^^^^^^
| |
| the parameter type `T` must be valid for the static lifetime...
| ...so that the type `T` will meet its required lifetime bounds
|
help: consider adding an explicit lifetime bound
|
LL | fn error1<T: 'static>(x: T) {
| +++++++++

error[E0310]: the parameter type `T` may not live long enough
--> $DIR/implied-bounds-on-external-params-1.rs:17:5
|
LL | || Wrap(x);
| ^^^^^^^^^^
| |
| the parameter type `T` must be valid for the static lifetime...
| ...so that the type `T` will meet its required lifetime bounds
|
help: consider adding an explicit lifetime bound
|
LL | fn error2<T: 'static>(x: T) {
| +++++++++

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0310`.
60 changes: 60 additions & 0 deletions tests/ui/implied-bounds/implied-bounds-on-external-params-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// A regression test for https://github.com/rust-lang/rust/issues/151637.

type Payload = Box<i32>;

trait StaticToStatic {
fn static_to_static(self) -> &'static Payload
where
Self: 'static;
}
impl<'a> StaticToStatic for &'a Payload {
fn static_to_static(self) -> &'static Payload
where
Self: 'static,
{
self // Legal. 'a must be 'static due to Self: 'static
}
}

struct Wrap<T: StaticToStatic + 'static>(T);

trait ToStatic {
fn to_static(self) -> &'static Payload;
}
impl<T: StaticToStatic> ToStatic for Wrap<T> {
fn to_static(self) -> &'static Payload {
self.0.static_to_static() // Legal. T: 'static is implied by Wrap<T>
}
}

// Trait to allow mentioning FnOnce without mentioning the return type directly
trait MyFnOnce {
type MyOutput;
fn my_call_once(self) -> Self::MyOutput;
}
impl<F: FnOnce() -> T, T> MyFnOnce for F {
type MyOutput = T;
fn my_call_once(self) -> T {
self()
}
}

fn call<F: MyFnOnce<MyOutput: ToStatic>>(f: F) -> &'static Payload {
f.my_call_once().to_static()
}

fn extend<T: StaticToStatic>(x: T) -> &'static Payload {
let c = move || {
//~^ ERROR: the parameter type `T` may not live long enough
// Probably should be illegal, since Wrap requires T: 'static
Wrap(x)
};
call(c)
}

fn main() {
let x = Box::new(Box::new(1));
let y = extend(&*x);
drop(x);
println!("{y}");
}
22 changes: 22 additions & 0 deletions tests/ui/implied-bounds/implied-bounds-on-external-params-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error[E0310]: the parameter type `T` may not live long enough
--> $DIR/implied-bounds-on-external-params-2.rs:47:13
|
LL | let c = move || {
| _____________^
LL | |
LL | | // Probably should be illegal, since Wrap requires T: 'static
LL | | Wrap(x)
LL | | };
| | ^
| | |
| |_____the parameter type `T` must be valid for the static lifetime...
| ...so that the type `T` will meet its required lifetime bounds
|
help: consider adding an explicit lifetime bound
|
LL | fn extend<T: StaticToStatic + 'static>(x: T) -> &'static Payload {
| +++++++++

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0310`.
1 change: 1 addition & 0 deletions tests/ui/nll/ice-106874.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub fn func<V, F: Fn(&mut V)>(f: F) -> A<impl X> {
//~| ERROR implementation of `FnOnce` is not general enough
//~| ERROR higher-ranked subtype error
//~| ERROR higher-ranked subtype error
//~| ERROR the parameter type `V` may not live long enough
}

trait X {}
Expand Down
17 changes: 16 additions & 1 deletion tests/ui/nll/ice-106874.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -89,5 +89,20 @@ LL | A(B(C::new(D::new(move |st| f(st)))))
= note: ...but it actually implements `Fn<(&'2 mut V,)>`, for some specific lifetime `'2`
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error: aborting due to 10 previous errors
error[E0310]: the parameter type `V` may not live long enough
--> $DIR/ice-106874.rs:8:23
|
LL | A(B(C::new(D::new(move |st| f(st)))))
| ^^^^^^^^^^^^^^^
| |
| the parameter type `V` must be valid for the static lifetime...
| ...so that the type `V` will meet its required lifetime bounds
|
help: consider adding an explicit lifetime bound
|
LL | pub fn func<V: 'static, F: Fn(&mut V)>(f: F) -> A<impl X> {
| +++++++++

error: aborting due to 11 previous errors

For more information about this error, try `rustc --explain E0310`.
Loading