regression: X does not live long enough #100544

Activity

1. Mark-Simulacrum

   added

   T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.

   regression-from-stable-to-betaPerformance or correctness regression from stable to beta.Performance or correctness regression from stable to beta.

   E-needs-bisectionCall for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustcCall for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc

   E-needs-mcveCall for participation: This issue has a repro, but needs a Minimal Complete and Verifiable ExampleCall for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example

   on Aug 14, 2022
2. Mark-Simulacrum

   added this to the 1.64.0 milestone on Aug 14, 2022
3. rustbot

   added

   I-prioritizeIssue needs a team member to assess the impact. Will be replaced by P-{low,medium,high,critical}Issue needs a team member to assess the impact. Will be replaced by P-{low,medium,high,critical}

   on Aug 14, 2022
4. ehuss commented on Aug 14, 2022

   

   ehuss

   on Aug 14, 2022

   Contributor

   More actions

   I think these are two separate (unrelated) issues.

   #98835 had a crater run where these regressions were identified: #98835 (comment)

   #99413 had a crater run where there were quite a lot of regressions, but nobody has reviewed it: #99413 (comment)

   There are quite a few regressions in #99413, but I suspect that will need to be accepted since it is a soundness fix.
5. estebank commented on Aug 15, 2022

   

   estebank

   on Aug 15, 2022

   Contributor

   More actions

   From #98835 (comment):

   The migration is trivial in most cases: removing lifetime annotations is sufficient, however innernet requires replacing Self constructor with the struct name.

   I wish this had been flagged as a potential diagnostic issue back when this was identified. Filed #100572.
6. jackh726 commented on Aug 15, 2022

   

   jackh726

   on Aug 15, 2022

   Member

   More actions

   In theory, we could back out of #98835, maybe with a future compat warning.
7. steffahn commented on Aug 16, 2022

   

   steffahn

   on Aug 16, 2022

   Last edited by steffahn

   Member

   More actions

   I bisected charcoal and dkr, both pointing to caee496 which does contain #98835.
   And I bisected operator, pointing to af7ab34 which contains #99413.
8. steffahn commented on Aug 17, 2022

   

   steffahn

   on Aug 17, 2022

   Last edited by steffahn

   Member

   More actions

   json2rss is also #99413
   (by the way, I have opened PRs to multiple affected crates, including json2rss and operator, those two are already merged ^^)

   FGS regresses at the same nightly as #98835, as does wonnx

   ft-cli is most definitely #99413 (the handlebars method “render” was involved in a lot of other #99413-regressions, too). I didn’t bisect or send a patch, since the repo is archived and has lots of dependencies.

   @rustbot label -E-needs-bisection

   @aliemjay provided a minimal example of the #98835-regression in their PRs:

   async fn test<'a>() { // `'a`, being a lifetime parameter, must be longer than the entire function body
       let f = |_: &'a str| {}; // the argument is required to outlive `'a`
       f(&String::new()); // an argument of shorter lifetime is passed!
   }

   For the #99413-regressions, we don’t need a mcve, since breakage was expected from the soundness fix.

   @rustbot label -E-needs-mcve
9. rustbot

   removed

   E-needs-bisectionCall for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustcCall for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc

   E-needs-mcveCall for participation: This issue has a repro, but needs a Minimal Complete and Verifiable ExampleCall for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example

   on Aug 17, 2022
10. seanpianka commented on Oct 3, 2022

    

    seanpianka

    on Oct 3, 2022

    Last edited by seanpianka

    More actions

    Is this actually a regression? I saw it mentioned that this is a correctness fixed introduced as a part of #98835.

    I think this issue is probably safe to close.
11. apiraino commented on Oct 5, 2022

    

    apiraino

    on Oct 5, 2022

    Contributor

    More actions

    @seanpianka I think at this point I agree with your opinion (also by looking at this comment).

    @jackh726 do you agree this can be closed without further action? is it a case of intended behaviour but perhaps poorly communicated to users? other interesting points to note here?

    thanks!

    👍React with 👍1Reacted by Sean Pianka
12. apiraino commented on Oct 26, 2022

    

    apiraino

    on Oct 26, 2022

    Last edited by apiraino

    Contributor

    More actions

    I'll tentatively close this issue as it this seems now it's an accepted breakage that will need crates to update their code (example, the above mentioned #102937)

    @rustbot label -I-prioritize
13. apiraino

    closed this as completedon Oct 26, 2022
14. rustbot

    removed

    I-prioritizeIssue needs a team member to assess the impact. Will be replaced by P-{low,medium,high,critical}Issue needs a team member to assess the impact. Will be replaced by P-{low,medium,high,critical}

    on Oct 26, 2022