Overwriting typing #3157
Merged
Overwriting typing #3157
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
Parser Change ChecklistThis PR modifies the parser. Please check that the following tests are updated:
This test should have examples of every new bit of syntax you are adding. Feel free to just check the box if your PR does not actually change the syntax (because it is refactoring the parser, say). |
goldfirere
reviewed
Oct 20, 2024
Co-authored-by: Richard Eisenberg <[email protected]>
Co-authored-by: Richard Eisenberg <[email protected]>
Co-authored-by: Richard Eisenberg <[email protected]>
goldfirere
reviewed
Oct 23, 2024
ocaml/testsuite/tests/typing-unique/overwriting_proj_push_down_bug.ml
Outdated
Show resolved
Hide resolved
goldfirere
reviewed
Oct 23, 2024
goldfirere
reviewed
Oct 23, 2024
goldfirere
reviewed
Oct 24, 2024
Sadly this doesn't actually work in the debugger.
goldfirere
force-pushed
the
overwriting-typing
branch
from
a8bac1f to
fd050dc
Compare
riaqn
reviewed
Oct 30, 2024
riaqn
reviewed
Oct 30, 2024
…ng tags (to be reverted)
…rned_tags instead of seq
goldfirere
reviewed
Nov 8, 2024
There was a problem hiding this comment.
Reviewed only the change to the tests -- I want to make sure we agree on the specification of what this is supposed to do before diving into the implementation. In particular, I'm worried about the case where a mutation on an untaken branch matters; that suggests to me that something is slightly off here.
|
Somehow that last slate of comments included some stale ones that I must have forgotten to push |
|
I've reviewed the test changes in the last three commits. Looks good! I think I will be able to review the new bits in uniqueness analysis today. |
|
This looks ready for merging to me. 🎆 (but I don't want to click the button in case there's something I'm missing) |
anfelor
added a commit
to anfelor/flambda-backend
that referenced
this pull request
Nov 27, 2024
* First draft of type checking * Check in uniqueness analysis that tags stay the same during overwriting * Report a syntax error on overwriting anything except an allocation * Full type-checking for tuples and records * Type checking fully implemented * Clean up type-checking and add ability to overwrite arbitrary expressions * Fix checking of tags * Test (labeled) tuples * Final tests and allow overwrite of inlined records * Review suggestion Co-authored-by: Richard Eisenberg <[email protected]> * Review suggestion Co-authored-by: Richard Eisenberg <[email protected]> * Review feedback: add more tests * Uniqueness analysis: Change grammar of error message to indicate parallel evaluation * Review feedback: Fix typo Co-authored-by: Richard Eisenberg <[email protected]> * Review feedback: add comments and tests * Prevent nested allocations in overwrite * Review feedback: Fix comment and improve readability * Review feedback: add comments, tests and nicer error messages * Fix nested record bug * Improve code quality * Disallow overwriting of unboxed records * Review feedback * Clarify the overwriting state for type_label_exp * Add debugging printers for uniqueness Sadly this doesn't actually work in the debugger. * Temporarily allow unique mutable fields for better tests of overwriting tags (to be reverted) * New scheme for checking that overwrite does not change tags * Refactor: split up Learned_tags and Overwrites and use match_with_learned_tags instead of seq * Add comment about fold_left1 and zero of semiring * Fix submode error attribution in records * Tune errors in tag checker * Don't record tags that have been overwritten and matched with a learned tag * Review feedback: comments and small improvements * Review feedback: tune comments --------- Co-authored-by: Richard Eisenberg <[email protected]> Co-authored-by: Richard Eisenberg <[email protected]>
|
Post merge: We accidentally forgot to revert c29a7bb. This enables unique mutable record fields but we do not want to enable this feature yet (and it is unclear what other safeguards we need to implement). This change was separately reverted in be5eded. If you plan on using the code from this PR, please make sure to also include that commit. |
anfelor
added a commit
to anfelor/flambda-backend
that referenced
this pull request
Dec 3, 2024
* First draft of type checking * Check in uniqueness analysis that tags stay the same during overwriting * Report a syntax error on overwriting anything except an allocation * Full type-checking for tuples and records * Type checking fully implemented * Clean up type-checking and add ability to overwrite arbitrary expressions * Fix checking of tags * Test (labeled) tuples * Final tests and allow overwrite of inlined records * Review suggestion Co-authored-by: Richard Eisenberg <[email protected]> * Review suggestion Co-authored-by: Richard Eisenberg <[email protected]> * Review feedback: add more tests * Uniqueness analysis: Change grammar of error message to indicate parallel evaluation * Review feedback: Fix typo Co-authored-by: Richard Eisenberg <[email protected]> * Review feedback: add comments and tests * Prevent nested allocations in overwrite * Review feedback: Fix comment and improve readability * Review feedback: add comments, tests and nicer error messages * Fix nested record bug * Improve code quality * Disallow overwriting of unboxed records * Review feedback * Clarify the overwriting state for type_label_exp * Add debugging printers for uniqueness Sadly this doesn't actually work in the debugger. * Temporarily allow unique mutable fields for better tests of overwriting tags (to be reverted) * New scheme for checking that overwrite does not change tags * Refactor: split up Learned_tags and Overwrites and use match_with_learned_tags instead of seq * Add comment about fold_left1 and zero of semiring * Fix submode error attribution in records * Tune errors in tag checker * Don't record tags that have been overwritten and matched with a learned tag * Review feedback: comments and small improvements * Review feedback: tune comments --------- Co-authored-by: Richard Eisenberg <[email protected]> Co-authored-by: Richard Eisenberg <[email protected]>
anfelor
added a commit
to anfelor/flambda-backend
that referenced
this pull request
Dec 5, 2024
* First draft of type checking * Check in uniqueness analysis that tags stay the same during overwriting * Report a syntax error on overwriting anything except an allocation * Full type-checking for tuples and records * Type checking fully implemented * Clean up type-checking and add ability to overwrite arbitrary expressions * Fix checking of tags * Test (labeled) tuples * Final tests and allow overwrite of inlined records * Review suggestion Co-authored-by: Richard Eisenberg <[email protected]> * Review suggestion Co-authored-by: Richard Eisenberg <[email protected]> * Review feedback: add more tests * Uniqueness analysis: Change grammar of error message to indicate parallel evaluation * Review feedback: Fix typo Co-authored-by: Richard Eisenberg <[email protected]> * Review feedback: add comments and tests * Prevent nested allocations in overwrite * Review feedback: Fix comment and improve readability * Review feedback: add comments, tests and nicer error messages * Fix nested record bug * Improve code quality * Disallow overwriting of unboxed records * Review feedback * Clarify the overwriting state for type_label_exp * Add debugging printers for uniqueness Sadly this doesn't actually work in the debugger. * Temporarily allow unique mutable fields for better tests of overwriting tags (to be reverted) * New scheme for checking that overwrite does not change tags * Refactor: split up Learned_tags and Overwrites and use match_with_learned_tags instead of seq * Add comment about fold_left1 and zero of semiring * Fix submode error attribution in records * Tune errors in tag checker * Don't record tags that have been overwritten and matched with a learned tag * Review feedback: comments and small improvements * Review feedback: tune comments --------- Co-authored-by: Richard Eisenberg <[email protected]> Co-authored-by: Richard Eisenberg <[email protected]>
anfelor
added a commit
that referenced
this pull request
Dec 6, 2024
* Basic overwriting syntax (#3089) * Basic overwriting syntax * Update line numbers in expect tests after cherry-pick * Implement suggestions * Update tests * Implement suggestions * Remove nested quotes * Revert "Remove nested quotes" This reverts commit 72eaa04. * Overwriting tests (#3123) * Tests for overwriting * Remove old tests for unique overwrites and fix typos * Add overwriting tests back * Add tests for lifting out constants * Update tests * Use extension universe alpha * Test all cases for gc soundness bug * Review feedback: add more CRs and clarify overwriting_map.ml * Review feedback: Add allocation counter to rbtree * Review feedback: also test mixed blocks and unboxed float blocks in the lift constants test * Review feedback: as discussed in meeting * Review feedback * Overwriting typing (#3157) * First draft of type checking * Check in uniqueness analysis that tags stay the same during overwriting * Report a syntax error on overwriting anything except an allocation * Full type-checking for tuples and records * Type checking fully implemented * Clean up type-checking and add ability to overwrite arbitrary expressions * Fix checking of tags * Test (labeled) tuples * Final tests and allow overwrite of inlined records * Review suggestion Co-authored-by: Richard Eisenberg <[email protected]> * Review suggestion Co-authored-by: Richard Eisenberg <[email protected]> * Review feedback: add more tests * Uniqueness analysis: Change grammar of error message to indicate parallel evaluation * Review feedback: Fix typo Co-authored-by: Richard Eisenberg <[email protected]> * Review feedback: add comments and tests * Prevent nested allocations in overwrite * Review feedback: Fix comment and improve readability * Review feedback: add comments, tests and nicer error messages * Fix nested record bug * Improve code quality * Disallow overwriting of unboxed records * Review feedback * Clarify the overwriting state for type_label_exp * Add debugging printers for uniqueness Sadly this doesn't actually work in the debugger. * Temporarily allow unique mutable fields for better tests of overwriting tags (to be reverted) * New scheme for checking that overwrite does not change tags * Refactor: split up Learned_tags and Overwrites and use match_with_learned_tags instead of seq * Add comment about fold_left1 and zero of semiring * Fix submode error attribution in records * Tune errors in tag checker * Don't record tags that have been overwritten and matched with a learned tag * Review feedback: comments and small improvements * Review feedback: tune comments --------- Co-authored-by: Richard Eisenberg <[email protected]> Co-authored-by: Richard Eisenberg <[email protected]> * Revert c29a7bb to disable unique mutable fields again * Remove out-of-date CRs --------- Co-authored-by: Richard Eisenberg <[email protected]> Co-authored-by: Richard Eisenberg <[email protected]>
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.
This PR contains the type checking code for the overwriting extension.
The translation in
translcore.mlis not implemented, that is every testcase that currently prints withAlert Translcoreactually type-checks successfully.