Support ppxlib.0.36.0 #448

patricoferris · 2025-06-20T11:40:09Z

This is a draft PR bump to bisect_ppx to the latest ppxlib.

I think this is realistically going to need #445 and I'll need to look into some of the failing tests (some are formatting/pretty-printing related I think).

kit-ty-kate · 2025-08-05T12:46:54Z

@patricoferris i believe this can be undrafted now

glondu · 2025-09-08T10:45:50Z

Any news?

Kakadu · 2025-09-13T13:54:06Z

@aantron ^^

NathanReb · 2025-09-18T12:28:49Z

I think this patch is causing #450. There's something fishy going on when instrumenting Pfunction_cases that can lead to it being converted to a Pfunction_body which triggers the compiler invariant in the linked issue.

I'll look into it and push a fix!

Once that's done I'm also happy to help getting a release in any way I can @aantron !

NathanReb · 2025-09-18T13:54:57Z

Opened a PR to @patricoferris' branch here: patricoferris#1

NathanReb · 2025-09-18T13:59:56Z

The test suite is broken at the moment so I tried to make the minimal set of changes required to make my new regression test work, for anybody willing to try it out, you can run just the one test with:

dune runtest test/instrument/function.t

to get a clean output that's not polluted by the other errors.

I'd also be happy to contribute a refresh of the test suite and to help with the project maintenance. Don't hesitate to reach out if you're looking for a bit of help on that front!

Signed-off-by: Nathan Rebours <nathan.rebours@ocamlpro.com>

patricoferris · 2025-09-18T19:37:29Z

Pulled your changes in @NathanReb -- thanks !

aantron · 2025-10-02T08:22:59Z

I took a look at all the background today and I'm going to continue with reviewing the code in the coming days.

aantron · 2025-10-02T08:25:24Z

Links for the future:

The OCaml RFC that caused this Syntactic function arity ocaml/RFCs#32
The OCaml PR Syntactic function arity ocaml/ocaml#12236, in particular ocaml/ocaml@1fdfc41 and ocaml/ocaml@4375486
Ppxlib PR trunk-support supports trunk ocaml-ppx/ppxlib#451

kit-ty-kate · 2025-10-10T16:56:49Z

FYI: OCaml 5.4.0 was released yesterday

aantron · 2025-10-12T16:18:22Z

The test suite presently works for master on 4.08-4.13 with ppxlib 0.35.0, including in CI. I just merged master into this PR, to have the CI test ppxlib's translation of what this PR does to older versions. Could you pull these commits down and comment on the failures?

dune build -p bisect_ppx
NO_COLOR=yes dune build -p bisect_ppx @runtest
File "test/instrument/control/function.t", line 1, characters 0-0:
/usr/bin/git --no-pager diff --no-index --color=always -u _build/default/test/instrument/control/function.t _build/default/test/instrument/control/function.t.corrected
diff --git a/_build/default/test/instrument/control/function.t b/_build/default/test/instrument/control/function.t.corrected
index bfb978d..a39693a 100644
--- a/_build/default/test/instrument/control/function.t
+++ b/_build/default/test/instrument/control/function.t.corrected
@@ -59,6 +59,7 @@ Or-pattern.
   > EOF
   let _ =
    fun ___bisect_matched_value___ ->
+    ___bisect_visit___ 2;
     match ___bisect_matched_value___ with
     | None | Some _ ->
         (match[@ocaml.warning "-4-8-9-11-26-27-28-33"]
File "test/instrument/control/newtype.t", line 1, characters 0-0:
/usr/bin/git --no-pager diff --no-index --color=always -u _build/default/test/instrument/control/newtype.t _build/default/test/instrument/control/newtype.t.corrected
diff --git a/_build/default/test/instrument/control/newtype.t b/_build/default/test/instrument/control/newtype.t.corrected
index 630a122..12927d2 100644
--- a/_build/default/test/instrument/control/newtype.t
+++ b/_build/default/test/instrument/control/newtype.t.corrected
@@ -28,6 +28,6 @@ Subexpression in tail position iff whole expression is in tail position.
   let _ = fun (type _t) -> ___bisect_post_visit___ 0 (print_endline "foo")
   
   let _ =
-   fun () ->
+   fun () (type _t) ->
     ___bisect_visit___ 1;
-    fun (type _t) -> print_endline "foo"
+    print_endline "foo"
File "test/instrument/pattern/exception.t", line 1, characters 0-0:
/usr/bin/git --no-pager diff --no-index --color=always -u _build/default/test/instrument/pattern/exception.t _build/default/test/instrument/pattern/exception.t.corrected
diff --git a/_build/default/test/instrument/pattern/exception.t b/_build/default/test/instrument/pattern/exception.t.corrected
index 5c660dc..e383838 100644
--- a/_build/default/test/instrument/pattern/exception.t
+++ b/_build/default/test/instrument/pattern/exception.t.corrected
@@ -38,10 +38,12 @@ patterns.
   >   | _ -> print_endline "baz"
   > EOF
   let _ =
-    let ___bisect_case_0___ x () =
+    let ___bisect_case_0___ x =
+     fun [@ppxlib.migration.stop_taking] () ->
       ignore x;
       ___bisect_post_visit___ 0 (print_endline "foo")
-    and ___bisect_case_1___ y () =
+    and ___bisect_case_1___ y =
+     fun [@ppxlib.migration.stop_taking] () ->
       ignore y;
       ___bisect_post_visit___ 1 (print_endline "bar")
     in
File "test/instrument/recent/gadt.t", line 1, characters 0-0:
/usr/bin/git --no-pager diff --no-index --color=always -u _build/default/test/instrument/recent/gadt.t _build/default/test/instrument/recent/gadt.t.corrected
diff --git a/_build/default/test/instrument/recent/gadt.t b/_build/default/test/instrument/recent/gadt.t.corrected
index d65beb4..789f6d4 100644
--- a/_build/default/test/instrument/recent/gadt.t
+++ b/_build/default/test/instrument/recent/gadt.t.corrected
@@ -39,6 +39,7 @@ With function.
   
   let f : type a. a t -> unit =
    fun ___bisect_matched_value___ ->
+    ___bisect_visit___ 2;
     match ___bisect_matched_value___ with
     | A | B ->
         (match[@ocaml.warning "-4-8-9-11-26-27-28-33"]
File "test/instrument/value.t", line 1, characters 0-0:
/usr/bin/git --no-pager diff --no-index --color=always -u _build/default/test/instrument/value.t _build/default/test/instrument/value.t.corrected
diff --git a/_build/default/test/instrument/value.t b/_build/default/test/instrument/value.t.corrected
index 2d9e0fa..58562b7 100644
--- a/_build/default/test/instrument/value.t
+++ b/_build/default/test/instrument/value.t.corrected
@@ -194,7 +194,11 @@ No instrumentation is inserted into expressions that are (syntactic) values.
   
   let _ = (___bisect_post_visit___ 1 (f ()) :> [ `Foo | `Bar ])
   
-  let _ = fun () -> (f () :> [ `Foo | `Bar ])
+  let _ =
+   fun () ->
+    (___bisect_visit___ 2;
+     f ()
+      :> [ `Foo | `Bar ])
 
 
   $ bash test.sh <<'EOF'
make: *** [Makefile:13: test] Error 1

aantron · 2025-10-12T16:21:07Z

This difference

diff --git a/_build/default/test/instrument/value.t b/_build/default/test/instrument/value.t.corrected
index 2d9e0fa..08f2932 100644
--- a/_build/default/test/instrument/value.t
+++ b/_build/default/test/instrument/value.t.corrected
@@ -194,7 +194,10 @@ No instrumentation is inserted into expressions that are (syntactic) values.
   
   let _ = (___bisect_post_visit___ 1 (f ()) :> [ `Foo | `Bar ])
   
-  let _ = fun () -> (f () :> [ `Foo | `Bar ])
+  let _ =
+   fun () ->
+    ___bisect_visit___ 2;
+    (f () :> [ `Foo | `Bar ])
 
 
   $ bash test.sh <<'EOF'

appears in the 4.14 and 5.0.0 run in master, without this PR, so probably not caused by it. IIRC I have a commit in a branch locally that suppresses this, so I'll push that in later. Don't spend time addressing this one :)

aantron · 2025-10-12T16:31:02Z

I'd also be happy to contribute a refresh of the test suite and to help with the project maintenance. Don't hesitate to reach out if you're looking for a bit of help on that front!

Would be very glad to merge updates to the test suite, and help with project maintenance. What would you like to do? Do you need write access to the repo?

patricoferris · 2025-10-17T01:49:42Z

Thanks @aantron.

-    let ___bisect_case_0___ x () =
+    let ___bisect_case_0___ x =
+     fun [@ppxlib.migration.stop_taking] () ->
       ignore x;
       ___bisect_post_visit___ 0 (print_endline "foo")
-    and ___bisect_case_1___ y () =
+    and ___bisect_case_1___ y =
+     fun [@ppxlib.migration.stop_taking] () ->
       ignore y;
       ___bisect_post_visit___ 1 (print_endline "bar")
     in

these failures are a known bug that is fixed upstream, see ocaml-ppx/ppxlib#604.

The others look like some interaction with the new function layout in the parsetree (which I may have not implemented correctly).

diff --git a/_build/default/test/instrument/control/newtype.t b/_build/default/test/instrument/control/newtype.t.corrected
index 630a122..12927d2 100644
--- a/_build/default/test/instrument/control/newtype.t
+++ b/_build/default/test/instrument/control/newtype.t.corrected
@@ -28,6 +28,6 @@ Subexpression in tail position iff whole expression is in tail position.
  let _ = fun (type _t) -> ___bisect_post_visit___ 0 (print_endline "foo")
  
  let _ =
-   fun () ->
+   fun () (type _t) ->
    ___bisect_visit___ 1;
-    fun (type _t) -> print_endline "foo"
+    print_endline "foo"

This looks like we go all the way to the end of the functions params before inserting the bisect code. Before ppxlib.0.36 this would have been Pexp_fun (x, Pexp_newtype (t ... )) but now it is Pexp_function([arg x; newtype t]...) so that might be causing this.

diff --git a/_build/default/test/instrument/control/function.t b/_build/default/test/instrument/control/function.t.corrected
index bfb978d..a39693a 100644
--- a/_build/default/test/instrument/control/function.t
+++ b/_build/default/test/instrument/control/function.t.corrected
@@ -59,6 +59,7 @@ Or-pattern.
   > EOF
   let _ =
    fun ___bisect_matched_value___ ->
+    ___bisect_visit___ 2;
     match ___bisect_matched_value___ with
     | None | Some _ ->
         (match[@ocaml.warning "-4-8-9-11-26-27-28-33"]

This looks similar, the representation before would have been Pexp_fun(__bisect..., Pexp_function(cases) whereas now it is Pexp_function([__bisect...], Pfunction_cases(cases))

aantron · 2025-10-17T02:24:40Z

```diff
-    let ___bisect_case_0___ x () =
+    let ___bisect_case_0___ x =
+     fun [@ppxlib.migration.stop_taking] () ->
       ignore x;
       ___bisect_post_visit___ 0 (print_endline "foo")
-    and ___bisect_case_1___ y () =
+    and ___bisect_case_1___ y =
+     fun [@ppxlib.migration.stop_taking] () ->
       ignore y;
       ___bisect_post_visit___ 1 (print_endline "bar")
     in

these failures are a known bug that is fixed upstream, see ocaml-ppx/ppxlib#604.

That looks like the wrong issue link but I was able to find ocaml-ppx/ppxlib#597 from your hint, thank you. Leaving this comment here so that we have a link to that issue for future reading :)

patricoferris · 2025-10-21T09:07:18Z

Thanks @aantron for catching that!

I have pushed a commit ebb3526 that fixes the control/function.t and control/newtype.t. The fix was to match more closely how the original instrumentation caught let f = function A -> ... sooner as there was a dedicated parsetree node for this. Now it is hidden within a Pexp_function with no parameters.

mbarbin · 2025-10-25T13:16:55Z

Thank you all for the work on this PR!

In the meanwhile, I have found great value in using https://github.com/kit-ty-kate/opam-alpha-repository Thanks!

@kit-ty-kate may I request that you add a new preview package for bisect_ppx which would include the latest fix for #450 this would allow continue the 5.4 testing while things settles. I am not completely sure it's possible to find a candidate version though (just in case this is possible to find such a revision to build from). Thank you!

AllanBlanchard · 2025-11-02T11:35:48Z

Apparently, the fix is not robust enough. It still does not work with the following example:

[@@@warning "-37"]
type 'a bt = (int * 'a) list
type x

type ('a, 'b) t =
  | B  : 'a bt -> ('a, 'b) t
  | C : ('a, 'b) t
  | D : ('a, 'b) t

let _action (f  : int -> 'a -> 'b -> 'b) (acc : 'b) : ('a, x) t -> 'b =
  function
  | C | D -> acc
  | B l -> List.fold_left (fun acc (cs,x) -> f cs x acc) acc l
[@@@warning "+37"]

Without coverage, it compiles, with bisect we get the following error:

File "file.ml", line 20, characters 13-16:
20 |   | C | D -> acc
                  ^^^
Error: The value acc has type 'b but an expression was expected of type
         ('a, x) t -> 'b
       The type variable 'b occurs inside ('a, x) t -> 'b

This is a reduced example taken from Frama-C. I can point out the actual file if it is useful.

NathanReb · 2025-11-04T16:12:57Z

@AllanBlanchard thanks for the minimization, I confirm I can reproduce the error with it and am working on a fix!

Signed-off-by: Nathan Rebours <nathan.rebours@ocamlpro.com>

NathanReb · 2025-11-04T17:00:57Z

Just opened patricoferris#2 which should fix the error @AllanBlanchard !

kit-ty-kate · 2025-11-04T18:18:25Z

@mbarbin done: kit-ty-kate/opam-alpha-repository@c9bdcd1

mbarbin · 2025-11-04T21:11:29Z

@mbarbin done: kit-ty-kate/opam-alpha-repository@c9bdcd1

@kit-ty-kate Very nice. I re-run a few CIs to pick up this new pkg and I can confirm this fixed the bisect_ppx related issues with 5.4 I had seen in these PRs. Continuing with the 5.4 transition here. Thanks a lot, and thank you too to maintainers and contributors for the work!

NathanReb · 2025-11-05T08:52:04Z

@aantron what are the remaining blockers on this PR?

patricoferris · 2025-11-05T09:42:36Z

Thanks @NathanReb! Merged into this branch.

AllanBlanchard · 2025-11-05T12:40:00Z

Just opened patricoferris#2 which should fix the error @AllanBlanchard !

Tested today, we can compile and test Frama-C with this fix. There are a few micro-differences on some counters but it changes global coverage by less than 0.05%.

Thanks a lot for that!

aantron · 2025-11-05T12:52:30Z

Thanks everyone! I will review it in its current state shortly, and if there are no any* obvious problems I'll try to fix them myself, then do a merge and release.

kit-ty-kate · 2025-12-19T12:15:21Z

Gentile ping

aantron · 2025-12-22T12:42:21Z

For transparency for what's going on on my end, right now I am handing over Dream to a team of maintainers (https://discuss.ocaml.org/t/dream-looking-for-maintainers-to-take-ownership/17607). I hope to complete the main part of that within a week (but subject to it being the holiday season for most people, though not me). After that I will either have enough attention resources to personally work on all the rest of my projects, including Bisect_ppx, since all of them together need much less attention than Dream, or I will start handing them over, too, probably starting with Bisect_ppx as the next-most significant one after Dream for the OCaml community. Thanks for still having patience, which I ended up testing considerably, though not intentionally!

patricoferris · 2026-01-06T16:14:02Z

Thanks for the update @aantron, I think the https://github.com/ocaml-ppx organisation could be a home for bisect_ppx (if you don't already have one in mind e.g. https://github.com/ocaml-community). Let me know if there's anything I can do on my end :)

Support ppxlib.0.36.0

f35fdf4

smorimoto mentioned this pull request Jul 15, 2025

Upgrade GitHub Actions workflow with latest runners and actions #449

Closed

patricoferris marked this pull request as ready for review August 6, 2025 11:10

Nathan Rebours added 2 commits September 18, 2025 20:35

Add regression test for function_cases instrumentation bug

07bfcee

Signed-off-by: Nathan Rebours <nathan.rebours@ocamlpro.com>

Fix instrumentation of [function _ ->] expressions

4f0cb2a

Signed-off-by: Nathan Rebours <nathan.rebours@ocamlpro.com>

redianthus mentioned this pull request Sep 18, 2025

use latest patch for bisect_ppx NixOS/nixpkgs#444109

Merged

aantron added 2 commits October 12, 2025 17:05

opam: update ppxlib constraint

3134448

Merge branch 'master' into 5.2

29a4b11

aantron mentioned this pull request Oct 19, 2025

Build fails from the master: Error: The constructor Pexp_function expects 3 argument(s), but is applied here to 1 argument(s) #451

Closed

Catch function cases earlier in instrumentation

ebb3526

patricoferris force-pushed the 5.2 branch from 1c610ee to ebb3526 Compare October 21, 2025 09:06

ajbt200128 mentioned this pull request Nov 3, 2025

ppxlib >= 0.36.0 compatibility #452

Closed

Nathan Rebours added 2 commits November 4, 2025 17:24

Add regression test for @AllanBlanchard instrumentation bug

a0ff8cf

Signed-off-by: Nathan Rebours <nathan.rebours@ocamlpro.com>

Fix instrumentation of Pexp_function

a64e135

Signed-off-by: Nathan Rebours <nathan.rebours@ocamlpro.com>

NathanReb mentioned this pull request Nov 4, 2025

Fix 5 2 port bis patricoferris/bisect_ppx#2

Merged

Additional fixes

7061d64

aantron mentioned this pull request Nov 7, 2025

cmdliner 2.0.0 support #444

Closed

psafont mentioned this pull request Dec 11, 2025

packages: replace ocaml 5.3 with ocaml 5.4 xapi-project/xs-opam#749

Closed

edwintorok mentioned this pull request Dec 17, 2025

coverage: remove dependency on bisect_ppx xapi-project/xen-api#6807

Merged

Uh oh!

Support ppxlib.0.36.0 #448

Are you sure you want to change the base?

Support ppxlib.0.36.0 #448

Uh oh!

Conversation

patricoferris commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kit-ty-kate commented Aug 5, 2025

Uh oh!

glondu commented Sep 8, 2025

Uh oh!

Kakadu commented Sep 13, 2025

Uh oh!

NathanReb commented Sep 18, 2025

Uh oh!

NathanReb commented Sep 18, 2025

Uh oh!

NathanReb commented Sep 18, 2025

Uh oh!

patricoferris commented Sep 18, 2025

Uh oh!

aantron commented Oct 2, 2025

Uh oh!

aantron commented Oct 2, 2025

Uh oh!

kit-ty-kate commented Oct 10, 2025

Uh oh!

aantron commented Oct 12, 2025

Uh oh!

aantron commented Oct 12, 2025

Uh oh!

aantron commented Oct 12, 2025

Uh oh!

patricoferris commented Oct 17, 2025

Uh oh!

aantron commented Oct 17, 2025

Uh oh!

patricoferris commented Oct 21, 2025

Uh oh!

mbarbin commented Oct 25, 2025

Uh oh!

AllanBlanchard commented Nov 2, 2025

Uh oh!

NathanReb commented Nov 4, 2025

Uh oh!

NathanReb commented Nov 4, 2025

Uh oh!

kit-ty-kate commented Nov 4, 2025

Uh oh!

mbarbin commented Nov 4, 2025

Uh oh!

NathanReb commented Nov 5, 2025

Uh oh!

patricoferris commented Nov 5, 2025

Uh oh!

AllanBlanchard commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aantron commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kit-ty-kate commented Dec 19, 2025

Uh oh!

aantron commented Dec 22, 2025

Uh oh!

patricoferris commented Jan 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

patricoferris commented Jun 20, 2025 •

edited

Loading

AllanBlanchard commented Nov 5, 2025 •

edited

Loading

aantron commented Nov 5, 2025 •

edited

Loading