inference: fix the correctness of inference bail out interface #48826
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Added the 1.9 backport label since this is somewhat critical fix on our effects system and so should be fixed even if the original issue doesn't happen on 1.9. |
vtjnash
reviewed
Feb 28, 2023
vtjnash
approved these changes
Feb 28, 2023
aviatesk
changed the title
_apply_iterate
Since we allow overloading of the `bail_out_xxx` hooks, we need to make sure that we widen both type and effects to the top when bailing on inference regardless of the condition presumed by a hook. This commit particularly fixes the correctness of `bail_out_apply` (fixes #48807). I wanted to make a simplified test case for this, but it turns out to be a bit tricky since it relies on the details of multiple match analysis and the bail out logic.
|
@nanosoldier |
|
Your benchmark job has completed - no performance regressions were detected. A full report can be found here. |
aviatesk
added a commit
that referenced
this pull request
Mar 1, 2023
Since we allow overloading of the `bail_out_xxx` hooks, we need to make sure that we widen both type and effects to the top when bailing on inference regardless of the condition presumed by a hook. This commit particularly fixes the correctness of `bail_out_apply` (fixes #48807). I wanted to make a simplified test case for this, but it turns out to be a bit tricky since it relies on the details of multiple match analysis and the bail out logic.
KristofferC
pushed a commit
that referenced
this pull request
Mar 3, 2023
Since we allow overloading of the `bail_out_xxx` hooks, we need to make sure that we widen both type and effects to the top when bailing on inference regardless of the condition presumed by a hook. This commit particularly fixes the correctness of `bail_out_apply` (fixes #48807). I wanted to make a simplified test case for this, but it turns out to be a bit tricky since it relies on the details of multiple match analysis and the bail out logic.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since we allow overloading of the
bail_out_xxxhooks, we need to makesure that we widen both type and effects to the top when bailing on
inference regardless of the condition presumed by a hook.
This commit particularly fixes the correctness of
bail_out_apply(fixes #48807). I wanted to make a simplified test case for this, but
it turns out to be a bit tricky since it relies on the details of
multiple match analysis and the bail out logic.