-
Notifications
You must be signed in to change notification settings - Fork 86
Make bind &
tighter than mod
in jkind parsing
#3076
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
Merged
Merged
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
Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Note that ``` type t3 : any & (any mod non_null) = #(t1 * t2);; ``` Does not have the same behavior as ``` type t3 : any & any mod non_null = #(t1 * t2);; ``` Signed-off-by: David Vulakh <dvulakh@janestreet.com>
(We discussed in person and David is also going to add a test to |
the test currently fails, so yes, we need to update the kind printer Signed-off-by: David Vulakh <dvulakh@janestreet.com>
there are sometimes unnecessary parentheses, but that seems fine in the name of both clarity of printed syntax and printer simplicity added a test with [with] along the way Signed-off-by: David Vulakh <dvulakh@janestreet.com>
and change the jkind bits of pprint ast to use [Misc.pp_parens_if] now that it's exposed Signed-off-by: David Vulakh <dvulakh@janestreet.com>
ccasin
approved these changes
Sep 25, 2024
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.
LGTM
lukemaurer
pushed a commit
to lukemaurer/flambda-backend
that referenced
this pull request
Oct 23, 2024
…caml-flambda#3076) * make `&` tighter than `mod` in jkind parsing Signed-off-by: David Vulakh <dvulakh@janestreet.com> * forbid `k mod m & l` without parens around `mod` Signed-off-by: David Vulakh <dvulakh@janestreet.com> * reflect new syntax in tests Signed-off-by: David Vulakh <dvulakh@janestreet.com> * revert 1eea0c2 Signed-off-by: David Vulakh <dvulakh@janestreet.com> * add typing test to demo `&` and `mod` precedence Note that ``` type t3 : any & (any mod non_null) = #(t1 * t2);; ``` Does not have the same behavior as ``` type t3 : any & any mod non_null = #(t1 * t2);; ``` Signed-off-by: David Vulakh <dvulakh@janestreet.com> * add test to [pprintast_unconditional.ml] the test currently fails, so yes, we need to update the kind printer Signed-off-by: David Vulakh <dvulakh@janestreet.com> * add more parentheses in printer there are sometimes unnecessary parentheses, but that seems fine in the name of both clarity of printed syntax and printer simplicity added a test with [with] along the way Signed-off-by: David Vulakh <dvulakh@janestreet.com> * fix oprint and change the jkind bits of pprint ast to use [Misc.pp_parens_if] now that it's exposed Signed-off-by: David Vulakh <dvulakh@janestreet.com> --------- Signed-off-by: David Vulakh <dvulakh@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.
Currently,
parses as
The crowdsourced concensus is that this is confusing. This PR makes it parse as
instead.
Why not require the user to be explicit and write parentheses either way? We'll likely make
ocamlformat
add more parentheses than are necessary, but feel that requiring parentheses in the parser would hurt approachability / discoverability of the new syntax.