Do not substitute nonexistent lifetime in RPIT in recursive function

[zirconium-n]

type f<'a>::_Return<'_a> might not contains a suitable lifetime to substitute, so we just skip the substitution when this happens. This turns previous compile error before #94081 (and ICE after #94081) into compile success.

This is my first PR so I'm not sure if I got this right. But it passes all tests, so 🤷 .

r? @oli-obk

fixes #110726
fixes #110623
fixes #109059
fixes #101852

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @oli-obk (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[oli-obk]

I'll need to check out the PR and run some tests with some logs. It does not seem clear to me that this is correct, but all of that code here is very haphazard. I'll keep this PR on the top of my queue for when I have a few hours to work on it.

[zirconium-n]

I'll need to check out the PR and run some tests with some logs. It does not seem clear to me that this is correct, but all of that code here is very haphazard. I'll keep this PR on the top of my queue for when I have a few hours to work on it.

Do you have some test cases in mind? I can do some more checks if you don't have time to check yourself.

Following code is still compile error (not ICE). But I think this is out of scope for this PR.

fn test<'a>() -> impl Sized + 'a {
    let r: &'static i32 = test();
    r
}

[oli-obk]

I don't unfortunately. I just fear this change will allow some unsound code to compile somewhere, and it will be in really weird edge cases that are super hard to create. Basically I want to approach this from the other side, which is analyze the code until I fully grok it again and then see what to do about it. It does effectively mean it would be me doing the work of this PR again from scratch, so a few hours may be an optimistic estimate.

I wanted to refactor and clean up this code anyway 😆

I wish I had something to tell you that you could do, but this is just me mistrusting any changes to this piece of code, nothing about this PR in particular.

[BoxyUwU]

Can you add all the tests from #111906 (and if not fixed comment there saying this doesnt fix it)

[zirconium-n]

Tests added and passed. Seems there's some conflict though

[pnkfelix]

I have nominated this for T-types to look at, because I am not confident that the high-level logic of "we don't always have a lifetime to substitute, so just skip when that occurs" is sound.

(I can envison models where its perfectly sound, and others where it is not at all sound. I don't know which category this falls into.)

[bors]

☔ The latest upstream changes (presumably #108860) made this pull request unmergeable. Please resolve the merge conflicts.

[lcnr]

talked about this in the t-types meeting today https://rust-lang.zulipchat.com/#narrow/stream/326132-t-types.2Fmeetings/topic/2023-07-03.20planning.20meeting/near/372020374

to summarize, it feels dangerous to merge this PR. This whole area is currently a bit of a mess and we would like a more comprehensive cleanup and/or writeup of the current design¹ and its shortcomings. Getting something wrong here is fairly likely to lead to unsoundness where we incorrectly handle regions. We therefore prefer the current conservative approach of failing in these examples instead.

I still appreciate your effort spent here but will close this with the above rationale.

Footnotes

1. which probably requires someone who is already at least somewhat familiar with the surrounding code and a fairly large time investment ↩