5.0 support

[dra27]

Alternate to #129 with DCO signed commits as the interim 5.0-only commit is removed - and on the v0.15 branch

[tov]

Hey, we have just one question remaining before merging this.

[dra27]

Base builds with 4.10 on my machine?

[tov]

Base builds with 4.10 on my machine?

It's failing in CI with an error about not understanding FLAMBDA-style inline annotations.

[tov]

You can see it here: https://github.com/tov/base/runs/7525879032?check_suite_focus=true

[kit-ty-kate]

These are only warnings. Could you use dune build --release instead? (or simply opam)

[tov]

These are only warnings. Could you use dune build --release instead? (or simply opam)

They are warnings, but in dev mode the warnings are fatal. Shouldn't people be able to build the library in dev mode?

[dra27]

I get that inlining warning with all released versions of OCaml (4.10-5.0 incl.) so I'd just ignored it (I was assuming that it was an flambda 2 optimisation, so working inside JS). I would say it's reasonable to have the dev profile build without error in the latest version, but it's not strictly necessary for it to build in dev mode for older compilers. In particular, ecosystem users will install base via opam and the package builds in release mode then.

As an aside, I'm suspicious at the 0s taken to build base in https://github.com/tov/base/runs/7525878932?check_suite_focus=true ?

[tov]

I think there are two different issues:

1. Warning 55 on all relevant OCaml versions (4.10 and later) that round_nearest cannot be inlined in src/float.ml:

   File "src/float.ml", line 492, characters 10-52:
   492 |   let t = (round_nearest [@ocaml.inlined always]) t0 in
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   Error (warning 55 [inlining-impossible]): Cannot inline: Function information unavailable
   

2. Warning 47 on OCaml < 4.11 that [@inlined hint] has an illegal payload at src/monad.ml:

   File "src/monad.ml", line 200, characters 32-39:
   200 |   let[@inline] bind a ~f = (f [@inlined hint]) a
                                         ^^^^^^^
   Error (warning 47): illegal payload for attribute 'inlined'.
   It must be either 'never', 'always' or empty
   

Both are fixed by building with the release profile.

Not understanding the release process (sorry), I merged a PR #137 into master that fixes 1 but not 2. Clearly 2 is not a reason to drop support for 4.10, nor do we need to fix it. But I do think we should fix 1.

[tov]

As an aside, I'm suspicious at the 0s taken to build base in https://github.com/tov/base/runs/7525878932?check_suite_focus=true ?

I see 1s to build and 0s to test. My machine agrees on 0s to test. I suspect the 1s to build is because the workflow is retaining the dune cache between runs.

[tov]

I've confirmed (with dune --verbose) that the fast builds are because of caching.

And I've created a draft PR #138 in which CI builds with --profile=release on specified OCaml versions.

[dra27]

Ah, I see what's happening - the failure building with 4.10 is on the master branch, which includes some unreleased v0.16 stuff... the change in src/monad.ml isn't on the v0.15 branch (which this PR is), which is why I was only seeing the warning in src/float.ml.

Do you want to push the commit from #137 to this branch so it gets merged to the v0.15 branch as well?

[tov]

Do you want to push the commit from #137 to this branch so it gets merged to the v0.15 branch as well?

Sure, I was trying to be a bit less reckless than I'd been.

[dra27]

Do you want to push the commit from #137 to this branch so it gets merged to the v0.15 branch as well?

Sure, I was trying to be a bit less reckless than I'd been.

All good for me 🙂