Do proper signature equality checking for tail calls

[WaffleLapkin]

View all comments

For tail calls to be valid, we need some support from the ABI, specifically the caller of the function needs to be able to do all necessary cleanup after the tail called function returns. This can be achieved with a callee-cleanup ABI, but we want to permit tail calls for other ABIs as well (in particular, extern "rust").

In such cases the property that we are looking for is "the caller and callee require the same cleanup". We approximate this with "caller and callee have the same ABI" and we approximate that with "caller and callee have the same signatures (modulo subtyping)".

Previously the code would compare FnSigs with ==, which would not support subtyping and produced other weird results. This PR changes tail call checks to use type checking machinery and in particular eq.

It also fixes handling of opaque types, given the next trait solver is enabled.

r? types

[folkertdev]

cc #144953

What exactly makes RPIT not work for the old trait solver?

View changes since the review

[WaffleLapkin]

I'm not quite sure (lcnr can probably explain better), but something something opaque types are more annoying to handle in the old solver and in this case they are not being revealed as their hidden type.

[lcnr]

the main issue is that the old solver does not have a way to say "we're in the defining scope of the following opaque types and have already inferred their hidden types".

So while you can kind of hack around this by allowing them to be redefined and then checking that they match the actually inferred value, this is annoying and doesn't fully work. This is what check_opaque_type_well_formed (I think that's what the method is called) does

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[lcnr]

this ignores nested obligations of the equality, and region constraints

let ocx = ObligationCtxt::new(&infcx); ocx.sub;

and then handle pending obligations and region constraints

View changes since the review

[WaffleLapkin]

Region constraints are ignored on purpose:

// Checks that the signatures of the caller and callee match (as a proxy to check that they
// are ABI compatible and tail calls can always happen).
//
// This ignores lifetimes and safety, as those do not affect ABI.

---

Re: nested obligations, I can't come up with an example that requires them. I.e. I can change this to use ocx, but I can't seem to make a test that would fail with the current code...

[WaffleLapkin]

Looking at rustc_type_ir/src/relate/solver_relating.rs, the only code path that modifies SolverRelating::goals are relating of two inference variables... but there can be no inference variables at this point afaik -- this is called after type checking and all types should be known by now.

[lcnr]

we also emit nested goals when encountering non-rigid aliases 🤔

// This ignores lifetimes and safety, as those do not affect ABI.

you need to consider lifetimes to correctly handle higher-ranked regions as these can impact TypeId and layout

I think in practice you could limit yourself to only a leak check and ignore constraints between free regions, I don't fully trust that until Boxy's assumptions on binders work is done

[WaffleLapkin]

I added a commit to use ocx, but still have 0 clue how to construct any tests which fail without this change...