Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce that closure: 'a requires that closure_ret_ty: 'a holds #84385

Closed
wants to merge 3 commits into from
Closed
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
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/intrinsicck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ pub struct InlineAsmCtxt<'a, 'tcx> {
get_operand_ty: Box<dyn Fn(&'tcx hir::Expr<'tcx>) -> Ty<'tcx> + 'a>,
}

impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> {
impl<'a, 'tcx: 'a> InlineAsmCtxt<'a, 'tcx> {
pub fn new_global_asm(tcx: TyCtxt<'tcx>) -> Self {
InlineAsmCtxt {
tcx,
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ pub struct InferCtxt<'tcx> {
universe: Cell<ty::UniverseIndex>,

normalize_fn_sig_for_diagnostic:
Option<Lrc<dyn Fn(&InferCtxt<'tcx>, ty::PolyFnSig<'tcx>) -> ty::PolyFnSig<'tcx>>>,
Option<Lrc<dyn Fn(&InferCtxt<'tcx>, ty::PolyFnSig<'tcx>) -> ty::PolyFnSig<'tcx> + 'tcx>>,
}

/// See the `error_reporting` module for more details.
Expand Down Expand Up @@ -552,7 +552,7 @@ pub struct InferCtxtBuilder<'tcx> {
defining_use_anchor: DefiningAnchor,
considering_regions: bool,
normalize_fn_sig_for_diagnostic:
Option<Lrc<dyn Fn(&InferCtxt<'tcx>, ty::PolyFnSig<'tcx>) -> ty::PolyFnSig<'tcx>>>,
Option<Lrc<dyn Fn(&InferCtxt<'tcx>, ty::PolyFnSig<'tcx>) -> ty::PolyFnSig<'tcx> + 'tcx>>,
}

pub trait TyCtxtInferExt<'tcx> {
Expand Down Expand Up @@ -589,7 +589,7 @@ impl<'tcx> InferCtxtBuilder<'tcx> {

pub fn with_normalize_fn_sig_for_diagnostic(
mut self,
fun: Lrc<dyn Fn(&InferCtxt<'tcx>, ty::PolyFnSig<'tcx>) -> ty::PolyFnSig<'tcx>>,
fun: Lrc<dyn Fn(&InferCtxt<'tcx>, ty::PolyFnSig<'tcx>) -> ty::PolyFnSig<'tcx> + 'tcx>,
) -> Self {
self.normalize_fn_sig_for_diagnostic = Some(fun);
self
Expand Down
36 changes: 32 additions & 4 deletions compiler/rustc_infer/src/infer/outlives/components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,42 @@ fn compute_components<'tcx>(
}

ty::Closure(_, ref substs) => {
let tupled_ty = substs.as_closure().tupled_upvars_ty();
compute_components(tcx, tupled_ty, out, visited);
let closure_substs = substs.as_closure();
compute_components(tcx, closure_substs.tupled_upvars_ty(), out, visited);
// Subtle: The return type of a closure can be used in a projection
// of the form `<closure as FnOnce>::Output`. Per the RFC 1214 rules
// for projections, we can use `closure: 'a` to conclude that
// `<closure as FnOnce>::Output: 'a` holds. In order for this to be sound,
// we must enforce that requiring `closure: 'a` also requires that.
// the return type of the closure outlive `a`.
// We do not require that any of the closure's *argument* types outlive 'a
// - this is sound, because there is no rule that would allow us to conclide
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// - this is sound, because there is no rule that would allow us to conclide
// - this is sound, because there is no rule that would allow us to conclude

// anything about the argument types from the fact that `closure: 'a` holds
// (the arguments of a closure do not appear in the output type of a trait impl
// for any trait implemented for a closure).
// This is inconsistent with function pointers, which require that all of their
// argument types (as well as the return type) outlive `'a` in order for
// `fn(A, B) -> R : ' a` to hold. It would be a breaking change to enforce this for
// closuers, and is not required for soundness.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// closuers, and is not required for soundness.
// closures, and is not required for soundness.

//
// Note: The 'skip_binder()' call here matches the way we handle fn substs
// via `walk_shallow`, which also skips binders.

// Hack - try to get a more accurate count on Crater by skipping this
// when checking dependencies.
if std::env::var("CARGO_PRIMARY_PACKAGE").is_ok() {
compute_components(tcx, closure_substs.sig().output().skip_binder(), out, visited);
}
}

ty::Generator(_, ref substs, _) => {
// Same as the closure case
let tupled_ty = substs.as_generator().tupled_upvars_ty();
compute_components(tcx, tupled_ty, out, visited);
let generator_substs = substs.as_generator();
compute_components(tcx, generator_substs.tupled_upvars_ty(), out, visited);
if std::env::var("CARGO_PKG_NAME").ok().as_deref() != Some("tokio-util") {
compute_components(tcx, generator_substs.yield_ty(), out, visited);
compute_components(tcx, generator_substs.return_ty(), out, visited);
}

// We ignore regions in the generator interior as we don't
// want these to affect region inference
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/iter/adapters/enumerate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ where
R: Try<Output = Acc>,
{
#[inline]
fn enumerate<'a, T, Acc, R>(
fn enumerate<'a, T, Acc, R: 'a>(
count: &'a mut usize,
mut fold: impl FnMut(Acc, (usize, T)) -> R + 'a,
) -> impl FnMut(Acc, T) -> R + 'a {
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/iter/adapters/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fn filter_fold<T, Acc>(
move |acc, item| if predicate(&item) { fold(acc, item) } else { acc }
}

fn filter_try_fold<'a, T, Acc, R: Try<Output = Acc>>(
fn filter_try_fold<'a, T, Acc, R: Try<Output = Acc> + 'a>(
predicate: &'a mut impl FnMut(&T) -> bool,
mut fold: impl FnMut(Acc, T) -> R + 'a,
) -> impl FnMut(Acc, T) -> R + 'a {
Expand Down
8 changes: 4 additions & 4 deletions library/core/src/iter/adapters/filter_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ fn filter_map_fold<T, B, Acc>(
}
}

fn filter_map_try_fold<'a, T, B, Acc, R: Try<Output = Acc>>(
fn filter_map_try_fold<'a, T, B, Acc, R: Try<Output = Acc> + 'a>(
f: &'a mut impl FnMut(T) -> Option<B>,
mut fold: impl FnMut(Acc, B) -> R + 'a,
) -> impl FnMut(Acc, T) -> R + 'a {
Expand Down Expand Up @@ -94,9 +94,9 @@ where
#[inline]
fn next_back(&mut self) -> Option<B> {
#[inline]
fn find<T, B>(
f: &mut impl FnMut(T) -> Option<B>,
) -> impl FnMut((), T) -> ControlFlow<B> + '_ {
fn find<'a, T, B: 'a>(
f: &'a mut impl FnMut(T) -> Option<B>,
) -> impl FnMut((), T) -> ControlFlow<B> + 'a {
move |(), x| match f(x) {
Some(x) => ControlFlow::Break(x),
None => ControlFlow::CONTINUE,
Expand Down
16 changes: 8 additions & 8 deletions library/core/src/iter/adapters/flatten.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,9 +334,9 @@ where
Fold: FnMut(Acc, U) -> Acc,
{
#[inline]
fn flatten<T: IntoIterator, Acc>(
fold: &mut impl FnMut(Acc, T::IntoIter) -> Acc,
) -> impl FnMut(Acc, T) -> Acc + '_ {
fn flatten<'a, T: IntoIterator, Acc: 'a>(
fold: &'a mut impl FnMut(Acc, T::IntoIter) -> Acc,
) -> impl FnMut(Acc, T) -> Acc + 'a {
move |acc, iter| fold(acc, iter.into_iter())
}

Expand Down Expand Up @@ -365,7 +365,7 @@ where
R: Try<Output = Acc>,
{
#[inline]
fn flatten<'a, T: IntoIterator, Acc, R: Try<Output = Acc>>(
fn flatten<'a, T: IntoIterator, Acc, R: Try<Output = Acc> + 'a>(
frontiter: &'a mut Option<T::IntoIter>,
fold: &'a mut impl FnMut(Acc, &mut T::IntoIter) -> R,
) -> impl FnMut(Acc, T) -> R + 'a {
Expand Down Expand Up @@ -403,9 +403,9 @@ where
Fold: FnMut(Acc, U) -> Acc,
{
#[inline]
fn flatten<T: IntoIterator, Acc>(
fold: &mut impl FnMut(Acc, T::IntoIter) -> Acc,
) -> impl FnMut(Acc, T) -> Acc + '_ {
fn flatten<'a, T: IntoIterator, Acc: 'a>(
fold: &'a mut impl FnMut(Acc, T::IntoIter) -> Acc,
) -> impl FnMut(Acc, T) -> Acc + 'a {
move |acc, iter| fold(acc, iter.into_iter())
}

Expand Down Expand Up @@ -434,7 +434,7 @@ where
R: Try<Output = Acc>,
{
#[inline]
fn flatten<'a, T: IntoIterator, Acc, R: Try>(
fn flatten<'a, T: IntoIterator, Acc, R: Try + 'a>(
backiter: &'a mut Option<T::IntoIter>,
fold: &'a mut impl FnMut(Acc, &mut T::IntoIter) -> R,
) -> impl FnMut(Acc, T) -> R + 'a {
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/iter/adapters/inspect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ fn inspect_fold<T, Acc>(
}
}

fn inspect_try_fold<'a, T, Acc, R>(
fn inspect_try_fold<'a, T, Acc, R: 'a>(
f: &'a mut impl FnMut(&T),
mut fold: impl FnMut(Acc, T) -> R + 'a,
) -> impl FnMut(Acc, T) -> R + 'a {
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/iter/adapters/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ fn map_fold<T, B, Acc>(
move |acc, elt| g(acc, f(elt))
}

fn map_try_fold<'a, T, B, Acc, R>(
fn map_try_fold<'a, T, B, Acc, R: 'a>(
f: &'a mut impl FnMut(T) -> B,
mut g: impl FnMut(Acc, B) -> R + 'a,
) -> impl FnMut(Acc, T) -> R + 'a {
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/iter/adapters/scan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ where
Fold: FnMut(Acc, Self::Item) -> R,
R: Try<Output = Acc>,
{
fn scan<'a, T, St, B, Acc, R: Try<Output = Acc>>(
fn scan<'a, T, St, B, Acc: 'a, R: Try<Output = Acc> + 'a>(
state: &'a mut St,
f: &'a mut impl FnMut(&mut St, T) -> Option<B>,
mut fold: impl FnMut(Acc, B) -> R + 'a,
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/iter/adapters/take.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ where
Fold: FnMut(Acc, Self::Item) -> R,
R: Try<Output = Acc>,
{
fn check<'a, T, Acc, R: Try<Output = Acc>>(
fn check<'a, T, Acc: 'a, R: Try<Output = Acc> + 'a>(
n: &'a mut usize,
mut fold: impl FnMut(Acc, T) -> R + 'a,
) -> impl FnMut(Acc, T) -> ControlFlow<R, Acc> + 'a {
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/iter/adapters/take_while.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ where
Fold: FnMut(Acc, Self::Item) -> R,
R: Try<Output = Acc>,
{
fn check<'a, T, Acc, R: Try<Output = Acc>>(
fn check<'a, T, Acc: 'a, R: Try<Output = Acc> + 'a>(
flag: &'a mut bool,
p: &'a mut impl FnMut(&T) -> bool,
mut fold: impl FnMut(Acc, T) -> R + 'a,
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/iter/traits/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3848,7 +3848,7 @@ where
F: FnMut(A::Item, B::Item) -> ControlFlow<T>,
{
#[inline]
fn compare<'a, B, X, T>(
fn compare<'a, B, X, T: 'a>(
b: &'a mut B,
mut f: impl FnMut(X, B::Item) -> ControlFlow<T> + 'a,
) -> impl FnMut(X) -> ControlFlow<ControlFlow<T, Ordering>> + 'a
Expand Down
37 changes: 37 additions & 0 deletions src/test/ui/closures/issue-84366-closure-outlives-ret.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Regression test for issue #84366
// Ensures that proving `closure: 'a` requires that
// we prove `closure_ret_type: 'a`

use std::fmt;
Aaron1011 marked this conversation as resolved.
Show resolved Hide resolved
trait Trait {
type Associated;
}

impl<R, F: Fn() -> R> Trait for F {
type Associated = R;
}

fn static_transfers_to_associated<T: Trait + 'static>(
_: &T,
x: T::Associated,
) -> Box<dyn fmt::Display /* + 'static */>
where
T::Associated: fmt::Display,
{
Box::new(x) // T::Associated: 'static follows from T: 'static
}

fn make_static_displayable<'a>(s: &'a str) -> Box<dyn fmt::Display> {
let f = || -> &'a str { "" };
// problem is: the closure type of `f` is 'static
static_transfers_to_associated(&f, s) //~ ERROR borrowed data escapes
}

fn main() {
let d;
{
let x = "Hello World".to_string();
d = make_static_displayable(&x);
}
println!("{}", d);
}
17 changes: 17 additions & 0 deletions src/test/ui/closures/issue-84366-closure-outlives-ret.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error[E0521]: borrowed data escapes outside of function
--> $DIR/issue-84366-closure-outlives-ret.rs:27:5
|
LL | fn make_static_displayable<'a>(s: &'a str) -> Box<dyn fmt::Display> {
| -- - `s` is a reference that is only valid in the function body
| |
| lifetime `'a` defined here
...
LL | static_transfers_to_associated(&f, s)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| `s` escapes the function body here
| argument requires that `'a` must outlive `'static`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0521`.
40 changes: 40 additions & 0 deletions src/test/ui/generator/issue-84366-generator-outlives-return.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// A version of issue #84366, but involving generator return types instead of closures

#![feature(generators)]
#![feature(generator_trait)]

use std::fmt;
use std::ops::Generator;

trait Trait {
type Associated;
}

impl<R, F: Generator<Return = R>> Trait for F {
type Associated = R;
}

fn static_transfers_to_associated<T: Trait + 'static>(
_: &T,
x: T::Associated,
) -> Box<dyn fmt::Display /* + 'static */>
where
T::Associated: fmt::Display,
{
Box::new(x) // T::Associated: 'static follows from T: 'static
}

fn make_static_displayable<'a>(s: &'a str) -> Box<dyn fmt::Display> {
let f = || { yield ""; "" };
// problem is: the closure type of `f` is 'static
static_transfers_to_associated(&f, s) //~ ERROR borrowed data escapes
}

fn main() {
let d;
{
let x = "Hello World".to_string();
d = make_static_displayable(&x);
}
println!("{}", d);
}
17 changes: 17 additions & 0 deletions src/test/ui/generator/issue-84366-generator-outlives-return.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error[E0521]: borrowed data escapes outside of function
--> $DIR/issue-84366-generator-outlives-return.rs:30:5
|
LL | fn make_static_displayable<'a>(s: &'a str) -> Box<dyn fmt::Display> {
| -- - `s` is a reference that is only valid in the function body
| |
| lifetime `'a` defined here
...
LL | static_transfers_to_associated(&f, s)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| `s` escapes the function body here
| argument requires that `'a` must outlive `'static`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0521`.
40 changes: 40 additions & 0 deletions src/test/ui/generator/issue-84366-generator-outlives-yield.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// A version of issue #84366, but involving generators instead of closures

#![feature(generators)]
#![feature(generator_trait)]

use std::fmt;
use std::ops::Generator;

trait Trait {
type Associated;
}

impl<R, F: Generator<Yield = R>> Trait for F {
type Associated = R;
}

fn static_transfers_to_associated<T: Trait + 'static>(
_: &T,
x: T::Associated,
) -> Box<dyn fmt::Display /* + 'static */>
where
T::Associated: fmt::Display,
{
Box::new(x) // T::Associated: 'static follows from T: 'static
}

fn make_static_displayable<'a>(s: &'a str) -> Box<dyn fmt::Display> {
let f = || { yield ""; "" };
// problem is: the closure type of `f` is 'static
static_transfers_to_associated(&f, s) //~ ERROR borrowed data escapes
}

fn main() {
let d;
{
let x = "Hello World".to_string();
d = make_static_displayable(&x);
}
println!("{}", d);
}
17 changes: 17 additions & 0 deletions src/test/ui/generator/issue-84366-generator-outlives-yield.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error[E0521]: borrowed data escapes outside of function
--> $DIR/issue-84366-generator-outlives-yield.rs:30:5
|
LL | fn make_static_displayable<'a>(s: &'a str) -> Box<dyn fmt::Display> {
| -- - `s` is a reference that is only valid in the function body
| |
| lifetime `'a` defined here
...
LL | static_transfers_to_associated(&f, s)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| `s` escapes the function body here
| argument requires that `'a` must outlive `'static`

error: aborting due to previous error

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