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

Remove some calls to if_newline and break_unless_newline and fix break before closing brackets #1168

Merged
merged 4 commits into from
Nov 28, 2019

Conversation

gpetiot
Copy link
Collaborator

@gpetiot gpetiot commented Nov 27, 2019

Still on the way to remove calls to our custom Format functions.

Only b6208ad produces a diff.
No diff with janestreet profile.
Diff with conventional profile:

diff --git a/sledge/src/llair/frontend.ml b/sledge/src/llair/frontend.ml
index 6f0ebabff..115fa2834 100644
--- a/sledge/src/llair/frontend.ml
+++ b/sledge/src/llair/frontend.ml
@@ -932,7 +932,6 @@ let xlate_instr :
           | [
               "_ZnwmSt11align_val_t";
               (* operator new(unsigned long, std::align_val_t) *)
-              
             ] ->
               let reg = xlate_name x instr in
               let num = xlate_value x (Llvm.operand instr 0) in
@@ -942,12 +941,10 @@ let xlate_instr :
           | [
               "_ZdlPvSt11align_val_t";
               (* operator delete(void* ptr, std::align_val_t) *)
-              
             ]
           | [
               "_ZdlPvmSt11align_val_t";
               (* operator delete(void* ptr, unsigned long, std::align_val_t) *)
-              
             ]
           | [ "free" (* void free(void* ptr) *) ] ->
               let ptr = xlate_value x (Llvm.operand instr 0) in
@@ -1055,7 +1052,6 @@ let xlate_instr :
       | [
           "_ZnwmSt11align_val_t";
           (* operator new(unsigned long num, std::align_val_t) *)
-          
         ]
         when num_actuals > 0 ->
           let reg = xlate_name x instr in

@jberdine
Copy link
Collaborator

Diff with conventional profile:

And with the config specified for the code being tested on?

@gpetiot
Copy link
Collaborator Author

gpetiot commented Nov 27, 2019

Here is the diff with the original config:

diff --git a/sledge/src/llair/frontend.ml b/sledge/src/llair/frontend.ml
index 3ffdbd279..227860071 100644
--- a/sledge/src/llair/frontend.ml
+++ b/sledge/src/llair/frontend.ml
@@ -955,7 +955,7 @@ let xlate_instr :
             (* operator delete(void* ptr, std::align_val_t) *) ]
          |[ "_ZdlPvmSt11align_val_t"
             (* operator delete(void* ptr, unsigned long, std::align_val_t) *)
-           ]
+          ]
          |["free" (* void free(void* ptr) *)] ->
             let ptr = xlate_value x (Llvm.operand instr 0) in
             emit_inst (Llair.Inst.free ~ptr ~loc)

@jberdine
Copy link
Collaborator

Nice. It is good to see if_newline going away. It and the standard one both have quite strange behavior regarding the computation of the length of a box's contents, and hence whether or not it breaks.

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 ! All the diffs look like fixes.

@gpetiot gpetiot merged commit 1add6b2 into ocaml-ppx:master Nov 28, 2019
@gpetiot gpetiot deleted the remove-if_newline branch November 28, 2019 11:07
Julow added a commit to Julow/opam-repository that referenced this pull request Jan 28, 2020
CHANGES:

#### New features

  + Add an option `--margin-check` to emit a warning if the formatted output exceeds the margin (ocaml-ppx/ocamlformat#1110) (Guillaume Petiot)
  + Preserve comment indentation when `wrap-comments` is unset (ocaml-ppx/ocamlformat#1138, ocaml-ppx/ocamlformat#1159) (Jules Aguillon)
  + Improve error messages (ocaml-ppx/ocamlformat#1147) (Jules Aguillon)
  + Display standard output in the emacs plugin even when ocamlformat does not fail (ocaml-ppx/ocamlformat#1189) (Guillaume Petiot)

#### Removed

  + Remove `ocamlformat_reason` (ocaml-ppx/ocamlformat#254, ocaml-ppx/ocamlformat#1185) (Etienne Millon).
    This tool has never been released to opam, has no known users, and overlaps
    with what `refmt` can do.
  + Remove `ocamlformat-diff` (ocaml-ppx/ocamlformat#1205) (Guillaume Petiot)
    This tool has never been released to opam, has no known users, and overlaps
    with what `merge-fmt` can do.

#### Packaging

  + Work with base v0.13.0 (ocaml-ppx/ocamlformat#1163) (Jules Aguillon)

#### Bug fixes

  + Fix placement of comments just before a '|' (ocaml-ppx/ocamlformat#1203) (Jules Aguillon)
  + Fix build version detection when building in the absence of a git root (ocaml-ppx/ocamlformat#1198) (Anil Madhavapeddy)
  + Fix wrapping of or-patterns in presence of comments with `break-cases=fit` (ocaml-ppx/ocamlformat#1167) (Jules Aguillon)
    This also fixes an unstable comment bug in or-patterns
  + Fix an unstable comment bug in variant declarations (ocaml-ppx/ocamlformat#1108) (Jules Aguillon)
  + Fix: break multiline comments (ocaml-ppx/ocamlformat#1122) (Guillaume Petiot)
  + Fix: types on named arguments were wrapped incorrectly when preceding comments (ocaml-ppx/ocamlformat#1124) (Guillaume Petiot)
  + Fix the indentation produced by max-indent (ocaml-ppx/ocamlformat#1118) (Guillaume Petiot)
  + Fix break after Psig_include depending on presence of docstring (ocaml-ppx/ocamlformat#1125) (Guillaume Petiot)
  + Remove some calls to if_newline and break_unless_newline and fix break before closing brackets (ocaml-ppx/ocamlformat#1168) (Guillaume Petiot)
  + Fix unstable cmt in or-pattern (ocaml-ppx/ocamlformat#1173) (Guillaume Petiot)
  + Fix location of comment attached to the underscore of an open record (ocaml-ppx/ocamlformat#1208) (Guillaume Petiot)
  + Fix parentheses around optional module parameter (ocaml-ppx/ocamlformat#1212) (Christian Barcenas)
  + Fix grouping of horizontally aligned comments (ocaml-ppx/ocamlformat#1209) (Guillaume Petiot)
  + Fix dropped comments around module pack expressions (ocaml-ppx/ocamlformat#1214) (Jules Aguillon)
  + Fix regression of comment position in list patterns (ocaml-ppx/ocamlformat#1141) (Josh Berdine)
  + Fix: adjust definition of Location.is_single_line to reflect margin (ocaml-ppx/ocamlformat#1102) (Josh Berdine)

#### Documentation

  + Fix documentation of option `version-check` (ocaml-ppx/ocamlformat#1135) (Wilfred Hughes)
  + Fix hint when using `break-separators=after-and-docked` (ocaml-ppx/ocamlformat#1130) (Greta Yorsh)
bogdan2412 pushed a commit to bogdan2412/ocamlformat that referenced this pull request Mar 28, 2020
…k before closing brackets (ocaml-ppx#1168)

* remove if_newline in fmt_type_extension
* remove break_unless_newline in Cmts
* remove break_unless_newline in wrap_collec and fix break after collec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants