Backport 5.1.0-beta1 standard parser changes #2412

Julow · 2023-07-25T13:27:34Z

Changes that are backported:

New Pmod_apply_unit module type constructor. We had a similar concept in parse-extended, so this is just a rename.
New value_constraint field in value bindings.
This makes the parser less permissive about bugs in the formatting of
type annotation on let-bindings.
Some changes in Location and Ast_mapper are not necessary but are
backported to reduce the diff.
String assignment operator has been removed upstream but remains
supported by commenting out the change.
Dead code in parser-extended have been removed in the past. I reverted that
and commented-out the code instead. This makes the code more similar with
parser-standard and helps recognize new code.
Ast rules have changed as some constructions are no longer special.

- New `Pmod_apply_unit` module type constructor. - New `value_constraint` field in value bindings. This makes the parser less permissive about bugs in the formatting of type annotation on let-bindings. - Some changes in `Location` and `Ast_mapper` are not necessary but are backported to reduce the diff. - String assignment operator has been removed upstream but remains supported by commenting out the change.

The extended AST already had a similar constructor. This is just a rename.

Add commented-out unused code instead of removing it. This helps backporting new changes by making the code looks more similar to the upstream parser and helps to recognize added code.

We already had a similar AST node but the `value_constraint` field is new and helps removing complex code in Sugar. The `pvb_is_pun` field is kept from the previous extended representation.

instead of the extended parser (the default).

Julow · 2023-07-25T13:31:49Z

test/passing/tests/js_source.ml.ref

 let foo : type a' b'. a' -> b' = fun a -> assert false
 let foo : type t'. t' = fun (type t') : t' -> assert false
-let foo : type t. t = assert false
+let foo : 't. 't = fun (type t) : t -> assert false


This rewrite is no longer done.

Julow · 2023-07-25T13:32:28Z

test/passing/tests/let_binding-in_indent.ml.ref

  let x : int = x in
  let (_ : int) = x in
-  let (_ : int) = x in
+  let _ : int = x in


Parentheses around let-binding patterns follow the original source. These are two different AST now.
Shouldn't change existing code.

Julow · 2023-07-25T13:34:04Z

test/passing/tests/polytypes-janestreet.ml.ref

+let t2
+  : 'a 'b.
+  'a t________________________________ -> 'b t_______________________________________


I do not consider this a regression as it's the formatting used in slightly different cases.

Julow · 2023-07-26T13:15:20Z

test_branch shows some regressions but I'm tempted to accept them as improvements.

Some patterns are no longer recognized as Ppat_constraint but rather a let-binding with annotation. Showing the inconsistency between the two code paths.

index f2e9236473..27701537f5 100644
--- a/infer/src/pulse/PulseCallOperations.ml
+++ b/infer/src/pulse/PulseCallOperations.ml
@@ -233,8 +233,8 @@ let apply_callee tenv ({ PathContext.timestamp } as path) ~caller_proc_desc
         let caller_return_var : Pvar.t =
           Procdesc.get_ret_var caller_proc_desc
         in
-        let (astate, caller_return_val_hist)
-              : AbductiveDomain.t * (AbstractValue.t * ValueHistory.t) =
+        let (astate, caller_return_val_hist) :
+            AbductiveDomain.t * (AbstractValue.t * ValueHistory.t) =
           PulseOperations.eval_var path call_loc caller_return_var astate
         in
         let+ (astate : AbductiveDomain.t) =

Some patterns were parenthesed because the expression was a Pexp_constraint. This bug is gone:

index 2a34ec8b1..faaf464be 100644
--- a/testsuite/tests/typing-poly/poly.ml
+++ b/testsuite/tests/typing-poly/poly.ml
@@ -1888,11 +1888,11 @@ module Polux :
 
 (* PR#5560 *)
 
-let (a, b) = (raise Exit : int * int)
+let a, b = (raise Exit : int * int)
 
 type t = { foo : int }
 
-let ({ foo }) = (raise Exit : t)
+let { foo } = (raise Exit : t)
 
 type s = A of int

This might also be an inconsistency between two code paths, though I can't pin it down yet.

index 395387a93..8b7b830aa 100644
--- a/src/memo/memo.ml
+++ b/src/memo/memo.ml
@@ -1051,9 +1051,9 @@ module Exec : sig
   val exec_dep_node : ('i, 'o) Dep_node.t -> 'o Fiber.t
 end = struct
   let rec restore_from_cache :
-            'o.
-            cached_value:'o Cached_value.t ->
-            'o Cached_value.t Cache_lookup.Result.t Fiber.t =
+        'o.
+        cached_value:'o Cached_value.t ->
+        'o Cached_value.t Cache_lookup.Result.t Fiber.t =
    fun ~cached_value ->
     match cached_value.value with
     | Cancelled _dependency_cycle ->

Julow · 2023-07-26T13:18:26Z

CHANGES.md

+
+### New features
+
+- Compatible with OCaml 5.1.0-beta1 (#2412, @Julow)


The regressions are small in size and affect rare code. I would not mark this as breaking.

For consistency with the new 'value_binding'.

test/cli/debug.t

@Julow

….26.1) CHANGES: ### Changed - Compatible with OCaml 5.1.0 (ocaml-ppx/ocamlformat#2412, @Julow) The syntax of let-bindings changed sligthly in this version. - Improved ocp-indent compatibility (ocaml-ppx/ocamlformat#2428, @Julow) - \* Removed extra break in constructor declaration with comment (ocaml-ppx/ocamlformat#2429, @Julow) - \* De-indent the `object` keyword in class types (ocaml-ppx/ocamlformat#2425, @Julow) - \* Consistent formatting of arrows in class types (ocaml-ppx/ocamlformat#2422, @Julow) ### Fixed - Fix dropped attributes on a begin-end in a match case (ocaml-ppx/ocamlformat#2421, @Julow) - Fix dropped attributes on begin-end in an if-then-else branch (ocaml-ppx/ocamlformat#2436, @gpetiot) - Fix non-stabilizing comments before a functor type argument (ocaml-ppx/ocamlformat#2420, @Julow) - Fix crash caused by module types with nested `with module` (ocaml-ppx/ocamlformat#2419, @Julow) - Fix ';;' formatting between doc-comments and toplevel directives (ocaml-ppx/ocamlformat#2432, @gpetiot)

@Julow

….26.1) CHANGES: ### Changed - Compatible with OCaml 5.1.0 (ocaml-ppx/ocamlformat#2412, @Julow) The syntax of let-bindings changed sligthly in this version. - Improved ocp-indent compatibility (ocaml-ppx/ocamlformat#2428, @Julow) - \* Removed extra break in constructor declaration with comment (ocaml-ppx/ocamlformat#2429, @Julow) - \* De-indent the `object` keyword in class types (ocaml-ppx/ocamlformat#2425, @Julow) - \* Consistent formatting of arrows in class types (ocaml-ppx/ocamlformat#2422, @Julow) ### Fixed - Fix dropped attributes on a begin-end in a match case (ocaml-ppx/ocamlformat#2421, @Julow) - Fix dropped attributes on begin-end in an if-then-else branch (ocaml-ppx/ocamlformat#2436, @gpetiot) - Fix non-stabilizing comments before a functor type argument (ocaml-ppx/ocamlformat#2420, @Julow) - Fix crash caused by module types with nested `with module` (ocaml-ppx/ocamlformat#2419, @Julow) - Fix ';;' formatting between doc-comments and toplevel directives (ocaml-ppx/ocamlformat#2432, @gpetiot)

Julow added 10 commits July 24, 2023 12:46

Backport Pmod_apply_unit to extended AST

3b853fc

The extended AST already had a similar constructor. This is just a rename.

Backport harmless changes to parser-extended

4f6a5ff

Add commented-out unused code instead of removing it. This helps backporting new changes by making the code looks more similar to the upstream parser and helps to recognize added code.

WIP: Backport value_binding change to parser-extended

8476f0f

We already had a similar AST node but the `value_constraint` field is new and helps removing complex code in Sugar. The `pvb_is_pun` field is kept from the previous extended representation.

tools/printast: Option for using standard parser

7400f9b

instead of the extended parser (the default).

Merge branch 'printast-std' into backport-510beta1

d7188d7

Fix desugaring

f3126e4

Promote debug test

c7439d3

Promote poly type regression

0a3f8c0

Fix poly type regression for default profile

0453882

Julow commented Jul 25, 2023

View reviewed changes

Julow added 3 commits July 25, 2023 15:36

Update CHANGES

3adbf9b

Fix regressions in let bindings

8209d47

Fix regressions in letops

35fb301

Julow commented Jul 26, 2023

View reviewed changes

parser-extended: Rename 'let_bindings' to 'value_bindings'

60aaf2d

For consistency with the new 'value_binding'.

gpetiot reviewed Sep 2, 2023

View reviewed changes

test/cli/debug.t Outdated Show resolved Hide resolved

Printast: Print missing pvb_loc

e5391f6

Julow mentioned this pull request Sep 5, 2023

Backport dead code in vendored parsers #2424

Merged

Merge branch 'main' into backport-510beta1

d00fbe3

Julow merged commit f623b63 into ocaml-ppx:main Sep 5, 2023

Julow mentioned this pull request Sep 15, 2023

[new release] ocamlformat-lib, ocamlformat and ocamlformat-rpc-lib (0.26.1) ocaml/opam-repository#24448

Merged

Julow mentioned this pull request Oct 26, 2023

Remove unnecessary Sugar.polynewtype #2467

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Backport 5.1.0-beta1 standard parser changes #2412

Backport 5.1.0-beta1 standard parser changes #2412

Uh oh!

Julow commented Jul 25, 2023

Uh oh!

Julow Jul 25, 2023

Uh oh!

Julow Jul 25, 2023

Uh oh!

Julow Jul 25, 2023

Uh oh!

Julow commented Jul 26, 2023

Uh oh!

Julow Jul 26, 2023

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		### New features

		- Compatible with OCaml 5.1.0-beta1 (#2412, @Julow)

Backport 5.1.0-beta1 standard parser changes #2412

Backport 5.1.0-beta1 standard parser changes #2412

Uh oh!

Conversation

Julow commented Jul 25, 2023

Uh oh!

Julow Jul 25, 2023

Choose a reason for hiding this comment

Uh oh!

Julow Jul 25, 2023

Choose a reason for hiding this comment

Uh oh!

Julow Jul 25, 2023

Choose a reason for hiding this comment

Uh oh!

Julow commented Jul 26, 2023

Uh oh!

Julow Jul 26, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants