Unboxed tuples #71
Merged
Unboxed tuples #71
Conversation
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
d154c6a to
9fc13f3
Compare
ccasin
reviewed
May 31, 2024
ccasin
left a comment
There was a problem hiding this comment.
This looks pretty good! A few questions/suggestions.
4f24ce1 to
25fb747
Compare
ccasin
approved these changes
Jun 3, 2024
ccasin
left a comment
There was a problem hiding this comment.
Looks good - thanks for the explanations. I'll let you decide when is the right time to merge this. Unboxed tuples in the compiler are still a couple weeks out.
25fb747 to
bc4a43a
Compare
Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com> Initial attempt. Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com> Update tests. Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com> Formatting Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com> Formatting. Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com> Fix ups Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com> Updated tests. Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com> Fix up mapper. Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com> Fix paren of last element of tuple. Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com> Factor out code. Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com> Fixups. Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com> Add test demonstrating behavior in special case. Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com> Add more tests. Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>
bc4a43a to
a0e59ad
Compare
mdelvecchio-jsc
added a commit
that referenced
this pull request
Jun 18, 2024
Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>
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.
Add support for unboxed tuples to ocamlformat. Formatting aligns as directly as possible with formatting of regular tuples, except that parentheses are always required, and some aligning needs to be adjusted to account for the extra '#' character.
Testing: Transformed every tuple in jane to an unboxed tuple, and ran ocamlformat on the entire tree successfully. Additionally skimmed the diff, and saw that formatting was reasonable. Added a test file: copied the existing tests for labeled tuples and adjusted it a bit, with some additional regression tests at the bottom.