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

Get rid of stdcompat #1191

Merged
merged 6 commits into from
Jul 31, 2024
Merged

Conversation

Halbaroth
Copy link
Collaborator

We do not need to use stdcompat because we only need to support few new functions from stdlib. This PR removes the stdcompat dependency and create a new module compat which contains all the functions we need to compile on OCaml 4.08.

I also rename Lists to my_list and myUnix to my_unix and now my_list only contains extra functions (which are not part of any stdlib versions).

@Halbaroth Halbaroth force-pushed the get-rid-of-stdcompat branch from 30f1171 to ead131b Compare July 31, 2024 13:02
We do not need to use `stdcompat` because we only need to support few
new functions from `stdlib`. This PR removes the `stdcompat` dependency
and create a new module `compat` which contains all the functions we
need to compile on OCaml 4.08.

I also rename `Lists` to `my_list` and `myUnix` to `my_unix` and now
`my_list` only contains extra functions (which are not part of any
stdlib versions).
@Halbaroth Halbaroth requested a review from bclement-ocp July 31, 2024 14:14
Copy link
Collaborator

@bclement-ocp bclement-ocp left a comment

Choose a reason for hiding this comment

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

You asked for review but kept the PR as draft, is this intended?

Looks fine overall.

@@ -462,13 +462,13 @@ let mk_theory_opt () no_contracongru
no_fm no_nla no_tcp no_theory restricted tighten_vars
_use_fpa (theories)
=
set_no_ac (not (Lists.mem Theories.equal Theories.AC theories));
set_no_ac (not (My_list.mem Theories.equal Theories.AC theories));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: I think we could get rid of My_list.mem everywhere with List.exists (Theories.equal Theories.AC) theories. Not sure we want to.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure

@@ -322,7 +322,7 @@ module Shostak (X : ALIEN) = struct
| Record (lbs, ty) ->
Record (List.map (fun (n,e') -> n, subst_access x s e') lbs, ty)
| Access (lb, e', _) when compare_mine x e' = 0 ->
Lists.assoc Uid.equal lb s
Compat.List.assoc Uid.equal lb s
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Compat.List.assoc Uid.equal lb s
My_list.assoc Uid.equal lb s

Comment on lines 15 to 23
(* The Alt-Ergo theorem prover *)
(* *)
(* Sylvain Conchon, Evelyne Contejean, Francois Bobot *)
(* Mohamed Iguernelala, Stephane Lescuyer, Alain Mebsout *)
(* *)
(* CNRS - INRIA - Universite Paris Sud *)
(* *)
(* --------------------------------------------------------------- *)
(* *)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to stay since we simply rename the file while keeping the existing code.

(* CNRS - INRIA - Universite Paris Sud *)
(* *)
(* --------------------------------------------------------------- *)
(* *)
(* More details can be found in the directory licenses/ *)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same, needs to stay.

Comment on lines 28 to 33
(** Lists utilies

This modules defines some helper functions on lists
*)

(** {3 Misc functions} *)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why remove this?

end

module Bytes = struct
include Bytes
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would argue it would be better to do open Bytes at the beginning of the module, and include Bytes at the end (with [@@@ocaml.warning "-32-33"] at the top of the file). This way, we would use the stdlib version when it is available and only fall back to our version if its not.

(Same for the List, Array, etc. in this module)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was looking for such trick in fact, thanks :)

@Halbaroth Halbaroth marked this pull request as ready for review July 31, 2024 14:42
@Halbaroth Halbaroth requested a review from bclement-ocp July 31, 2024 14:56
@Halbaroth Halbaroth merged commit 72f4c27 into OCamlPro:next Jul 31, 2024
15 checks passed
bclement-ocp added a commit to bclement-ocp/alt-ergo that referenced this pull request Aug 1, 2024
The syntax used for silencing warning in OCamlPro#1191 is incorrect.
Halbaroth pushed a commit that referenced this pull request Aug 1, 2024
The syntax used for silencing warning in #1191 is incorrect.
bclement-ocp added a commit to bclement-ocp/alt-ergo that referenced this pull request Aug 2, 2024
The syntax used for silencing warning in OCamlPro#1191 is incorrect.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants