Don't desugar let _ = local_ x
#21
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
Author
|
I noticed more cases of invalid syntax and restricted the desugaring even more. Examples of invalid syntaxes that were generated: let local_ () = x
let local_ {b} = y |
ccasin
reviewed
Jun 28, 2023
lib/Sugar.ml
Outdated
| annotations and extensions into the pattern and the | ||
| expression. *) | ||
| match lb_pat.ppat_desc with | ||
| | Ppat_var _ | Ppat_constraint ({ppat_desc = Ppat_var _;_}, _) -> |
There was a problem hiding this comment.
If I understand correctly, this will cause let (x : string) = local_ "hi" in ... to be rewritten to let local_ (x : string) = "hi" in .... But local_ only works on a let binding if the pattern is exactly a variable.
Author
There was a problem hiding this comment.
This indeed cause a syntax error but an equivalent syntax exists:
let local_ x : string = local_ "hi"This rewrite could be allowed by improving the formatting function.
Author
There was a problem hiding this comment.
The formatting function is not the only problem. The AST of these two lines is different:
let local_ x : string = local_ "hi"
let (x : string) = local_ "hi"I added a commit to distinguish between these two cases (and a third one more complex). The problem is fixed but the code is complex and the rewrite don't happen when it could.
To go further, the parser should be changed to place the constraint in the same place in both cases.
Here are the ASTs:
Line 1:
Ptop_def
[
structure_item (_none_[0,0+-1]..[0,0+-1]) ghost
Pstr_value Nonrec
[
<def>
pattern (_none_[0,0+-1]..[0,0+-1]) ghost
Ppat_var "x" (_none_[0,0+-1]..[0,0+-1]) ghost
expression (_none_[0,0+-1]..[0,0+-1]) ghost
Pexp_apply
expression (_none_[0,0+-1]..[0,0+-1]) ghost
Pexp_extension "extension.local"
[]
[
<arg>
Nolabel
expression (_none_[0,0+-1]..[0,0+-1]) ghost
attribute "extension.local"
[]
Pexp_constraint
expression (_none_[0,0+-1]..[0,0+-1]) ghost
Pexp_constant PConst_string("hi",(_none_[0,0+-1]..[0,0+-1]) ghost,None)
core_type (_none_[0,0+-1]..[0,0+-1]) ghost
Ptyp_constr "string" (_none_[0,0+-1]..[0,0+-1]) ghost
[]
]
]
]
Line 2:
Ptop_def
[
structure_item (_none_[0,0+-1]..[0,0+-1]) ghost
Pstr_value Nonrec
[
<def>
pattern (_none_[0,0+-1]..[0,0+-1]) ghost
Ppat_constraint
pattern (_none_[0,0+-1]..[0,0+-1]) ghost
Ppat_var "x" (_none_[0,0+-1]..[0,0+-1]) ghost
core_type (_none_[0,0+-1]..[0,0+-1]) ghost
Ptyp_constr "string" (_none_[0,0+-1]..[0,0+-1]) ghost
[]
expression (_none_[0,0+-1]..[0,0+-1]) ghost
Pexp_apply
expression (_none_[0,0+-1]..[0,0+-1]) ghost
Pexp_extension "extension.local"
[]
[
<arg>
Nolabel
expression (_none_[0,0+-1]..[0,0+-1]) ghost
Pexp_constant PConst_string("hi",(_none_[0,0+-1]..[0,0+-1]) ghost,None)
]
]
]
There was a problem hiding this comment.
I am surprised by the first AST here. The parser in the compiler includes a Ppat_constraint in the pattern for this example, and I am having trouble finding where the ocamlformat parsers differ in a way that would account for the absence of this node.
Author
There was a problem hiding this comment.
I don't know what I did wrong in my previous comment but the ASTs are:
line 1:
Ptop_def
[
structure_item (./test.ml[1,0+0]..[1,0+35])
Pstr_value Nonrec
[
<def> (./test.ml[1,0+0]..[1,0+35])
pattern (./test.ml[1,0+11]..[1,0+21]) ghost
attribute "extension.local" (_none_[0,0+-1]..[0,0+-1]) ghost (_none_[0,0+-1]..[0,0+-1]) ghost
[]
Ppat_constraint
pattern (./test.ml[1,0+11]..[1,0+12])
Ppat_var "x" (./test.ml[1,0+11]..[1,0+12])
core_type (./test.ml[1,0+15]..[1,0+21]) ghost
Ptyp_poly
core_type (./test.ml[1,0+15]..[1,0+21])
Ptyp_constr "string" (./test.ml[1,0+15]..[1,0+21])
[]
expression (./test.ml[1,0+4]..[1,0+35]) ghost
Pexp_apply
expression (_none_[0,0+-1]..[0,0+-1]) ghost
Pexp_extension "extension.local" (_none_[0,0+-1]..[0,0+-1]) ghost
[]
[
<arg>
Nolabel
expression (./test.ml[1,0+4]..[1,0+35])
attribute "extension.local" (_none_[0,0+-1]..[0,0+-1]) ghost (_none_[0,0+-1]..[0,0+-1])
ghost
[]
Pexp_constraint
expression (./test.ml[1,0+24]..[1,0+35]) ghost
Pexp_apply
expression (_none_[0,0+-1]..[0,0+-1]) ghost
Pexp_extension "extension.local" (_none_[0,0+-1]..[0,0+-1]) ghost
[]
[
<arg>
Nolabel
expression (./test.ml[1,0+31]..[1,0+35])
Pexp_constant
constant (./test.ml[1,0+31]..[1,0+35])
PConst_string("hi",(./test.ml[1,0+32]..[1,0+34]),None)
]
core_type (./test.ml[1,0+15]..[1,0+21])
Ptyp_constr "string" (./test.ml[1,0+15]..[1,0+21])
[]
]
]
]
line 2:
Ptop_def
[
structure_item (./test.ml[2,36+0]..[2,36+30])
Pstr_value Nonrec
[
<def> (./test.ml[2,36+0]..[2,36+30])
pattern (./test.ml[2,36+4]..[2,36+16])
Ppat_constraint
pattern (./test.ml[2,36+5]..[2,36+6])
Ppat_var "x" (./test.ml[2,36+5]..[2,36+6])
core_type (./test.ml[2,36+9]..[2,36+15])
Ptyp_constr "string" (./test.ml[2,36+9]..[2,36+15])
[]
expression (./test.ml[2,36+19]..[2,36+30]) ghost
Pexp_apply
expression (_none_[0,0+-1]..[0,0+-1]) ghost
Pexp_extension "extension.local" (_none_[0,0+-1]..[0,0+-1]) ghost
[]
[
<arg>
Nolabel
expression (./test.ml[2,36+26]..[2,36+30])
Pexp_constant
constant (./test.ml[2,36+26]..[2,36+30])
PConst_string("hi",(./test.ml[2,36+27]..[2,36+29]),None)
]
]
]
Both cases have a Ppat_constraint indeed. One have the constraint on the expression, while the other doesn't.
ccasin
reviewed
Jul 4, 2023
| let local_ x : 'a -> 'a -> 'a = "hi" (* Annotation on the pattern *) | ||
| let (x : string) = local_ "hi" (* Annotation on the pattern *) | ||
| ]} *) |
There was a problem hiding this comment.
I'm confused. I took a look at the parsetree produced by the compiler for the first two examples here, and they are the same. So I suppose there is a difference in the ocamlformat parsers - is there an easy way to get ocamlformat to dump the parsetree so I can understand better? (I don't see a -dparsetree equivalent in the options).
Author
There was a problem hiding this comment.
I use dune exec -- tools/printast/printast.exe ./test.ml to read the AST.
The two examples indeed have the same AST, I fixed the comment in a new commit.
There was a problem hiding this comment.
Thanks!
I think an unfortunate related case is:
let x : 'a . 'a -> 'a -> 'a = local_ "hi"If I understand correctly, this will currently be rewritten in such a way as to fail the roundtrip test because we are missing the attribute on the pattern. I'm not sure what the best resolution is - we could check perhaps for the attribute when deciding to sugar?
Also, this comment is very helpful, but I think it could be slightly clearer in a few ways:
- Because it's a comment involving
local_, my first guess when reading it was that "annotation on the ..." was a reference to howlocal_is parsed, but I see now it's a reference to how the type annotation is parsed. So perhaps say "type annotation" or "constraint node" to make it more obvious. - The comment and the name of the function refer to "sugar" but don't say what is being sugared. One of them should mention it's a question about when we can sugar "local_" in particular.
- I think in the first case the type annotation ends up on both the expression and the pattern, not just the expression.
There was a problem hiding this comment.
Sorry, one more annoying request: The comment should probably explain how the location comparison in the code is accomplishing your goal (though I haven't fully understood this bit yet).
Author
There was a problem hiding this comment.
The function name is unclear but it's specifically about "local lets" (whether it can be rewritten into let local_ x ...).
I'll improve that and look at the new case.
This code is a hack and not the solution. The solution is to work on the parsers:
- The standard parser should output identical ASTs for pretended-identical syntaxes. This makes the rules of what we can rewrite clearer.
- The extended parser should output different ASTs for the different syntaxes. This avoids the need to guess the current syntax and the possible rewrites.
There was a problem hiding this comment.
All this makes sense - but what should we do in the short term about failing on
let x : 'a . 'a -> 'a -> 'a = local_ "hi"?
If my suggestion to check for the presence of the attribute on the pattern and not sugar in that case works, perhaps you can just add that to this PR, perhaps add a little more text to the comment to explain the code and document that we intend to come back, and then we can merge this.
If that doesn't work and no other easy fix is available, then we can just merge it as is, with a comment documenting the issue.
Or are you proposing we shouldn't merge this at all and wait until someone has time to change the extended AST?
There was a problem hiding this comment.
(Also, the CI tests are currently failing on this PR. I think we expect the windows test to fail, but not the others - probably it just needs to be rebased).
Author
There was a problem hiding this comment.
This PR is already fixing bugs, so it can be merged at any time. I'll look at this case and CI shortly.
Author
There was a problem hiding this comment.
I fixed the bug you mentioned and did a rebase. I hope the comment is clearer too.
The fix is mostly to not rewrite into the shorter form when that would mess with the formatting code or change the AST. A more concrete AST will allow to remove the tricky code and to rewrite in more cases.
There was a problem hiding this comment.
Great - thanks for dealing with all these requests!
The desugared syntax `let local_ _ = x` is invalid. Signed-off-by: Jules Aguillon <jules@j3s.fr>
This restricts the desugaring of `let x = local_ y` to the cases where `x` is an identifier or an identifier with a type annotation. Signed-off-by: Jules Aguillon <jules@j3s.fr>
There are syntax rewrite on locals, it's important not to change the test input.
These two items don't have the same AST and must be recognized:
let local_ x : string = "hi"
let (x : string) = local_ "hi"
These two items don't have the same AST:
let x : 'a . 'a -> 'a = local_ "hi"
let local_ x : 'a. 'a -> 'a = "hi"
ccasin
approved these changes
Jul 6, 2023
| let local_ x : 'a -> 'a -> 'a = "hi" (* Annotation on the pattern *) | ||
| let (x : string) = local_ "hi" (* Annotation on the pattern *) | ||
| ]} *) |
There was a problem hiding this comment.
Great - thanks for dealing with all these requests!
mdelvecchio-jsc
pushed a commit
that referenced
this pull request
Jul 20, 2023
* Don't desugar `let _ = local_ x`
The desugared syntax `let local_ _ = x` is invalid.
Signed-off-by: Jules Aguillon <jules@j3s.fr>
* Restrict `let local_ ...` rewrite even more
This restricts the desugaring of `let x = local_ y` to the cases where
`x` is an identifier or an identifier with a type annotation.
Signed-off-by: Jules Aguillon <jules@j3s.fr>
* Tests: Add a .ref file for local.ml
There are syntax rewrite on locals, it's important not to change the
test input.
* Fix local_ desugaring generating invalid syntax
These two items don't have the same AST and must be recognized:
let local_ x : string = "hi"
let (x : string) = local_ "hi"
* Preserve `local_` position when ptyp_poly constraint
These two items don't have the same AST:
let x : 'a . 'a -> 'a = local_ "hi"
let local_ x : 'a. 'a -> 'a = "hi"
---------
Signed-off-by: Jules Aguillon <jules@j3s.fr>
ccasin
pushed a commit
that referenced
this pull request
Sep 19, 2023
* Don't desugar `let _ = local_ x`
The desugared syntax `let local_ _ = x` is invalid.
Signed-off-by: Jules Aguillon <jules@j3s.fr>
* Restrict `let local_ ...` rewrite even more
This restricts the desugaring of `let x = local_ y` to the cases where
`x` is an identifier or an identifier with a type annotation.
Signed-off-by: Jules Aguillon <jules@j3s.fr>
* Tests: Add a .ref file for local.ml
There are syntax rewrite on locals, it's important not to change the
test input.
* Fix local_ desugaring generating invalid syntax
These two items don't have the same AST and must be recognized:
let local_ x : string = "hi"
let (x : string) = local_ "hi"
* Preserve `local_` position when ptyp_poly constraint
These two items don't have the same AST:
let x : 'a . 'a -> 'a = local_ "hi"
let local_ x : 'a. 'a -> 'a = "hi"
---------
Signed-off-by: Jules Aguillon <jules@j3s.fr>
mdelvecchio-jsc
pushed a commit
that referenced
this pull request
Sep 20, 2023
* Don't desugar `let _ = local_ x`
The desugared syntax `let local_ _ = x` is invalid.
Signed-off-by: Jules Aguillon <jules@j3s.fr>
* Restrict `let local_ ...` rewrite even more
This restricts the desugaring of `let x = local_ y` to the cases where
`x` is an identifier without a type annotation.
Otherwise, just preserves sugaring in original file.
Signed-off-by: Jules Aguillon <jules@j3s.fr>
* Tests: Add a .ref file for local.ml
There are syntax rewrite on locals, it's important not to change the
test input.
* Fix local_ desugaring generating invalid syntax
These two items don't have the same AST and must be recognized:
let local_ x : string = "hi"
let (x : string) = local_ "hi"
---------
Signed-off-by: Jules Aguillon <jules@j3s.fr>
mdelvecchio-jsc
pushed a commit
that referenced
this pull request
Oct 12, 2023
(Note for future rebases: at this stage it is expected that `dune b
@test/passing/runtest` will give errors related to extra
`[@ocaml.curry]`s)
local: support local_ and curry in function types
Note for future rebases: it is expected that `dune b
@test/passing/runtest` will fail at this stage, in my case with
output:
File "test/passing/dune.inc", line 3594, characters 0-205:
3594 | (rule
3595 | (deps tests/.ocamlformat )
3596 | (package ocamlformat)
3597 | (action
3598 | (with-stdout-to local.ml.stdout
3599 | (with-stderr-to local.ml.stderr
3600 | (run %{bin:ocamlformat} --margin-check %{dep:tests/local.ml})))))
Command exited with code 1.
local: support 'fun (local_ x) ... -> '
local: nonlocal/global record field decls
For future reference during rebasing: `dune b @test/passing/runtest`
is not expected to pass on this commit. Some uses of `local_` are
turned into `[%extension.local]`.
local: return expressions 'fun ... -> local_ ...'
For reference during future rebases: `dune b @test/passing/runtest` is
expected to pass on this commit.
local: apply formatting
For future rebases: `make test` should pass at this stage. It may be
necessary to run `dune fmt` and `make regtests-promote` (but I checked
`dune b @test/passing/runtest` succeeded first)
Update to ocaml-jst a78975eb57
Autogenerated attributes are now 'extension.foo' instead of 'ocaml.foo'
change module types to export equalities
Signed-off-by: Cameron Wong <cwong@janestreet.com>
Support for "include functor" (#5)
* Initial support for the include_functor extension
* Add test for include functor
Signed-off-by: Chris Casinghino <ccasinghino@janestreet.com>
local: bugfix for punned named arguments with type annotations (#3)
A bug in the previous patch for locals causes ocamlformat to incorrectly
format local arguments which are punned with a label and have a type signature:
fun ~(local_ x : t) -> ...
by putting an extra invalid pair of parens around (x : t). This patch removes
the parens.
Signed-off-by: Stephen Dolan <sdolan@janestreet.com>
Ensure parentheses are placed around (local_ E) when necessary (#4)
Signed-off-by: Stephen Dolan <sdolan@janestreet.com>
Remove unnecessary parentheses around expressions with trailing [@attrs] (#6)
Signed-off-by: Stephen Dolan <sdolan@janestreet.com>
update parsers and printer for polymorphic parameters (with tests)
Signed-off-by: Chris Casinghino <ccasinghino@janestreet.com>
Support for comprehensions and immutable arrays
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
Update to the improved language extension infrastructure
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
add support for global constructor arguments
Signed-off-by: Zesen Qian <github@riaqn.org>
support for layout annotations on type parameters
Signed-off-by: Chris Casinghino <ccasinghino@janestreet.com>
Address review feedback.
Signed-off-by: Chris Casinghino <ccasinghino@janestreet.com>
Fix a test failure due to misformatted comment
Signed-off-by: Chris Casinghino <ccasinghino@janestreet.com>
Fix a (CI) test failure in help printing due to version mismatch
Signed-off-by: Chris Casinghino <ccasinghino@janestreet.com>
support exclave_ syntax
Signed-off-by: Zesen Qian <github@riaqn.org>
no fail fast (#24)
Signed-off-by: Zesen Qian <github@riaqn.org>
Don't desugar `let _ = local_ x` (#21)
* Don't desugar `let _ = local_ x`
The desugared syntax `let local_ _ = x` is invalid.
Signed-off-by: Jules Aguillon <jules@j3s.fr>
* Restrict `let local_ ...` rewrite even more
This restricts the desugaring of `let x = local_ y` to the cases where
`x` is an identifier or an identifier with a type annotation.
Signed-off-by: Jules Aguillon <jules@j3s.fr>
* Tests: Add a .ref file for local.ml
There are syntax rewrite on locals, it's important not to change the
test input.
* Fix local_ desugaring generating invalid syntax
These two items don't have the same AST and must be recognized:
let local_ x : string = "hi"
let (x : string) = local_ "hi"
* Preserve `local_` position when ptyp_poly constraint
These two items don't have the same AST:
let x : 'a . 'a -> 'a = local_ "hi"
let local_ x : 'a. 'a -> 'a = "hi"
---------
Signed-off-by: Jules Aguillon <jules@j3s.fr>
better exclave_ formatting (#28)
Signed-off-by: Zesen Qian <github@riaqn.org>
Support unboxed float literals (#25)
* Automatic changes to patch files
This just happened when I hit `dune build`. Don't really know what's
going on.
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Updated the standard parsetree for unboxed literals
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Support unboxed literals
Testing story not complete yet; want advice.
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Make parsetree changes separated
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Checkpoint
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Still not quite working, but done for tonight
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Vendor in jane-syntax into standard
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Separate out Jane Street stuff more
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* More tests, and a bugfix
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
---------
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
Add instructions for how to update ocamlformat (#26)
* Write instructions
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Chris's comments
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Document testing
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Typo
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Follow on from Chris's edits
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Further advice about dune errors
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Comment about `make test`
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
---------
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
Co-authored-by: Chris Casinghino <ccasinghino@janestreet.com>
Support float# (#27)
* Update parser-standard for float#
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Add support for float#
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Add tests
But this doesn't work. I'm very confused about how to test here.
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Some more type tests
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Test float#
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Forward-proof printing of float#
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Update parser-standard/language_extension
This also removes the differences between ocamlformat's
language_extension and janestreet's. Instead, we now just
call enable_maximal. Seems simpler.
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
---------
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
Check formatting in CI (#29)
* Fix formatting
This required removing some of the "Jane Street extension" markers, as
ocamlformat was moving them to unhelpful places.
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
* Check formatting in CI
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
---------
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
Capture a few notes from our working conventions (#30)
* Capture a few notes from Working Conventions
Merging this will allow me to stub out the section in Working
Conventions.
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
* Chris's suggestions
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
---------
Signed-off-by: Richard Eisenberg <reisenberg@janestreet.com>
Add a mode for erasing erasable jane-syntax (#32)
* Add (untested) support for erasing erasable Jane syntax
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
* Make normalization ignore erasable jane syntax if requested
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
* Add failing test of erasure
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
* Update parser-standard to antalsz/flambda-backend 5b23940cfccd3047
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
* Update parser-standard and location to antalsz/flambda-backend c056f5f2b0082711b596337f270a8eccf06d0b06
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
* Update `--erase-jane-syntax` documentation per review
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
* Documentation for `Erase_jane_syntax`
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
* Only remove erasable `Jane_syntax` attributes from the old AST
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
* Fix formatting
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
---------
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
Add support for float64 layout (#35)
Signed-off-by: Chris Casinghino <ccasinghino@janestreet.com>
Support explicit strengthening (#39)
* Support explicit strengthening
Support the syntax introduced in
oxcaml/oxcaml#1337.
Signed-off-by: Roman Leshchinskiy <rleshchinskiy@janestreet.com>
* Fix parens with attributes
Signed-off-by: Roman Leshchinskiy <rleshchinskiy@janestreet.com>
---------
Signed-off-by: Roman Leshchinskiy <rleshchinskiy@janestreet.com>
Co-authored-by: Roman Leshchinskiy <rleshchinskiy@janestreet.com>
Update build workflow to OCaml 4.14 (#33)
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
In an if-then with no else, the body should have parens if local_ (#40)
Signed-off-by: Chris Casinghino <ccasinghino@janestreet.com>
Add a mode for rewriting `[@Local]`, `[%local]`, `[@ocaml.global]`, etc. to their keyword forms (`local_`, `global_`, etc.) (#38)
* Add mode to rewrite `[@Local]`, etc., into `local_`
This is available under the option
`--rewrite-old-style-jane-street-local-annotations`
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
* Add tests of rewriting `[@Local]` and fix the normalization check
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
* Update `ocamlformat-help.txt`
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
* Remove export of function that was for debugging
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
---------
Signed-off-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
Fix bug caused by comments before local_ at the beginning of bindings. (#41)
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
2 participants
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.
The desugared syntax
let local_ _ = xis invalid.Same as #20, rebased on the
janebranch.