Skip to content

more compact formatting for while-loops with no body - #2812

Merged
Julow merged 1 commit into
ocaml-ppx:mainfrom
v-gb:push-wmwplxprntzm
Jul 30, 2026
Merged

more compact formatting for while-loops with no body#2812
Julow merged 1 commit into
ocaml-ppx:mainfrom
v-gb:push-wmwplxprntzm

Conversation

@v-gb

@v-gb v-gb commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Concretely, this is what changes:

 while
   loooong
     condition
-do
-  ()
-done
+do () done

A real example where I wished for a more compact formatting:

ocaml/ocaml-re@59198dd#diff-a91a869b078e8995843c8b377da588505468c962166d62d7029893c27be32833R70-R80

To explain a bit: while-loops are fairly inexpressive in ocaml due to the absence of break. In a decent number of cases, it's nicer to give up on the separation between condition and body, by moving the body into the condition. In that case, it's a bit silly to waste 3 lines on the empty body.

For instance, if you wanted to write a loop where the first iteration always runs, like a do-while in C, the obvious thing to do (without auxiliary functions) is:

let first = ref true in
while !first || condition; do first := false; body done

but the simpler version is:

while body; condition do () done

Or iterating through a stack:

while not (Stack.is_empty s) do f (Stack.pop_exn s) done

can be done without double-checking that the stack is empty:

while
  match Stack.pop s with
  | None -> false
  | Some elt -> f elt; true
do () done

@v-gb
v-gb force-pushed the push-wmwplxprntzm branch from 27f6344 to 5f09a94 Compare June 29, 2026 13:11
@EmileTrotignon

Copy link
Copy Markdown
Collaborator

Not sure why the CI is failing, but I agree with the goal. The tests and implementation are good.

@v-gb
v-gb force-pushed the push-wmwplxprntzm branch from 5f09a94 to b7cca64 Compare June 29, 2026 16:33
@v-gb

v-gb commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Thanks! Right, the failures are unrelated to the change, though no idea either why they are occurring.

I added the PR number to the CHANGES file (it's going to incidentally retrigger builds, but I expect they will fail again).

@Julow Julow left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good! The CI should run if you merge the main branch.

Concretely, this is what changes:

```diff
 while
   loooong
     condition
-do
-  ()
-done
+do () done
```

A real example where I wished for a more compact formatting:

ocaml/ocaml-re@59198dd#diff-a91a869b078e8995843c8b377da588505468c962166d62d7029893c27be32833R70-R80

To explain a bit: while-loops are fairly inexpressive in ocaml due to the absence of
`break`.  In a decent number of cases, it's nicer to give up on the separation between
condition and body, by moving the body into the condition. In that case, it's a bit
silly to waste 3 lines on the empty body.

For instance, if you wanted to write a loop where the first iteration always runs, like
a do-while in C, the obvious thing to do (without auxiliary functions) is:

```ocaml
let first = ref true in
while !first || condition; do first := false; body done
```

but the simpler version is:

```ocaml
while body; condition do () done
```

Or iterating through a stack:

```ocaml
while not (Stack.is_empty s) do f (Stack.pop_exn s) done
```

can be done without double-checking that the stack is empty:

```ocaml
while
  match Stack.pop s with
  | None -> false
  | Some elt -> f elt; true
do () done
```
@v-gb
v-gb force-pushed the push-wmwplxprntzm branch from b7cca64 to d74010b Compare July 29, 2026 21:23
@v-gb

v-gb commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

After rebase, there's still some kind of unrelated failure.

@Julow

Julow commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Thanks! The remaining failures are not related, I'll fix them separately.

@Julow
Julow merged commit 20c4543 into ocaml-ppx:main Jul 30, 2026
8 of 12 checks passed
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.

3 participants