NaN can be used as a key in Map

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Map.enso

@@ -15,6 +15,12 @@ from project import Error, Nothing, Any, Panic
    for equality any objects that can appear in Enso - primitives, Atoms, values coming from
    different languages, etc.
 
+   For keys that are not reflexive, like `Number.nan`,
+   [Same Value equality specification](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Equality_comparisons_and_sameness#same-value-zero_equality)
+   is used. This means that both `Number.nan` and types with comparators that violate
+   reflexivity (e.g. their `compare` method always returns `Nothing`) can be used as keys
+   in the Map.
+
    A single key-value pair is called an *entry*.
 
    It is possible to pass a Map created in Enso to foreign functions, where it will be treated

engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java

@@ -22,7 +22,8 @@
 @BuiltinMethod(
     type = "Any",
     name = "==",
-    description = """
+    description =
+        """
       Compares self with other object and returns True iff `self` is exactly the same as
       the other object, including all its transitively accessible properties or fields,
       False otherwise.
@@ -32,9 +33,9 @@
       Does not throw dataflow errors or panics.
 
       Note that this is different than `Meta.is_same_object`, which checks whether two
-      references point to the same object on the heap.
-      """
-)
+      references point to the same object on the heap. Moreover, `Meta.is_same_object`
+      implies `Any.==` for all object with the exception of `Number.nan`.
+      """)
 @GenerateUncached
 public abstract class EqualsNode extends Node {
 
@@ -44,11 +45,6 @@ public static EqualsNode build() {
 
   public abstract boolean execute(@AcceptsError Object self, @AcceptsError Object right);
 
-  /**
-   * Primitive values
-   **/
-
-
   @Specialization
   boolean equalsBoolBool(boolean self, boolean other) {
     return self == other;
@@ -166,10 +162,7 @@ boolean equalsLongBigInt(long self, EnsoBigInteger other) {
     }
   }
 
-  @Specialization(guards = {
-    "selfText.is_normalized()",
-    "otherText.is_normalized()"
-  })
+  @Specialization(guards = {"selfText.is_normalized()", "otherText.is_normalized()"})
   boolean equalsTextText(Text selfText, Text otherText) {
     return selfText.toString().compareTo(otherText.toString()) == 0;
   }
@@ -199,13 +192,14 @@ boolean equalsTextBigInt(Text self, EnsoBigInteger other) {
    * normalization. See {@code Text_Utils.compare_to}.
    */
   @TruffleBoundary
-  @Specialization(guards = {
-      "selfInterop.isString(selfString)",
-      "otherInterop.isString(otherString)"
-  }, limit = "3")
-  boolean equalsStrings(Object selfString, Object otherString,
-                        @CachedLibrary("selfString") InteropLibrary selfInterop,
-                        @CachedLibrary("otherString") InteropLibrary otherInterop) {
+  @Specialization(
+      guards = {"selfInterop.isString(selfString)", "otherInterop.isString(otherString)"},
+      limit = "3")
+  boolean equalsStrings(
+      Object selfString,
+      Object otherString,
+      @CachedLibrary("selfString") InteropLibrary selfInterop,
+      @CachedLibrary("otherString") InteropLibrary otherInterop) {
     String selfJavaString;
     String otherJavaString;
     try {
@@ -214,43 +208,43 @@ boolean equalsStrings(Object selfString, Object otherString,
     } catch (UnsupportedMessageException e) {
       throw new IllegalStateException(e);
     }
-    return Normalizer.compare(
-        selfJavaString,
-        otherJavaString,
-        Normalizer.FOLD_CASE_DEFAULT
-    ) == 0;
+    return Normalizer.compare(selfJavaString, otherJavaString, Normalizer.FOLD_CASE_DEFAULT) == 0;
   }
 
-  @Specialization(
-    guards = "isPrimitive(self, strings) != isPrimitive(other, strings)"
-  )
-  boolean equalsDifferent(Object self, Object other, @CachedLibrary(limit = "10") InteropLibrary strings) {
+  @Specialization(guards = "isPrimitive(self, interop) != isPrimitive(other, interop)")
+  boolean equalsDifferent(
+      Object self, Object other, @CachedLibrary(limit = "10") InteropLibrary interop) {
     return false;
   }
 
   /** Equals for Atoms and AtomConstructors */
-
   @Specialization
   boolean equalsAtomConstructors(AtomConstructor self, AtomConstructor other) {
     return self == other;
   }
 
   @Specialization
-  boolean equalsAtoms(Atom self, Atom other, @Cached EqualsAtomNode equalsNode) {
-    return equalsNode.execute(self, other);
+  boolean equalsAtoms(
+      Atom self,
+      Atom other,
+      @Cached EqualsAtomNode equalsNode,
+      @Cached IsSameObjectNode isSameObjectNode) {
+    return isSameObjectNode.execute(self, other) || equalsNode.execute(self, other);
   }
-  
-  @Specialization(guards = "isNotPrimitive(self, other, strings, warnings)")
+
+  @Specialization(guards = "isNotPrimitive(self, other, interop, warnings)")
   boolean equalsComplex(
-    Object self, Object other,
-    @Cached EqualsComplexNode complex,
-    @CachedLibrary(limit = "10") InteropLibrary strings,
-    @CachedLibrary(limit = "10") WarningsLibrary warnings
-  ) {
-    return complex.execute(self, other);
+      Object self,
+      Object other,
+      @Cached EqualsComplexNode equalsComplex,
+      @Cached IsSameObjectNode isSameObjectNode,
+      @CachedLibrary(limit = "10") InteropLibrary interop,
+      @CachedLibrary(limit = "5") WarningsLibrary warnings) {
+    return isSameObjectNode.execute(self, other) || equalsComplex.execute(self, other);
   }
 
-  static boolean isNotPrimitive(Object a, Object b, InteropLibrary strings, WarningsLibrary warnings) {
+  static boolean isNotPrimitive(
+      Object a, Object b, InteropLibrary interop, WarningsLibrary warnings) {
     if (a instanceof AtomConstructor && b instanceof AtomConstructor) {
       return false;
     }
@@ -260,22 +254,22 @@ static boolean isNotPrimitive(Object a, Object b, InteropLibrary strings, Warnin
     if (warnings.hasWarnings(a) || warnings.hasWarnings(b)) {
       return true;
     }
-    return !isPrimitive(a, strings) && !isPrimitive(b, strings);
+    return !isPrimitive(a, interop) && !isPrimitive(b, interop);
   }
 
   /**
-   * Return true iff object is a primitive value used in some specializations
-   * guard. By primitive value we mean any value that can be present in Enso, so,
-   * for example, not Integer, as that cannot be present in Enso.
-   * All the primitive types should be handled in their corresponding specializations.
-   * See {@link org.enso.interpreter.node.expression.builtin.interop.syntax.HostValueToEnsoNode}.
+   * Return true iff object is a primitive value used in some specializations guard. By primitive
+   * value we mean any value that can be present in Enso, so, for example, not Integer, as that
+   * cannot be present in Enso. All the primitive types should be handled in their corresponding
+   * specializations. See {@link
+   * org.enso.interpreter.node.expression.builtin.interop.syntax.HostValueToEnsoNode}.
    */
-  static boolean isPrimitive(Object object, InteropLibrary strings) {
-    return object instanceof Boolean ||
-        object instanceof Long ||
-        object instanceof Double ||
-        object instanceof EnsoBigInteger ||
-        object instanceof Text ||
-        strings.isString(object);
+  static boolean isPrimitive(Object object, InteropLibrary interop) {
+    return object instanceof Boolean
+        || object instanceof Long
+        || object instanceof Double
+        || object instanceof EnsoBigInteger
+        || object instanceof Text
+        || interop.isString(object);
   }
 }

engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/IsSameObjectNode.java

@@ -1,6 +1,7 @@
 package org.enso.interpreter.node.expression.builtin.meta;
 
 import com.oracle.truffle.api.dsl.Fallback;
+import com.oracle.truffle.api.dsl.GenerateUncached;
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.interop.InteropLibrary;
 import com.oracle.truffle.api.interop.UnsupportedMessageException;
@@ -15,6 +16,7 @@
     name = "is_same_object",
     description = "Checks if the two arguments share an underlying reference.",
     autoRegister = false)
+@GenerateUncached
 public abstract class IsSameObjectNode extends Node {
 
   public static IsSameObjectNode build() {
@@ -23,6 +25,15 @@ public static IsSameObjectNode build() {
 
   public abstract boolean execute(@AcceptsError Object left, @AcceptsError Object right);
 
+  @Specialization
+  boolean isSameDouble(double left, double right) {
+    if (Double.isNaN(left) && Double.isNaN(right)) {
+      return true;
+    } else {
+      return left == right;
+    }
+  }
+
   @Specialization
   boolean isSameType(Type typeLeft, Type typeRight) {
     return typeLeft == typeRight;

test/Tests/src/Data/Map_Spec.enso

@@ -10,9 +10,22 @@ import Standard.Test.Extensions
 polyglot java import java.nio.file.Path as JavaPath
 polyglot java import java.util.Map as JavaMap
 
+## Type that violates reflexivity
+type My_Nan
+    Value comment:Text
+
+type My_Nan_Comparator
+    compare _ _ = Nothing
+    hash _ = 0
+
+Comparable.from (_:My_Nan) = My_Nan_Comparator
+
 foreign js js_str str = """
     return new String(str)
 
+foreign js js_null = """
+    return null
+
 foreign js js_empty_dict = """
     return new Map()
 
@@ -125,6 +138,18 @@ spec =
             map.size.should_equal 1
             map.get [1, "a", 2] . should_equal "Value"
 
+        Test.specify "should support NaN as keys" <|
+            Map.empty.insert Number.nan 1 . contains_key Number.nan . should_be_true
+            Map.empty.insert Number.nan 1 . values . should_equal [1]
+            Map.empty.insert Number.nan 1 . insert Number.nan 2 . contains_key Number.nan . should_be_true
+            Map.empty.insert Number.nan 1 . insert Number.nan 2 . values . should_equal [2]
+            Map.empty.insert Number.nan 1 . insert "key" 2 . insert Number.nan 3 . contains_key Number.nan . should_be_true
+            Map.empty.insert Number.nan 1 . insert "key" 2 . insert Number.nan 3 . contains_key "key" . should_be_true
+            Map.empty.insert Number.nan 1 . insert "key" 2 . insert Number.nan 3 . at Number.nan . should_equal 3
+            Map.empty.insert Number.nan 1 . insert "key" 2 . insert Number.nan 3 . at "key" . should_equal 2
+            Map.empty.insert Number.nan 1 . insert Number.nan Number.nan . at Number.nan . to_text . should_equal "NaN"
+            Map.empty.insert Number.nan 1 . insert Number.nan Number.nan . remove Number.nan . size . should_equal 0
+
         Test.specify "should support dates as keys" <|
             map = Map.empty.insert (Date.new 1993) 1 . insert (Date.new 1993 2 5) 2 . insert (Date_Time.new 1993 2 5 13 45) 3
             map.size.should_equal 3
@@ -143,6 +168,33 @@ spec =
             (map.get key_map).should_equal 23
             (map.get map).should_equal Nothing
 
+        Test.specify "should support another hash map with NaN keys as key" <|
+            Map.singleton (Map.singleton Number.nan 1) 42 . size . should_equal 1
+            Map.singleton (Map.singleton Number.nan 1) 42 . keys . at 0 . keys . to_text . should_equal "[NaN]"
+            Map.singleton (Map.singleton Number.nan 1) 42 . keys . at 0 . get Number.nan . should_equal 1
+            Map.singleton (Map.singleton Number.nan 1) 42 . at (Map.singleton Number.nan 1) . should_equal 42
+
+        Test.specify "should support atoms with custom comparators that violate reflexivity as keys" <|
+            k = My_Nan.Value "foo"
+            k2 = My_Nan.Value "foo"
+            (k==k).should_be_true
+            (k==k2).should_be_false
+            Meta.is_same_object k k2 . should_be_false
+            Meta.is_same_object k k . should_be_true
+            m = Map.empty.insert k 10
+            m.contains_key k . should_be_true
+            m.get k . should_equal 10
+            m.contains_key k2 . should_be_false
+
+            m2 = m.insert k2 20
+            m2.get k . should_equal 10
+            m2.get k2 . should_equal 20
+            m2.size . should_equal 2
+
+            m3 = m2.insert k 30
+            m3.size . should_equal 2
+            m3.get k . should_equal 30
+
         Test.specify "should handle keys with standard equality semantics" <|
             map = Map.singleton 2 "Hello"
             (map.get 2).should_equal "Hello"
@@ -152,6 +204,9 @@ spec =
         Test.specify "should handle Nothing as keys" <|
             Map.singleton Nothing 3 . get Nothing . should_equal 3
             Map.singleton Nothing 1 . insert Nothing 2 . get Nothing . should_equal 2
+            Map.singleton Nothing 1 . should_equal (Map.singleton Nothing 1)
+            Map.singleton Nothing 1 . insert Nothing 2 . at Nothing . should_equal 2
+            Map.singleton js_null 1 . at Nothing . should_equal 1
 
         Test.specify "should handle incomparable values as keys" <|
             Map.empty.insert Number.nan 1 . insert Number.nan 2 . get Number.nan . should_equal 2

test/Tests/src/Data/Vector_Spec.enso

@@ -22,6 +22,16 @@ type T_Comparator
 
 Comparable.from (_:T) = T_Comparator
 
+## Type that violates reflexivity
+type My_Nan
+    Value value
+
+type My_Nan_Comparator
+    compare _ _ = Nothing
+    hash _ = 0
+
+Comparable.from (_:My_Nan) = My_Nan_Comparator
+
 type My_Error
     Value a
 
@@ -654,8 +664,26 @@ type_spec name alter = Test.group name <|
         alter [1, 1.0, 2, 2.0] . distinct . should_equal [1, 2]
         alter [] . distinct . should_equal []
 
+    Test.specify "should be able to handle distinct on different primitive values" <|
+        alter [1, "a"] . distinct . should_equal [1, "a"]
+        alter ["a", 1] . distinct . should_equal ["a", 1]
+        alter [Nothing, Nothing] . distinct . should_equal [Nothing]
+        alter [Number.nan, Number.nan] . distinct . to_text . should_equal "[NaN]"
+        alter [Nothing, Number.nan, Nothing, Number.nan] . distinct . to_text . should_equal "[Nothing, NaN]"
+        alter [1, Nothing] . distinct . should_equal [1, Nothing]
+        alter [Nothing, 1, Nothing] . distinct . should_equal [Nothing, 1]
+        alter [1, Number.nan] . distinct . to_text . should_equal "[1, NaN]"
+        alter [Number.nan, 1, Number.nan] . distinct . to_text . should_equal "[NaN, 1]"
+        alter [1, Number.nan, 1] . distinct . to_text . should_equal "[1, NaN]"
+        alter [Number.nan, My_Nan.Value 42, My_Nan.Value 23] . distinct . to_text . should_equal "[NaN, (My_Nan.Value 42), (My_Nan.Value 23)]"
+        my_nan = My_Nan.Value 42
+        alter [my_nan, Number.nan, my_nan] . distinct . to_text . should_equal "[(My_Nan.Value 42), NaN]"
+
     Test.specify "should correctly handle distinct with types that have custom comparators" <|
         alter [T.Value 1 2, T.Value 3 3, T.Value 1 2] . distinct . should_equal [T.Value 1 2, T.Value 3 3]
+        alter [T.Value 1 2, T.Value 3 3, T.Value 1 2, Nothing] . distinct . should_equal [T.Value 1 2, T.Value 3 3, Nothing]
+        alter [Nothing, T.Value 1 2, T.Value 3 3, T.Value 1 2, Nothing] . distinct . should_equal [Nothing, T.Value 1 2, T.Value 3 3]
+        alter [T.Value 1 2, Date.new hours=3] . distinct . should_equal [T.Value 1 2, Date.new hours=3]
 
     Test.specify "should return a vector containing only unique elements up to some criteria" <|
         alter [Pair.new 1 "a", Pair.new 2 "b", Pair.new 1 "c"] . distinct (on = _.first) . should_equal [Pair.new 1 "a", Pair.new 2 "b"]

test/Tests/src/Semantic/Equals_Spec.enso

@@ -33,6 +33,16 @@ type Child_Comparator
 
 Comparable.from (_:Child) = Child_Comparator
 
+## Type that violates reflexivity
+type My_Nan
+    Value val
+
+type My_Nan_Comparator
+    compare _ _ = Nothing
+    hash _ = 0
+
+Comparable.from (_:My_Nan) = My_Nan_Comparator
+
 type Parent
     Value child
 
@@ -159,6 +169,14 @@ spec =
             four_field = FourFieldType.Value 1 2 3 4
             (rect == four_field).should_be_false
 
+        Test.specify "Any.== should check for Meta.is_same_object" <|
+            obj_1 = My_Nan.Value 42
+            obj_2 = My_Nan.Value 42
+            (obj_1 == obj_2).should_be_false
+            Meta.is_same_object obj_1 obj_2 . should_be_false
+            (obj_1 == obj_1).should_be_true
+            Meta.is_same_object obj_1 obj_1 . should_be_true
+
         Test.specify "should handle `==` on types" <|
             (Child == Child).should_be_true
             (Child == Point).should_be_false

test/Tests/src/Semantic/Meta_Spec.enso

@@ -281,13 +281,18 @@ spec =
             Meta.get_annotation value "Value" "bar" . should_equal Nothing
             Meta.get_annotation value "Value" "baz" . should_equal (My_Type.Value 1 2 3)
 
-    Test.group "Check Nothing" <|
+    Test.group "Check Nothing and NaN" <|
         Test.specify "Nothing.is_a Nothing" <|
             Nothing.is_a Nothing . should_be_true
+            Meta.is_same_object Nothing Nothing . should_be_true
 
         Test.specify "type_of Nothing is Nothing" <|
             Meta.type_of Nothing . should_equal Nothing
 
+        Test.specify "NaN and NaN should be the same object" <|
+            Meta.is_same_object Number.nan Number.nan . should_be_true
+            (Number.nan == Number.nan) . should_be_false
+
     Test.group "Atom with holes" <|
         Test.specify "construct and fill" <|
             pair = Meta.atom_with_hole (e -> My_Type.Value 1 e 3)