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

Set.map does not work reliably #7403

Closed
vicuna opened this issue Nov 5, 2016 · 4 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link

commented Nov 5, 2016

Original bug ID: 7403
Reporter: talex
Assigned to: @gasche
Status: resolved (set by @gasche on 2016-11-07T19:34:09Z)
Resolution: fixed
Priority: normal
Severity: major
Version: 4.04.0
Fixed in version: 4.04.1+dev
Category: standard library

Bug description

There seems to be a bug in the new Set.map in 4.04.

I think perhaps the two v variables here should be v':

if (l' = Empty || Ord.compare (max_elt l') v < 0)

Steps to reproduce

module S = Set.Make(String)

let f = function
| "b" -> "z"
| x -> x

let () =
let a = S.of_list ["b"; "a"; "c"] in
let b = S.map f a in
print_endline "Elements of b:";
S.iter (Printf.printf "- %s\n") b;
let b_has x =
Printf.printf "%S in b = %b\n" x (S.mem x b) in
List.iter b_has ["a"; "b"; "c"; "z"]

Produces:

Elements of b:

  • a
  • z
  • c
    "a" in b = true
    "b" in b = false
    "c" in b = false
    "z" in b = true
@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 5, 2016

Comment author: @gasche

Ouch, I'm to blame on that one... The code was too clever for its own good.

Could you submit a pull-request with a testsuite regression test?

@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 6, 2016

Comment author: @gasche

I will write the pull-request and regression test. I hope that we can put the fix as an opam patch, and warn the packagers, so that users are not too affected -- I think it would make sense to wait a bit more to consider a 4.04.1 bugfix release, given that other issues may be encountered as 4.04.0 adoption progresses.

@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 6, 2016

Comment author: @gasche

#894

@vicuna

This comment has been minimized.

Copy link
Author

commented Nov 7, 2016

Comment author: @gasche

Fixed in the 4.04 branch -- will cherry-pick in trunk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.