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

4.08 support broken (List.concat_map) #195

Closed
bclement-ocp opened this issue Oct 16, 2023 · 10 comments
Closed

4.08 support broken (List.concat_map) #195

bclement-ocp opened this issue Oct 16, 2023 · 10 comments

Comments

@bclement-ocp
Copy link
Contributor

bclement-ocp commented Oct 16, 2023

#190 uses List.concat_map which is not available on OCaml 4.08 (introduced in 4.10).

Found by Alt-Ergo's CI here: https://github.com/OCamlPro/alt-ergo/actions/runs/6532270561/job/17735127849?pr=893

@Gbury
Copy link
Owner

Gbury commented Oct 16, 2023

Ah that's annoying indeed.

For the context: CI does not test 4.08 anymore because the model part depends on farith which introduces a dependency on ocaml >= 4.10. However, the parsing and typing part of the library actually still build with 4.08, even if we cannot actually test them (because most of the tests require the bin part which depends on the model part).

@Halbaroth
Copy link
Contributor

Halbaroth commented Oct 16, 2023

We may use stdcompat as we did in AE now? I can do it if you want :)

@Gbury
Copy link
Owner

Gbury commented Oct 16, 2023

We may use stdcompat as we did in AE now? I can do it if you want :)

It's not the problem: the fix in the code is trivial (and dolmen already re-implements quite a few usual functions for lists here and there). The point is rather how to setup things so that the CI can check that things build with 4.08 (that one is relatively easy) and if possible that it behaves correctly, i.e. that the tests pass (that one is trickier, as explained above).

@Halbaroth
Copy link
Contributor

You don't want to bump the minimal version to 4.10? If the model part of Dolmen isn't a separated library, it will be painful to test the remainder part on 4.08.

@Gbury
Copy link
Owner

Gbury commented Oct 16, 2023

That's the easy solution, but is that okay for alt-ergo ?

@Halbaroth
Copy link
Contributor

Halbaroth commented Oct 16, 2023

I don't know. We should ask if it's okay for Why3 ;)

@bclement-ocp
Copy link
Contributor Author

Why? Why3 does not depend on alt-ergo.

@Gbury
Copy link
Owner

Gbury commented Oct 16, 2023

In any case, I just pushed baaf1f9 to master which should solve the build issue on 4.08 for now. Ill continue and try to think if/how to best support versions of ocaml < 4.10 for the parsing library.

@Gbury Gbury closed this as completed Oct 16, 2023
@bclement-ocp
Copy link
Contributor Author

FWIW list_concat_map was already defined in src/standard/expr.ml

@Gbury
Copy link
Owner

Gbury commented Oct 16, 2023

woops

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

No branches or pull requests

3 participants