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

regression: parameter type may not live long enough #97607

Closed
Mark-Simulacrum opened this issue Jun 1, 2022 · 5 comments · Fixed by #103341
Closed

regression: parameter type may not live long enough #97607

Mark-Simulacrum opened this issue Jun 1, 2022 · 5 comments · Fixed by #103341
Assignees
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented Jun 1, 2022

See https://crater-reports.s3.amazonaws.com/beta-1.62-1/beta-2022-05-20/gh/FSMaxB.rust-genealogy/log.txt:

[INFO] [stdout] error[E0311]: the parameter type `Extracted` may not live long enough
[INFO] [stdout]   --> genealogy-java-apis/src/comparator.rs:24:13
[INFO] [stdout]    |
[INFO] [stdout] 24 |             compare: Box::new(move |left, right| match (self.compare)(left, right) {
[INFO] [stdout]    |                      ^^^^^^^^ ...so that the type `Extracted` will meet its required lifetime bounds...
[INFO] [stdout]    |
[INFO] [stdout] note: ...that is required by this bound
[INFO] [stdout]   --> genealogy-java-apis/src/comparator.rs:21:28
[INFO] [stdout]    |
[INFO] [stdout] 21 |         for<'a> Extracted: Ord + 'a,
[INFO] [stdout]    |                                  ^^
[INFO] [stdout] help: consider adding an explicit lifetime bound...
[INFO] [stdout]    |
[INFO] [stdout] 21 |         for<'a> Extracted: Ord + 'a + 'a,
[INFO] [stdout]    |                                     ++++
[INFO] [stdout] 
[INFO] [stdout] 
[INFO] [stdout] error: aborting due to previous error

This is a 1.62 regression, it seems, based on Crater. At the very least, it's a diagnostics issue that we're suggesting adding an already existing lifetime bound.

@Mark-Simulacrum Mark-Simulacrum added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Jun 1, 2022
@Mark-Simulacrum Mark-Simulacrum added this to the 1.62.0 milestone Jun 1, 2022
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 1, 2022
@compiler-errors
Copy link
Member

Bisected

searched nightlies: from nightly-2022-04-01 to nightly-2022-05-31
regressed nightly: nightly-2022-05-14
searched commit range: a5ad0d2...f1f721e
regressed commit: a7d6408

bisected with cargo-bisect-rustc v0.6.3

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start=2022-04-01 --end=2022-05-31 

#96899, cc @oli-obk

Minimized

https://play.rust-lang.org/?version=beta&mode=debug&edition=2021&gist=8bcb86adc0a98d57aecbe3e2b80229c0

fn test<T, F, U>(f: F) -> Box<dyn Fn(T) -> U + 'static>
where
    F: 'static + Fn(T) -> U,
    for<'a> U: 'a, // < This is the problematic line -- remove it, and it passes.
{
    Box::new(move |t| f(t))
}

@compiler-errors
Copy link
Member

The issue is that for<'a> U: 'a bound is causing the new wf check introduced in #96899 to fail.

Regarding the diagnostics being weird, the issue is probably because we don't handle the case where a higher-ranked lifetime doesn't outlive itself. We just see a borrowck error, see that it's due to the binder's 'a lifetime, and suggest adding it. Not sure if that needs fixed, personally think it's not worth it.

@apiraino
Copy link
Contributor

apiraino commented Jun 8, 2022

Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.

@rustbot label -I-prioritize +P-medium

@rustbot rustbot added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jun 8, 2022
@compiler-errors compiler-errors self-assigned this Jun 12, 2022
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 13, 2022
…k, r=compiler-errors

Revert "Check that closures satisfy their where bounds"

This reverts commit 253408b from rust-lang#96899

This is only performed on beta to give us another few weeks to fix rust-lang#97607 on nightly. The planned fix is likely way too large to backport anyway.

r? `@compiler-errors`
@oli-obk
Copy link
Contributor

oli-obk commented Jun 14, 2022

This got fixed on nightly by enabling nll by default

@oli-obk oli-obk added E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. and removed P-medium Medium priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Jun 14, 2022
@compiler-errors compiler-errors removed their assignment Oct 20, 2022
@Rageking8
Copy link
Contributor

@rustbot claim
For the test

notriddle added a commit to notriddle/rust that referenced this issue Oct 21, 2022
…, r=compiler-errors

Add test for issue 97607

Fixes rust-lang#97607

r? `@compiler-errors`

Not sure which UI test dir to put this under, kindly let me know of a better dir if necessary and I will change it. Thanks.
@bors bors closed this as completed in db4696a Oct 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
6 participants