Revert "Subtype: avoid false alarm caused by eager forall_exists_subtype. (#48441)"

[kpamnany]

Closes #49857.

This reverts commit 2a2068d2677eae3fe2101c89f9ed6c98e51f3a62 which was from PR #48441. Removing this fixes the freeze but since #48441 fixed #40865, merging this will reopen that issue.

It's probably safe to say that a freeze is worse than an error but we're open to discussion for how to best resolve this. Cc: @AzamatB

Please note that #49857 means that 1.9.0 is broken for us and unless all commits affecting subtype.c up to and including 280f9993608956f76eac30fc85e1c6ebbca4f5e6 are backported, all of 1.9 is broken for us. Cc: @vtjnash and @N5N3, the author of #48441.

Cc: @KristofferC to confirm that I've opened the PR correctly to land it in 1.9.1 if it's merged.

Cc: @quinnj and @NHDaly

[KristofferC]

unless all commits affecting subtype.c up to and including 280f9993608956f76eac30fc85e1c6ebbca4f5e6 are backported, all of 1.9 is broken for us

Why is this the case?

[kpamnany]

Why is this the case?

See the issue. We ran into this while testing with 1.9.0 -- a pretty basic call (create_database()) never returned due to this freeze.

[KristofferC]

Yes, but why does "all commits to subtype.c have to be included" and not just the one that fixes the problem?

[kpamnany]

Ah. Because I went 4 PRs deep in trying to get the fix commit to apply and was still getting merge conflicts. It's certainly possible that someone who knows subtype.c better can reduce the needed commits -- it was looking like over a dozen commits needed to me.

[KristofferC]

Maybe @N5N3 can give some input.

[NHDaly]

@kpamnany's comment above was also based on this discussion with @vtjnash on slack:

@vtjnash: Ah, that might be tricky. We had a lot of subtyping fixes like this on master, so we were planning not to backport most of them
[...]
@vtjnash: you might want to take the whole list of changes to subtype.c and try to apply all of them up to that commit
@kpamnany: I've instead tried to back out the breaking commit. There were only a couple of conflicts which seemed simple to resolve. It's building now.

Which is how we ended up here.

[N5N3]

Apparently, we get a Union explosion here, and 280f999 avoid that with separatable subtyping.
We can't just revert #48441 (#48441 was backported to fix the bug caused by #48221).
A better fix (if we don't want to backport 280f999 ) is to disable the slow path in local_forall_exists_subtype.

[kpamnany]

A better fix (if we don't want to backport 280f999 ) is to disable the slow path in local_forall_exists_subtype.

It isn't that we don't want to backport it -- it depends on a previous commit, etc.

Could you please clarify disabling the slow path? Do what in this else block?

[N5N3]

Replacing if (noRmore || (limit_slow && (count > 3 || !sub))) above with if (noRmore || limit_slow) should be enough.

BTW, we’d better test #49014 in this PR.

[kpamnany]

Please see #49875. I'll close this if that's a more acceptable solution.