Conversation
I think the time has come to flip this. This was added when the type system was much less reliable at producing intersections. It is true that we still have the occasional type sytem bug, but we're already trusting inference in a bunch of other places. At the same time, the cost of this has grown in terms of bloated IR needing to be visited in places like irinterp, so let's flip the bit and we'll deal with type system bugs the way we usually due.
aviatesk
reviewed
Jul 6, 2023
Member
aviatesk
left a comment
aviatesk
left a comment
There was a problem hiding this comment.
I'm fine with this. Can't we just delete this option entirely and remove the related branches also?
Member
|
I agree with @aviatesk |
Member
|
Refactored. |
oscardssmith
changed the title
trust_inference option
vtjnash
approved these changes
Jul 6, 2023
Member
|
Not sure why |
Member
Author
Known rr bug rr-debugger/rr#3468 |
maleadt
reviewed
Jul 7, 2023
| # We're now in the fall through block, decide what to do | ||
| if fully_covered | ||
| if !params.trust_inference | ||
| e = Expr(:call, GlobalRef(Core, :throw), FATAL_TYPE_BOUND_ERROR) |
Member
There was a problem hiding this comment.
Any reason you didn't remove the FATAL_TYPE_BOUND_ERROR definition?
Member
There was a problem hiding this comment.
was this the only use of it? If so the reason is because I didn't notice that there weren't any other uses.
vtjnash
pushed a commit
that referenced
this pull request
Aug 31, 2023
Fix #50709 This issue *appears* fixed on master due to #50432, which simply removed the error. This fixes the underlying cause, which is that we sometimes return typevars in the environment from intersection that have free vars in their bounds. I think it's reasonable to just widen these aggressively, since we usually cannot do much with these kinds of unknown static parameters anyway.
IanButterworth
pushed a commit
that referenced
this pull request
Sep 16, 2023
Fix #50709 This issue *appears* fixed on master due to #50432, which simply removed the error. This fixes the underlying cause, which is that we sometimes return typevars in the environment from intersection that have free vars in their bounds. I think it's reasonable to just widen these aggressively, since we usually cannot do much with these kinds of unknown static parameters anyway. (cherry picked from commit a3e2316)
nalimilan
pushed a commit
that referenced
this pull request
Nov 5, 2023
Fix #50709 This issue *appears* fixed on master due to #50432, which simply removed the error. This fixes the underlying cause, which is that we sometimes return typevars in the environment from intersection that have free vars in their bounds. I think it's reasonable to just widen these aggressively, since we usually cannot do much with these kinds of unknown static parameters anyway. (cherry picked from commit a3e2316)
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
I think the time has come to flip this. This was added when the type system was much less reliable at producing intersections. It is true that we still have the occasional type sytem bug, but we're already trusting inference in a bunch of other places. At the same time, the cost of this has grown in terms of bloated IR needing to be visited in places like irinterp, so let's flip the bit and we'll deal with type system bugs the way we usually due.