Remove region variable leaks from higher_ranked_sub().

[ebfull]

Fixes #28279.

Currently

common_supertype(*mut for<'a> Fn(&'a usize), *mut for<'a> Fn(&'a usize) + 'static)

equals *mut Fn(&usize) which seems to be caused by higher_ranked_sub() allowing region variables to escape the comparison. This prevents inference from working properly with stuff like Rc<Fn(&T)>.

r? @nikomatsakis

[ebfull]

This could be better with tests to make sure that the tainted region checks are sound, but I can't come up with any.

[nikomatsakis]

Actually, the return value from sub (and equate) ought to be ignored, as described...well, somewhere or other. I think the bug is in the relate_with_variance for lub etc. That said, it is probably wrong to return result here. It would be better to return a (which is also consistent with what the other sub cases do). That would probably fix lub, fwiw, but it still seems like it's wrong to return the result from equate. I guess it'd be better to say that sub always returns a subtype (i.e., a), if it does not yield an error. I don't think that introduces any particular difficulties to do. Similarly, equate can be defined to return a something that is equal.

[ebfull]

Nice, that makes much more sense. I've rebased with a much more conservative fix that covers most of the bases. If you think I should go through and return a from all of the pertinent routines (such as higher_ranked_sub etc.) I can do that too.

[nikomatsakis]

@ebfull it seems like the policy is a bit unclear; I think at least a comment on Sub and Equate indicating that they are supposed to:

• make a a subtype of (resp. equal to) b
• return a

would make the policy clear.

[brson]

I've started a crater run.

[nikomatsakis]

@ebfull ah, I see you pushed a commit, sorry, I didn't notice (GH doesn't notify me). This looks good to me. @brson, is there any particular reason you started a crater run for this branch? (I wouldn't expect regressions from it in particular...?)

[nikomatsakis]

@bors r+

[bors]

📌 Commit e567cb5 has been approved by nikomatsakis

[bors]

⌛ Testing commit e567cb5 with merge ecbd8c3...

[ebfull]

@nikomatsakis I asked @brson if it would be a good idea, in case it fixes soundness holes we weren't aware of. I apologize if this was an unnecessary use of crater resources.

[nikomatsakis]

@ebfull oh not to worry, I just wondered if there was some known interaction

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt

[brson]

Oh, shoot, sorry I let this crater run slip. It's still in progress.

[brson]

I've been running into problems with crater. Still trying to get this through.

[brson]

@ebfull Crater found no problems