Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

added to_rev_seq to map, splay and set #1031

Merged

Conversation

jabot2
Copy link
Contributor

@jabot2 jabot2 commented Feb 26, 2021

No description provided.

@jabot2
Copy link
Contributor Author

jabot2 commented Feb 26, 2021

This PR is not enough to achieve compatibility with 4.12 though.

File "src/batteries_compattest.ml", line 119, characters 53-59:
119 |   let _ = assert ([1,1;2,2;3,3;] = (sort_map (module IntMap) [3; 1; 2;]))
                                                           ^^^^^^
Error: Signature mismatch:
       ...
       Type declarations do not match:
         type 'a t = 'a BatMap.Int.t
       is not included in
         type +!'a t
       Their variances do not agree.
       File "map.mli", line 70, characters 4-15: Expected declaration
       File "src/batMap.mli", line 76, characters 2-14: Actual declaration

Annoyingly, the new syntax for variances is not backwards compatible, i.e. writing type +!a t = ... won't compile on e.g. 4.02.

@kit-ty-kate
Copy link
Contributor

The Map.t type could simply be conditionally written with and without the injectivity annotation using batteries' prepocessor. e.g.

##>=4.12 type +!'a t ...
##<4.12 type +'a t ...

@jabot2
Copy link
Contributor Author

jabot2 commented Feb 26, 2021

@kit-ty-kate is it possible to add those annotations to a .ml / .mli file? or do the files need to be renamed?

@UnixJunkie
Copy link
Member

Use this:

type +
##V>=4.12## !
'a t ...

@UnixJunkie
Copy link
Member

If it's a .mli file, it will need to be renamed .mliv.
If it's a .ml file --> .mlv.

@UnixJunkie
Copy link
Member

I will try to merge this tonight and fix what's needed; don't bother and thanks a lot for contributing this.

Copy link
Member

@gasche gasche left a comment

Choose a reason for hiding this comment

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

The code is fine. Thanks!

@gasche gasche merged commit 7ba3b28 into ocaml-batteries-team:master Feb 27, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants