jkind subsumption #3525
Open
jkind subsumption #3525
+2,548
−1,550
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
liam923
force-pushed
the
liam-jkind-subsumption
branch
3 times, most recently
from
bf21eef to
8ff9303
Compare
liam923
commented
Jan 29, 2025
typing/typedecl.ml
Outdated
| match decl.type_manifest, decl.type_kind with | ||
| | None, _ -> decl | ||
| | Some _, (Type_record _ | Type_record_unboxed_product _ | Type_variant _ | Type_open) | ||
| when not (Builtin_attributes.has_or_null_reexport decl.type_attributes) |
There was a problem hiding this comment.
Revisit this after rebase
goldfirere
reviewed
Jan 29, 2025
glittershark
force-pushed
the
aspsmith/normalize-jkind
branch
from
6bea51c to
a1afe21
Compare
liam923
force-pushed
the
liam-jkind-subsumption
branch
from
2229bad to
ee61b4c
Compare
|
Note to self: I've reviewed through "Promote tests" (except for the files that GitHub knows I haven't finished yet) |
goldfirere
reviewed
Jan 31, 2025
|
After reviewing the tests, I'm pleased to report that I didn't spot any soundness bugs! 🔉 Now onto the actual implementation, which I can happily lower my level of scrutiny, as I'm pretty sure it's basically correct via the tests. |
goldfirere
reviewed
Feb 2, 2025
|
OK. I have reviewed everything in this patch up to now. Looking pretty good! |
goldfirere
reviewed
Feb 3, 2025
|
I've reviewed commits up through "Improve subsumption comments" |
…iam-jkind-subsumption
liam923
commented
Feb 6, 2025
|
Reviewed most recent commits. |
…iam-jkind-subsumption
…iam-jkind-subsumption
|
Reviewed most recent commits. |
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.
Improve the implementation of
Jkind.sub_jkind_lto give a proper subsumption check. The implementation is based on the rules injane/doc/proposals/kind-inference.md. At the moment, this check is quadratic because we don't have a proper comparison function on types. But based on my anecdotal experience building and running tests, this doesn't seem to hurt performance too bad.