Skip to content
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

Do not use ghost locations for type constraints (ocaml#10555) #1779

Merged
merged 2 commits into from
Sep 2, 2021

Conversation

gpetiot
Copy link
Collaborator

@gpetiot gpetiot commented Aug 26, 2021

Applying ocaml/ocaml#10555

No diff with test_branch.sh, the changes in comments_in_record.ml are a fix.

@gpetiot gpetiot added the no changelog set this to bypass the CI check for changelog entries label Aug 26, 2021
@gpetiot gpetiot requested a review from Julow August 26, 2021 11:43
Comment on lines -608 to +619
| Some t, Some r ->
if type_first then field_space $ fmt_type t $ fmt "@ " $ fmt_rhs r
else
| Some t, Some r -> (
match constraint_loc with
| Some (`Constraint_lid loc) ->
field_space
$ Cmts.fmt_before c loc ~pro:(Fmt.break 1 0) ~epi:noop
$ fmt_type t $ fmt "@ " $ fmt_rhs r $ Cmts.fmt_after c loc
| Some (`Constraint_exp loc) ->
field_space $ fmt "=@;<1 2>"
$ hvbox 0
@@ Cmts.fmt c loc
(str "(" $ cbox 0 r $ str " " $ fmt_type ~parens:true t)
| None ->
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this change needed ?

Copy link
Collaborator Author

@gpetiot gpetiot Sep 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because ocaml/ocaml#10555 also changed the location span used for Ppat_constraint/Pexp_constraint, so the formatting had to be updated accordingly.
Otherwise here is the diff I had:

 let _ =
   { (* a *) a (* a' *)= (* b *) b (* b' *)
-  ; (* c' *) (* c *) c: (* d *) d (* d' *) = (* e *) e (* e' *)
+  ; (* c' *)
+    (* c *) c: (* d *) d (* d' *) = (* e *) e
+    (* e' *)
   ; (* f *) f (* f' *)
   ; (* j *)
     (* g *) g (* g' *)= ((* h *) h (* h' *) : (* i *) i (* i' *))
     (* j' *) }
 
 let { (* a *) a (* a' *)= (* b *) b (* b' *)
-    ; (* c' *) (* c *) c: (* d *) d (* d' *) = (* e *) e (* e' *)
+    ; (* c' *)
+      (* c *) c: (* d *) d (* d' *) = (* e *) e
+      (* e' *)
     ; (* f *) f (* f' *)
     ; (* j *)
       (* g *) g (* g' *)= ((* h *) h (* h' *) : (* i *) i (* i' *))

(the previous formatting was not 100% correct by the way)

Copy link
Collaborator

@Julow Julow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good :)

@gpetiot gpetiot merged commit e410241 into ocaml-ppx:main Sep 2, 2021
@gpetiot gpetiot deleted the ocaml#10555 branch September 2, 2021 14:07
@ocaml-ppx ocaml-ppx deleted a comment Sep 16, 2021
@ocaml-ppx ocaml-ppx deleted a comment Sep 16, 2021
@ocaml-ppx ocaml-ppx deleted a comment Oct 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog set this to bypass the CI check for changelog entries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants