Check that RPITs constrained by a recursive call in a closure are compatible

[compiler-errors]

Fixes #99073

Adapts a similar visitor pattern to find_opaque_ty_constraints (that we use to check TAITs), but with some changes:
0. Only walk the "OnlyBody" children, instead of all items in the RPIT's defining scope

1. Only walk through the body's children if we found a constraining usage
2. Don't actually do any inference, just do a comparison and error if they're mismatched

---

r? @oli-obk -- you know all this impl-trait stuff best... is this the right approach? I can explain the underlying issue better if you'd like, in case that might reveal a better solution. Not sure if it's possible to gather up the closure's defining usages of the RPIT while borrowck'ing the outer function, that might be a better place to put this check...

[oli-obk]

Ugh... so we end up with an RPIT's hidden type constrained within a closure and I didn't add appropriate checks against that. I used to have some logic to bubble all such hidden types out into the main function to solve this.

Ideally we should probably unify RPIT and TAIT logic in type_of to allow closures and nested functions inside a function to register hidden types for all RPIT of that function.

That's more of a lang change tho. We should outright forbid closures to bind the hidden types of RPIT imo. And then once that is landed we can ask the lang team about unifying the TAIT and RPIT defining scope logic

[compiler-errors]

We should outright forbid closures to bind the hidden types of RPIT imo. And then once that is landed we can ask the lang team about unifying the TAIT and RPIT defining scope logic

@oli-obk: not sure how to do that... My understanding is that we would need to adjust the defining_use_anchor when borrowck'ing closures so that we don't think any RPIT constraint from within the closure is "defining". But simply adjusting the defining_use_anchor to point to the closure when borrowck'ing a closure doesn't seem to do the trick. Where exactly should I be putting this logic to deny RPIT constraints in closures?

[oli-obk]

doesn't do the trick

As in, no change in behaviour? I can imagine that, as we go back to the main function afterwards, and then suddenly it sees the old anchor again.

I think we'll need to double check that an opaque type was in its defining scope at the end, when we collect it in borrowck.

[compiler-errors]

@rustbot author

[compiler-errors]

Superseded by #99345, though I would like to move towards something like this eventually if possible. But I understand that this is a lang decision to change behavior.

@rustbot blocked

[bors]

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

[oli-obk]

Yes, this is the right step towards unifying RPIT and TAIT defining scopes.

After this PR is merged, we should check if we can remove the call to replace_opaque_types_with_inference_vars in closure.rs. I assume "no", but we can do it in a way that allows more things to compile (closures, and potentially all nested items, becoming part of the defining scope of an RPIT) and ask T-lang for what they think about it.

Not sure if it's possible to gather up the closure's defining usages of the RPIT while borrowck'ing the outer function

it is, I had this in an earlier version of the lazy TAIT PR. I removed it in favour of the current scheme, since the current scheme can be extended towards allowing nested things to be part of the defining scope.

[oli-obk]

r=me after a rebase

[compiler-errors]

I reverted #99345 as well, since this PR allows strictly more programs to compile and without reverting that one I don't think we'd see any changes.

[compiler-errors]

Just double checking if that's ok ^ @oli-obk

[oli-obk]

To be clear: this PR enables no new stable code to compile, right?

[compiler-errors]

@oli-obk, actually this does allow "new" stable code to compile. Well, not really new, but code that used to compile before #99345.

For example:

use std::fmt::Display;

fn foo<T: Display + Copy>(t: T, recurse: bool) -> impl Display {
    if recurse {
        (|| {
            let t: T = foo(t, false);
            println!("{t}");
        })();
    }
    t
}

fn main() {
    foo(1i32, true);
}

I don't think there's any value in landing this PR without reverting #99345, since we'll error out during borrowck when a closure has a defining usage of an RPIT due to that PR, so this new code path won't ever get touched.

[oli-obk]

@bors r+

[bors]

📌 Commit 5e8f1e8 has been approved by oli-obk

It is now in the queue for this repository.