Polymorphic compare in ReasonML/Bucklescript

#1

There is currently an ongoing discussion on the topic polymorphic compare in the OCaml forum:

Is this also a concern in the ReasonML/Bucklescript ecosystem, or has it been addressed in a different way?

#2

The issue exist also with bucklescript.

Here is a simple example that illustrates the problem:

module type IS = {
  type t;
  let empty: t;
  let add: (t, int) => t;
  let has: (t, int) => bool;
  let equal: (t, t) => bool;
};

module IntSet: IS = {
  type t = list(int);
  let empty = [];
  let add = (set, value) => Belt.List.add(set, value);
  let has = (set, value) => Belt.List.has(set, value, (==));
  let rec equal = (a, b) =>
    switch (a, b) {
    | ([x, ...a], b) => has(b, x) && equal(a, b)
    | ([], b) => true
    };
};

let a = IntSet.(empty->add(1)->add(2)->add(3));
let b = IntSet.(empty->add(1)->add(3)->add(2));

/* a and b are semantically equal */
/* But pervasive.compare or (==) consider them as different */

Js.log(IntSet.equal(a, b)); /* true */
Js.log(a == b); /* false (But we expect a and b to be true)*/
Js.log(a === b); /* false */

a and b are semantically equal.
But pervasive.compare or (==) consider them as different.

#3

This is really confusing with OCaml, since the semantics are different for built-in modules.

module IntSet = Set.Make(Int32);
let a = IntSet.add(Int32.of_int(1),IntSet.singleton(Int32.of_int(2)));
let b = IntSet.add(Int32.of_int(1),IntSet.singleton(Int32.of_int(2)));

IntSet.equal(a, b); /* true */
a == b; /* true (compared to false in your example) */
a === b; /* false */

It would be nice if it would be possible to define what function to invoke for a specific operator such as ==,
but I understand this is a limitation in the OCaml compiler and is hard to fix.

The solution in Jane Street’s Base/Core to simply block == even for primitives such as strings seems a bit extreme to me:

open Base;;
utop # "foo" = "foo";;
Error: This expression has type string but an expression was expected of type
         int

I hope the ReasonML/Bucklescript project manage to come up with a more user-friendly solution.
I’m hopeful, since they have been very successful in hacking on the OCaml compiler so far, perhaps there is a clean safe solution to this problem as well.

#4

@gunner In my example I reverse the order of adding data in the set.
Try to reverse in your example you will have the same observation.

The solution in Jane Street’s Base/Core to simply block == even for primitives such as strings seems a bit extreme to me:

I didn’t think the == on string could be blocked :scream:

#5

I don’t believe this is what bucklescript is doing. It’s true that Bucklescript is a fork of OCaml compiler but it’s for generating better JS code. It doesn’t change OCaml’s semantics

#6

Yes it does change semantics.

#7

Actually we does better than the native side.
There is a flag in BuckleScript to provide warnings when bucklescript compiler can not specialize the polymophic comparision. It is recommended to turn it on (warn-error) for experienced users

#8

This is completely awesome! Thanks!

Demo:

print_endline("Compare strings: " ++ string_of_bool("foo" == "foo"));

module IntSet = Set.Make(Int32);
let s1 = IntSet.singleton(Int32.zero);
let s2 = IntSet.singleton(Int32.zero);
print_endline("Compare sets:" ++ string_of_bool(s1 == s2));

Added +102 to bsconfig.json:

{"warnings": {
    "number": "+102",
    "error" : "+101+102"
  }}

List of warnings here: https://github.com/BuckleScript/bucklescript/blob/master/vendor/ocaml/utils/warnings.mli

Polymophic comparision detected and stopped by bsb:

$ bsb
[3/3] Building src/Demo.mlast.d
[1/1] Building src/Demo-ReasonPolymorphicCompare.cmj

  Warning number 102
  /Users/joel/src/reason-polymorphic-compare/src/Demo.re 6:49-56

  4 │ let s1 = IntSet.singleton(Int32.zero);
  5 │ let s2 = IntSet.singleton(Int32.zero);
  6 │ print_endline("Compare sets:" ++ string_of_bool(s1 == s2));

  polymorphic comparison introduced (maybe unsafe)

  We've found a bug for you!
  /Users/joel/src/reason-polymorphic-compare/src/Demo.re

  Some fatal warnings were triggered (1 occurrences)
#9

Comparison of simple lists/arrays/tuples containing primitives also seems to be caught by the warning:

print_endline("Compare list of ints: " ++ string_of_bool([1,2,3] == [1,2,3]));

It would be nice if this would also be allowed with +102 turned on.

If the list/array/tuples only contain primitives (base types), I can’t see how there can be any ambiguity on how to do comparison, but I suppose it can be technically very hard to implement if the information in the typed AST is not sufficient.

#10

but I suppose it can be technically very hard to implement if the information

Yes, that’s the reason we stop here.
The warning-error (+102) is strongly recommended to turn it on for people who know what they are doing, actually it is by default before, but we flipped it later since some people complained about it…