From ff88018190668a8a55bbddca7e4dc7598cee69bb Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 17 Apr 2023 10:53:53 +0200 Subject: [PATCH 01/17] Check for Meta.is_same_object in Any.== --- .../lib/Standard/Base/0.0.0-dev/src/Any.enso | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Any.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Any.enso index 1c41b6337ab6..f803914c4307 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Any.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Any.enso @@ -109,21 +109,22 @@ type Any a == 147 == : Any -> Boolean == self that = - # If there is No_Such_Conversion, then `self` and `that` are probably - # host or polyglot values, so we just compare them with the default comparator. - eq_self = Comparable.from self - eq_that = Comparable.from that - similar_type = Meta.is_same_object eq_self eq_that - case similar_type of - False -> False - True -> - case Meta.is_same_object eq_self Default_Comparator of - # Shortcut for objects with Default_Comparator, because of the performance. - True -> Comparable.equals_builtin self that - False -> - case eq_self.compare self that of - Ordering.Equal -> True - _ -> False + case Meta.is_same_object self that of + True -> True + False -> + eq_self = Comparable.from self + eq_that = Comparable.from that + similar_type = Meta.is_same_object eq_self eq_that + case similar_type of + False -> False + True -> + case Meta.is_same_object eq_self Default_Comparator of + # Shortcut for objects with Default_Comparator, because of the performance. + True -> Comparable.equals_builtin self that + False -> + case eq_self.compare self that of + Ordering.Equal -> True + _ -> False ## ALIAS Inequality From 6302281ee6f60622bd80a80887b3e12cfaa1793d Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 17 Apr 2023 11:17:48 +0200 Subject: [PATCH 02/17] Add some tests for Vector.distinct --- test/Tests/src/Data/Vector_Spec.enso | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/Tests/src/Data/Vector_Spec.enso b/test/Tests/src/Data/Vector_Spec.enso index e5cd7c8c4886..ce8488fb28df 100644 --- a/test/Tests/src/Data/Vector_Spec.enso +++ b/test/Tests/src/Data/Vector_Spec.enso @@ -654,8 +654,23 @@ 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"] + 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"] From 32d81f46c0d54d533e3404c98dcffaa1dbb00230 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 17 Apr 2023 15:31:03 +0200 Subject: [PATCH 03/17] `Meta.is_same_object NaN NaN` returns True --- .../node/expression/builtin/meta/IsSameObjectNode.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/IsSameObjectNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/IsSameObjectNode.java index 7c21cc40fcb9..58e9395b1df4 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/IsSameObjectNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/IsSameObjectNode.java @@ -23,6 +23,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; From 061cb7992ba513022b3e7868f28050726ca5e3c3 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 17 Apr 2023 15:50:49 +0200 Subject: [PATCH 04/17] Add Map tests for various incomparable keys --- test/Tests/src/Data/Map_Spec.enso | 58 +++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/test/Tests/src/Data/Map_Spec.enso b/test/Tests/src/Data/Map_Spec.enso index 64975c067654..86d2fa42fd47 100644 --- a/test/Tests/src/Data/Map_Spec.enso +++ b/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,21 @@ 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 . should_equal Map.singleton Number.nan 1 + Map.empty.insert Number.nan 1 . contains_key Number.nan . should_be_true + Map.empty.insert Number.nan 1 . insert Number.nan 2 . should_equal Map.singleton Number.nan 2 + Map.empty.insert Number.nan 1 . insert "key" 2 . insert Number.nan 3 . should_equal Map.singleton Number.nan 3 + 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 vector with NaNs as keys" <| + Map.emtpy.insert [Number.nan] 1 . keys . to_text . should_equal [["NaN"]] + Map.emtpy.insert [Number.nan] 1 . values . should_equal [1] + Map.emtpy.insert [Number.nan] 1 . at Number.nan . should_equal 1 + Map.emtpy.insert [Number.nan] 1 . insert [Number.nan, Number.nan] 2 . keys . to_text . should_equal [["NaN"], ["NaN", "NaN"]] + Map.emtpy.insert [Number.nan] 1 . insert [Number.nan, Number.nan] 2 . values . should_equal [1, 2] + 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 +171,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 42 + 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 +207,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 . should_equal Map.singleton Nothing 1 + Map.singleton js_null 1 . should_equal Map.singleton Nothing 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 From 5f35024a67c09b1cb2bdccca49220fa167cc0cbb Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 17 Apr 2023 15:51:41 +0200 Subject: [PATCH 05/17] Test that `Meta.is_same_object` complies with "Same Value Zero" equality. Spec at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Equality_comparisons_and_sameness#same-value-zero_equality --- test/Tests/src/Semantic/Meta_Spec.enso | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/Tests/src/Semantic/Meta_Spec.enso b/test/Tests/src/Semantic/Meta_Spec.enso index ffbeba14712b..aff21b19532f 100644 --- a/test/Tests/src/Semantic/Meta_Spec.enso +++ b/test/Tests/src/Semantic/Meta_Spec.enso @@ -280,13 +280,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 + Nothing.is_same_object 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) From 6963198cf53cd60e47dfae1fdbc77d31e102a4fb Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 17 Apr 2023 16:01:00 +0200 Subject: [PATCH 06/17] Revert "`Meta.is_same_object NaN NaN` returns True" This reverts commit 32d81f46c0d54d533e3404c98dcffaa1dbb00230. --- .../node/expression/builtin/meta/IsSameObjectNode.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/IsSameObjectNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/IsSameObjectNode.java index 58e9395b1df4..7c21cc40fcb9 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/IsSameObjectNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/IsSameObjectNode.java @@ -23,15 +23,6 @@ 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; From aebbf0bfb1f386ea313771d5ff8aad497105cd16 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 17 Apr 2023 16:01:59 +0200 Subject: [PATCH 07/17] Add docs to Map --- distribution/lib/Standard/Base/0.0.0-dev/src/Data/Map.enso | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Map.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Map.enso index ad0a4a85f12b..0c8e3b4b4d60 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Map.enso +++ b/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", from JavaScript + specification in 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 From 35074849afa320cac15d866b9c7f3921ef3f2bc4 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 17 Apr 2023 16:14:28 +0200 Subject: [PATCH 08/17] Any.== checks for Meta.is_same_object (#6065) --- .../expression/builtin/meta/EqualsNode.java | 61 +++++++++---------- test/Tests/src/Semantic/Equals_Spec.enso | 18 ++++++ 2 files changed, 46 insertions(+), 33 deletions(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java index 06aa91ab52ed..879043a6f15a 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java @@ -214,22 +214,16 @@ 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; @@ -237,20 +231,21 @@ boolean equalsAtomConstructors(AtomConstructor self, AtomConstructor other) { @Specialization boolean equalsAtoms(Atom self, Atom other, @Cached EqualsAtomNode equalsNode) { - return equalsNode.execute(self, other); + return 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, + @CachedLibrary(limit = "10") InteropLibrary interop, + @CachedLibrary(limit = "10") WarningsLibrary warnings) { + return 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 +255,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); } } diff --git a/test/Tests/src/Semantic/Equals_Spec.enso b/test/Tests/src/Semantic/Equals_Spec.enso index 151ed10911ac..f7a80350c408 100644 --- a/test/Tests/src/Semantic/Equals_Spec.enso +++ 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 From cb520ed6c75464c22d13b16cd982f71e47699a7b Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 17 Apr 2023 15:31:03 +0200 Subject: [PATCH 09/17] `Meta.is_same_object NaN NaN` returns True --- .../node/expression/builtin/meta/IsSameObjectNode.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/IsSameObjectNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/IsSameObjectNode.java index 7c21cc40fcb9..58e9395b1df4 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/IsSameObjectNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/IsSameObjectNode.java @@ -23,6 +23,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; From 268c5f3aea06dd6690107078108fa77d62ff4d70 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 17 Apr 2023 19:06:15 +0200 Subject: [PATCH 10/17] Remove "should support vector with NaNs as keys". This is not according to the JS spec. --- test/Tests/src/Data/Map_Spec.enso | 7 ------- 1 file changed, 7 deletions(-) diff --git a/test/Tests/src/Data/Map_Spec.enso b/test/Tests/src/Data/Map_Spec.enso index 86d2fa42fd47..f6ea3dea07b9 100644 --- a/test/Tests/src/Data/Map_Spec.enso +++ b/test/Tests/src/Data/Map_Spec.enso @@ -146,13 +146,6 @@ spec = 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 vector with NaNs as keys" <| - Map.emtpy.insert [Number.nan] 1 . keys . to_text . should_equal [["NaN"]] - Map.emtpy.insert [Number.nan] 1 . values . should_equal [1] - Map.emtpy.insert [Number.nan] 1 . at Number.nan . should_equal 1 - Map.emtpy.insert [Number.nan] 1 . insert [Number.nan, Number.nan] 2 . keys . to_text . should_equal [["NaN"], ["NaN", "NaN"]] - Map.emtpy.insert [Number.nan] 1 . insert [Number.nan, Number.nan] 2 . values . should_equal [1, 2] - 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 From 7720f1487b20eba78c9bfe7d0cfa7e2b67cb5b71 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 17 Apr 2023 19:06:39 +0200 Subject: [PATCH 11/17] Fix typos in tests --- test/Tests/src/Data/Map_Spec.enso | 20 ++++++++++++-------- test/Tests/src/Data/Vector_Spec.enso | 10 +++++----- test/Tests/src/Semantic/Equals_Spec.enso | 4 ++-- test/Tests/src/Semantic/Meta_Spec.enso | 4 ++-- 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/test/Tests/src/Data/Map_Spec.enso b/test/Tests/src/Data/Map_Spec.enso index f6ea3dea07b9..132d4896ac53 100644 --- a/test/Tests/src/Data/Map_Spec.enso +++ b/test/Tests/src/Data/Map_Spec.enso @@ -139,10 +139,14 @@ spec = map.get [1, "a", 2] . should_equal "Value" Test.specify "should support NaN as keys" <| - Map.empty.insert Number.nan 1 . should_equal Map.singleton Number.nan 1 Map.empty.insert Number.nan 1 . contains_key Number.nan . should_be_true - Map.empty.insert Number.nan 1 . insert Number.nan 2 . should_equal Map.singleton Number.nan 2 - Map.empty.insert Number.nan 1 . insert "key" 2 . insert Number.nan 3 . should_equal Map.singleton Number.nan 3 + 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 @@ -166,8 +170,8 @@ spec = 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 42 + 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" <| @@ -200,9 +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 . should_equal Map.singleton Nothing 1 - Map.singleton js_null 1 . should_equal Map.singleton Nothing 1 + 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 diff --git a/test/Tests/src/Data/Vector_Spec.enso b/test/Tests/src/Data/Vector_Spec.enso index ce8488fb28df..526cd79f699f 100644 --- a/test/Tests/src/Data/Vector_Spec.enso +++ b/test/Tests/src/Data/Vector_Spec.enso @@ -658,13 +658,13 @@ type_spec name alter = Test.group name <| 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 [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 [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]" 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] diff --git a/test/Tests/src/Semantic/Equals_Spec.enso b/test/Tests/src/Semantic/Equals_Spec.enso index f7a80350c408..ced1df6a608f 100644 --- a/test/Tests/src/Semantic/Equals_Spec.enso +++ b/test/Tests/src/Semantic/Equals_Spec.enso @@ -172,9 +172,9 @@ spec = 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 + (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 + (obj_1 == obj_1).should_be_true Meta.is_same_object obj_1 obj_1 . should_be_true Test.specify "should handle `==` on types" <| diff --git a/test/Tests/src/Semantic/Meta_Spec.enso b/test/Tests/src/Semantic/Meta_Spec.enso index 05296bde5d7f..2779a0bfa608 100644 --- a/test/Tests/src/Semantic/Meta_Spec.enso +++ b/test/Tests/src/Semantic/Meta_Spec.enso @@ -284,14 +284,14 @@ spec = Test.group "Check Nothing and NaN" <| Test.specify "Nothing.is_a Nothing" <| Nothing.is_a Nothing . should_be_true - Nothing.is_same_object 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 + (Number.nan == Number.nan) . should_be_false Test.group "Atom with holes" <| Test.specify "construct and fill" <| From 4b3baa3743acbee6718ac0c624b21573003bb7fd Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 17 Apr 2023 19:06:59 +0200 Subject: [PATCH 12/17] bench_download tool: fail if there are no job reports --- tools/performance/engine-benchmarks/bench_download.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/performance/engine-benchmarks/bench_download.py b/tools/performance/engine-benchmarks/bench_download.py index 8743c296e20a..956015535cec 100755 --- a/tools/performance/engine-benchmarks/bench_download.py +++ b/tools/performance/engine-benchmarks/bench_download.py @@ -706,7 +706,8 @@ def commit_to_str(commit: Commit) -> str: else: bench_runs = get_bench_runs(since, until) if len(bench_runs) == 0: - print(f"No successful benchmarks found within period since {since} until {until}") + print( + f"No successful benchmarks found within period since {since} until {until}") exit(1) job_reports: List[JobReport] = [] for bench_run in bench_runs: @@ -714,6 +715,11 @@ def commit_to_str(commit: Commit) -> str: if job_report: job_reports.append(job_report) logging.debug(f"Got {len(job_reports)} job reports") + if len(job_reports) == 0: + print("There were 0 job_reports in the specified time interval, so " + "there is nothing to visualize or compare.") + exit(1) + if create_csv: write_bench_reports_to_csv(job_reports, csv_fname) logging.info(f"Benchmarks written to {csv_fname}") From 55ceaba9b1ff2a0cf63fec5b11aabbad834c0e06 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 18 Apr 2023 11:31:18 +0200 Subject: [PATCH 13/17] Use IsSameObject instead of reference comparison --- .../expression/builtin/meta/EqualsNode.java | 48 +++++++++---------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java index 879043a6f15a..9977ce6e2aa6 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java +++ b/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. However `Any.==` implies + `Meta.is_same_object` for all object with the exception of `Number.nan`. + """) @GenerateUncached public abstract class EqualsNode extends Node { @@ -44,11 +45,7 @@ public static EqualsNode build() { public abstract boolean execute(@AcceptsError Object self, @AcceptsError Object right); - /** - * Primitive values - **/ - - + /** Primitive values */ @Specialization boolean equalsBoolBool(boolean self, boolean other) { return self == other; @@ -166,10 +163,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 +193,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 { @@ -230,8 +225,12 @@ boolean equalsAtomConstructors(AtomConstructor self, AtomConstructor other) { } @Specialization - boolean equalsAtoms(Atom self, Atom other, @Cached EqualsAtomNode equalsNode) { - return self == other || 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, interop, warnings)") @@ -239,9 +238,8 @@ boolean equalsComplex( Object self, Object other, @Cached EqualsComplexNode equalsComplex, - @CachedLibrary(limit = "10") InteropLibrary interop, - @CachedLibrary(limit = "10") WarningsLibrary warnings) { - return self == other || equalsComplex.execute(self, other); + @Cached IsSameObjectNode isSameObjectNode) { + return isSameObjectNode.execute(self, other) || equalsComplex.execute(self, other); } static boolean isNotPrimitive( From a91e9852b8148e913c27c0f92a62c11aaba1e1bf Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Tue, 18 Apr 2023 11:59:29 +0200 Subject: [PATCH 14/17] Small fixes after merge --- .../interpreter/node/expression/builtin/meta/EqualsNode.java | 4 +++- .../node/expression/builtin/meta/IsSameObjectNode.java | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java index 9977ce6e2aa6..776b3efb9bb6 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java @@ -238,7 +238,9 @@ boolean equalsComplex( Object self, Object other, @Cached EqualsComplexNode equalsComplex, - @Cached IsSameObjectNode isSameObjectNode) { + @Cached IsSameObjectNode isSameObjectNode, + @CachedLibrary(limit = "10") InteropLibrary interop, + @CachedLibrary(limit = "5") WarningsLibrary warnings) { return isSameObjectNode.execute(self, other) || equalsComplex.execute(self, other); } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/IsSameObjectNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/IsSameObjectNode.java index 58e9395b1df4..24d25edad247 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/IsSameObjectNode.java +++ b/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() { From fc5aace5a9cd0cc3a3a00ba30dfd2fcb1199b96e Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 19 Apr 2023 10:27:29 +0200 Subject: [PATCH 15/17] Use markdown in docs --- distribution/lib/Standard/Base/0.0.0-dev/src/Data/Map.enso | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Map.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Map.enso index 0c8e3b4b4d60..108115c8c1a0 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Map.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Map.enso @@ -15,8 +15,8 @@ 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", from JavaScript - specification in https://developer.mozilla.org/en-US/docs/Web/JavaScript/Equality_comparisons_and_sameness#same-value-zero_equality, + 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. From d65e5e9f38a7b8b77e9673447af301744ff2d0a7 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 19 Apr 2023 13:34:19 +0200 Subject: [PATCH 16/17] Fix implication in docs --- .../interpreter/node/expression/builtin/meta/EqualsNode.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java index 776b3efb9bb6..43c180f2b7c3 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java @@ -33,8 +33,8 @@ 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. However `Any.==` implies - `Meta.is_same_object` for all object with the exception of `Number.nan`. + 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 { From 6bae7283fe1ce8276de282dcee37ef45abaa7d89 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 20 Apr 2023 10:32:17 +0200 Subject: [PATCH 17/17] Add more incomparable tests to Vector_Spec --- .../node/expression/builtin/meta/EqualsNode.java | 1 - test/Tests/src/Data/Vector_Spec.enso | 13 +++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java index 43c180f2b7c3..28edcd97d572 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsNode.java @@ -45,7 +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; diff --git a/test/Tests/src/Data/Vector_Spec.enso b/test/Tests/src/Data/Vector_Spec.enso index 526cd79f699f..d18e33628b80 100644 --- a/test/Tests/src/Data/Vector_Spec.enso +++ b/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 @@ -665,6 +675,9 @@ type_spec name alter = Test.group name <| 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]