Skip to content

Possible fix for CI#50

Closed
JTrenerry wants to merge 3 commits intomainfrom
JTrenerry-patch-1
Closed

Possible fix for CI#50
JTrenerry wants to merge 3 commits intomainfrom
JTrenerry-patch-1

Conversation

@JTrenerry
Copy link
Collaborator

@JTrenerry JTrenerry commented Feb 17, 2026

Issue seems to be the new update for containers 2 days ago, not sure why this flips the order though.

@JTrenerry JTrenerry self-assigned this Feb 17, 2026
@JTrenerry
Copy link
Collaborator Author

lol wrong way

@JTrenerry
Copy link
Collaborator Author

Doubt this is what we want in the long term but possible quick fix.

@JTrenerry JTrenerry requested a review from agle February 17, 2026 06:36
@b-paul
Copy link
Contributor

b-paul commented Feb 17, 2026

it looks like this commit is what broke things c-cube/ocaml-containers@571f9f3 (did a bisect with pinning versions of containers)

the only change in this commit that differs between ocaml 5.3 and ocaml 4.14.x is the implementation of Map.of_list and Map.to_list, and of_list is used in loadir.ml....... suspicious

though the implementation of of_list in containers and stdlib looks identical, so it must be to_list?!?!

to_list is implemented with a fold in containers which iterates left before right, but is implemented with recursion in stdlib that iterates right before left?!?!?!?! that's probably where things get reversed...

@agle
Copy link
Owner

agle commented Feb 18, 2026

Thanks, b-paul, it looks like the fix is to use bindings which defines the order, instead of to_list which preserves stdlib behvaiour (which is undefined for 4.*).

@agle
Copy link
Owner

agle commented Feb 18, 2026

I don't think we get any real benefit from supporting OCaml 4.14 so I may just remove it from the test CI matrix and not put more effort into supporting it---this is the second instance of differing stdlib semantics between 4 and 5 I've come across.

@agle agle closed this Feb 18, 2026
@JTrenerry JTrenerry deleted the JTrenerry-patch-1 branch February 18, 2026 02:05
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

Successfully merging this pull request may close these issues.

3 participants