-
-
Notifications
You must be signed in to change notification settings - Fork 418
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix an issue with infinite loops while typechecking some expressions #4274
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
550bbb8
Fix coinductive subtyping of interfaces.
1532f16
Traits also need coinductivity due to f-bounded arguments
89197c3
Add subtype test for this case with U8
e2f9bfe
Don't restrict which subtype checks use coinductivity for equality
2c602b7
Update test
2bb3d71
Change test to be more precise
9796b95
Add release notes
3f04bfa
Fix/Clean test for test environment
21747ad
Move regression test to libponyc-run
e0518ed
Fix name overlap
f16a0ea
Update comment to clarify check timing.
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
## Fix some infinite loops during typechecking | ||
|
||
Some recursive constraints could cause the typechecker to go into | ||
an infinite loop while trying to check subtyping. These infinite | ||
loops have now been fixed. An example program that demonstrates | ||
the issue is below: | ||
|
||
``` | ||
primitive Foo[T: (Unsigned & UnsignedInteger[T])] | ||
fun boom() => | ||
iftype T <: U8 then | ||
None | ||
end | ||
|
||
actor Main | ||
new create(env: Env) => | ||
Foo[U8].boom() | ||
``` | ||
|
||
The use of T: UnsignedInteger[T] is valid and expected, but wasn't | ||
being handled correctly in this situation by the typechecker in this context. |
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
primitive Foo[T: (Unsigned & UnsignedInteger[T])] | ||
fun boom() => | ||
iftype T <: U8 then | ||
None | ||
end | ||
// more cases | ||
// note that these method bodies are checked at their definition | ||
// site so there's no need to call them | ||
fun baam1[S: (Unsigned & UnsignedInteger[S] & U8)](t: S): U32 => | ||
t.u32() | ||
fun baam2[S: (U8 & Unsigned & UnsignedInteger[S])](t: S): U32 => | ||
t.u32() | ||
|
||
actor Main | ||
new create(env: Env) => | ||
Foo[U8].boom() |
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comment at the top level of this file explaining in general that this is a compile time check so the "additional cases" are covered because this is compile time, not runtime checking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in f16a0ea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Er, I added the comment locally, not at the top, is that fine?