From dce1b25f2e0db690f7b50eab0098821152d31ec2 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 15 Feb 2024 05:48:25 +0100 Subject: [PATCH 01/27] Enabling equality tests for Complex and Number --- test/Base_Tests/src/Data/Numbers_Spec.enso | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/test/Base_Tests/src/Data/Numbers_Spec.enso b/test/Base_Tests/src/Data/Numbers_Spec.enso index 0c38843bf73d..56f7d70164fd 100644 --- a/test/Base_Tests/src/Data/Numbers_Spec.enso +++ b/test/Base_Tests/src/Data/Numbers_Spec.enso @@ -27,10 +27,6 @@ type Complex < self (that:Complex) = Complex_Comparator.compare self that == Ordering.Less > self (that:Complex) = Complex_Comparator.compare self that == Ordering.Greater - == self (that:Complex) = Complex_Comparator.compare self that == Ordering.Equal - - pending_equality -> Text = "== with conversions isn't yet supported" - Complex.from (that:Number) = Complex.new that type Complex_Comparator @@ -591,7 +587,7 @@ add_specs suite_builder = v2 = (Complex.new 6 6) Ordering.compare v1 v2 . catch Incomparable_Values (_->42) . should_equal 42 - group_builder.specify "Equality of complex and number" pending=Complex.pending_equality <| + group_builder.specify "Equality of complex and number" <| v1 = (Complex.new 3) v2 = 3 v1 . should_equal v2 @@ -599,7 +595,7 @@ add_specs suite_builder = v1==v2 . should_be_true v2==v1 . should_be_true - group_builder.specify "Equality of number and complex" pending=Complex.pending_equality <| + group_builder.specify "Equality of number and complex" <| v1 = 3 v2 = (Complex.new 3) v1 . should_equal v2 From 7c467398eb27e3c7592505c3525fc7ed54a57c68 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 15 Feb 2024 05:51:23 +0100 Subject: [PATCH 02/27] Adding one more node in front of previously existing EqualsNode --- .../builtin/meta/EqualsComplexNode.java | 3 +- .../expression/builtin/meta/EqualsNode.java | 371 +---------------- .../builtin/meta/EqualsSimpleNode.java | 387 ++++++++++++++++++ 3 files changed, 395 insertions(+), 366 deletions(-) create mode 100644 engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsSimpleNode.java diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsComplexNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsComplexNode.java index ff580800227e..b7d6f85ba68d 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsComplexNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsComplexNode.java @@ -419,7 +419,8 @@ boolean equalsGeneric( // all the other guards on purpose. boolean fallbackGuard( Object left, Object right, InteropLibrary interop, WarningsLibrary warnings) { - if (EqualsNode.isPrimitive(left, interop) && EqualsNode.isPrimitive(right, interop)) { + if (EqualsSimpleNode.isPrimitive(left, interop) + && EqualsSimpleNode.isPrimitive(right, interop)) { return false; } if (isJavaObject(left) && isJavaObject(right)) { 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 eec00c5f2c08..be5087862e9b 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 @@ -1,27 +1,13 @@ package org.enso.interpreter.node.expression.builtin.meta; -import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; import com.oracle.truffle.api.dsl.Cached; -import com.oracle.truffle.api.dsl.Cached.Shared; 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.TruffleObject; -import com.oracle.truffle.api.interop.UnsupportedMessageException; import com.oracle.truffle.api.library.CachedLibrary; import com.oracle.truffle.api.nodes.Node; -import java.math.BigInteger; import org.enso.interpreter.dsl.AcceptsError; import org.enso.interpreter.dsl.BuiltinMethod; -import org.enso.interpreter.node.expression.builtin.number.utils.BigIntegerOps; -import org.enso.interpreter.runtime.EnsoContext; -import org.enso.interpreter.runtime.data.EnsoMultiValue; -import org.enso.interpreter.runtime.data.atom.Atom; -import org.enso.interpreter.runtime.data.atom.AtomConstructor; -import org.enso.interpreter.runtime.data.text.Text; -import org.enso.interpreter.runtime.error.WarningsLibrary; -import org.enso.interpreter.runtime.number.EnsoBigInteger; -import org.enso.polyglot.common_utils.Core_Text_Utils; +import org.enso.interpreter.runtime.library.dispatch.TypesLibrary; @BuiltinMethod( type = "Any", @@ -50,357 +36,12 @@ public static EqualsNode build() { public abstract boolean execute(@AcceptsError Object self, @AcceptsError Object right); @Specialization - boolean equalsBoolBool(boolean self, boolean other) { - return self == other; - } - - @Specialization - boolean equalsBoolDouble(boolean self, double other) { - return false; - } - - @Specialization - boolean equalsBoolLong(boolean self, long other) { - return false; - } - - @Specialization - boolean equalsBoolBigInt(boolean self, EnsoBigInteger other) { - return false; - } - - @Specialization - boolean equalsBoolText(boolean self, Text other) { - return false; - } - - @Specialization - boolean equalsBoolInterop( - boolean self, - Object other, - @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop) { - try { - return self == interop.asBoolean(other); - } catch (UnsupportedMessageException ex) { - return false; - } - } - - @Specialization - boolean equalsLongLong(long self, long other) { - return self == other; - } - - @Specialization - boolean equalsLongBool(long self, boolean other) { - return false; - } - - @Specialization - boolean equalsLongDouble(long self, double other) { - return (double) self == other; - } - - @Specialization - boolean equalsLongText(long self, Text other) { - return false; - } - - @Specialization - @TruffleBoundary - boolean equalsLongBigInt(long self, EnsoBigInteger other) { - if (BigIntegerOps.fitsInLong(other.getValue())) { - return BigInteger.valueOf(self).compareTo(other.getValue()) == 0; - } else { - return false; - } - } - - @Specialization - boolean equalsLongInterop( - long self, - Object other, - @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop) { - try { - return self == interop.asLong(other); - } catch (UnsupportedMessageException ex) { - return false; - } - } - - @Specialization - boolean equalsDoubleDouble(double self, double other) { - if (Double.isNaN(self) || Double.isNaN(other)) { - return false; - } else { - return self == other; - } - } - - @Specialization - boolean equalsDoubleLong(double self, long other) { - return self == (double) other; - } - - @Specialization - boolean equalsDoubleBool(double self, boolean other) { - return false; - } - - @Specialization - boolean equalsDoubleText(double self, Text other) { - return false; - } - - @Specialization - boolean equalsDoubleInterop( - double self, - Object other, - @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop) { - try { - if (interop.fitsInDouble(other)) { - return self == interop.asDouble(other); - } - var otherBig = interop.asBigInteger(other); - return self == asDouble(otherBig); - } catch (UnsupportedMessageException ex) { - return false; - } - } - - @TruffleBoundary - private static double asDouble(BigInteger big) { - return big.doubleValue(); - } - - @Specialization(guards = {"isBigInteger(iop, self)", "isBigInteger(iop, other)"}) - @TruffleBoundary - boolean other( - Object self, - Object other, - @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary iop) { - return asBigInteger(iop, self).equals(asBigInteger(iop, other)); - } - - @Specialization(guards = "isBigInteger(iop, self)") - @TruffleBoundary - boolean equalsBigIntDouble( - Object self, - double other, - @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary iop) { - return asBigInteger(iop, self).doubleValue() == other; - } - - @Specialization(guards = "isBigInteger(iop, self)") - @TruffleBoundary - boolean equalsBigIntLong( - Object self, long other, @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary iop) { - var v = asBigInteger(iop, self); - if (BigIntegerOps.fitsInLong(v)) { - return v.compareTo(BigInteger.valueOf(other)) == 0; - } else { - return false; - } - } - - @Specialization - boolean equalsBigIntBool(EnsoBigInteger self, boolean other) { - return false; - } - - @Specialization - boolean equalsBigIntText(EnsoBigInteger self, Text other) { - return false; - } - - @TruffleBoundary - @Specialization(guards = {"isBigInteger(iop, self)", "!isPrimitiveValue(other)"}) - boolean equalsBigIntInterop( + boolean sameTypeCheck( Object self, Object other, - @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary iop) { - try { - var otherBigInteger = InteropLibrary.getUncached().asBigInteger(other); - return asBigInteger(iop, self).equals(otherBigInteger); - } catch (UnsupportedMessageException ex) { - return false; - } - } - - @Specialization(guards = {"selfText.is_normalized()", "otherText.is_normalized()"}) - boolean equalsTextText(Text selfText, Text otherText) { - return selfText.toString().compareTo(otherText.toString()) == 0; - } - - @Specialization - boolean equalsTextBool(Text self, boolean other) { - return false; - } - - @Specialization - boolean equalsTextLong(Text selfText, long otherLong) { - return false; - } - - @Specialization - boolean equalsTextDouble(Text selfText, double otherDouble) { - return false; - } - - @Specialization - boolean equalsTextBigInt(Text self, EnsoBigInteger other) { - return false; - } - - /** - * Compares interop string with other object. If the other object doesn't support conversion to - * String, it is not equal. Otherwise the two strings are compared according to the - * lexicographical order, handling Unicode normalization. See {@code Text_Utils.compare_to}. - */ - @TruffleBoundary - @Specialization( - guards = {"selfInterop.isString(selfString)"}, - limit = "3") - boolean equalsStrings( - Object selfString, - Object otherString, - @CachedLibrary("selfString") InteropLibrary selfInterop, - @CachedLibrary("otherString") InteropLibrary otherInterop) { - String selfJavaString; - String otherJavaString; - try { - selfJavaString = selfInterop.asString(selfString); - otherJavaString = otherInterop.asString(otherString); - } catch (UnsupportedMessageException e) { - return false; - } - return Core_Text_Utils.equals(selfJavaString, otherJavaString); - } - - @Specialization(guards = "isPrimitive(self, interop) != isPrimitive(other, interop)") - boolean equalsDifferent( - Object self, - Object other, - @Shared("interop") @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 equalsAtomNode, - @Shared("isSameObjectNode") @Cached IsSameObjectNode isSameObjectNode) { - return isSameObjectNode.execute(self, other) || equalsAtomNode.execute(self, other); - } - - @Specialization - boolean equalsReverseBoolean( - TruffleObject self, - boolean other, - @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop, - @Shared("reverse") @Cached EqualsNode reverse) { - return reverse.execute(other, self); - } - - @Specialization - boolean equalsReverseLong( - TruffleObject self, - long other, - @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop, - @Shared("reverse") @Cached EqualsNode reverse) { - return reverse.execute(other, self); - } - - @Specialization - boolean equalsReverseDouble( - TruffleObject self, - double other, - @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop, - @Shared("reverse") @Cached EqualsNode reverse) { - return reverse.execute(other, self); - } - - @Specialization - boolean equalsReverseBigInt( - TruffleObject self, - EnsoBigInteger other, - @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop, - @Shared("reverse") @Cached EqualsNode reverse) { - return reverse.execute(other, self); - } - - @Specialization(guards = "isNotPrimitive(self, other, interop, warnings)") - boolean equalsComplex( - Object self, - Object other, - @Cached EqualsComplexNode equalsComplex, - @Shared("isSameObjectNode") @Cached IsSameObjectNode isSameObjectNode, - @Shared("interop") @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 interop, WarningsLibrary warnings) { - if (a instanceof AtomConstructor && b instanceof AtomConstructor) { - return false; - } - if (a instanceof Atom && b instanceof Atom) { - return false; - } - if (warnings.hasWarnings(a) || warnings.hasWarnings(b)) { - return true; - } - if (a instanceof EnsoMultiValue || b instanceof EnsoMultiValue) { - return true; - } - 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}. - */ - static boolean isPrimitive(Object object, InteropLibrary interop) { - return isPrimitiveValue(object) - || object instanceof EnsoBigInteger - || object instanceof Text - || interop.isString(object) - || interop.isNumber(object) - || interop.isBoolean(object); - } - - static boolean isPrimitiveValue(Object object) { - return object instanceof Boolean || object instanceof Long || object instanceof Double; - } - - static boolean isBigInteger(InteropLibrary iop, Object v) { - if (v instanceof EnsoBigInteger) { - return true; - } else { - return !iop.fitsInDouble(v) && !iop.fitsInLong(v) && iop.fitsInBigInteger(v); - } - } - - BigInteger asBigInteger(InteropLibrary iop, Object v) { - if (v instanceof EnsoBigInteger big) { - return big.getValue(); - } else { - try { - return iop.asBigInteger(v); - } catch (UnsupportedMessageException ex) { - throw EnsoContext.get(this).raiseAssertionPanic(this, "Expecting BigInteger", ex); - } - } + @CachedLibrary(limit = "3") TypesLibrary types, + @Cached EqualsSimpleNode node) { + var areEqual = node.execute(self, other); + return areEqual; } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsSimpleNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsSimpleNode.java new file mode 100644 index 000000000000..d684e4de8e75 --- /dev/null +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsSimpleNode.java @@ -0,0 +1,387 @@ +package org.enso.interpreter.node.expression.builtin.meta; + +import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; +import com.oracle.truffle.api.dsl.Cached; +import com.oracle.truffle.api.dsl.Cached.Shared; +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.TruffleObject; +import com.oracle.truffle.api.interop.UnsupportedMessageException; +import com.oracle.truffle.api.library.CachedLibrary; +import com.oracle.truffle.api.nodes.Node; +import java.math.BigInteger; +import org.enso.interpreter.node.expression.builtin.number.utils.BigIntegerOps; +import org.enso.interpreter.runtime.EnsoContext; +import org.enso.interpreter.runtime.data.EnsoMultiValue; +import org.enso.interpreter.runtime.data.atom.Atom; +import org.enso.interpreter.runtime.data.atom.AtomConstructor; +import org.enso.interpreter.runtime.data.text.Text; +import org.enso.interpreter.runtime.error.WarningsLibrary; +import org.enso.interpreter.runtime.number.EnsoBigInteger; +import org.enso.polyglot.common_utils.Core_Text_Utils; + +@GenerateUncached +public abstract class EqualsSimpleNode extends Node { + + public static EqualsSimpleNode build() { + return EqualsSimpleNodeGen.create(); + } + + public abstract boolean execute(Object self, Object right); + + @Specialization + boolean equalsBoolBool(boolean self, boolean other) { + return self == other; + } + + @Specialization + boolean equalsBoolDouble(boolean self, double other) { + return false; + } + + @Specialization + boolean equalsBoolLong(boolean self, long other) { + return false; + } + + @Specialization + boolean equalsBoolBigInt(boolean self, EnsoBigInteger other) { + return false; + } + + @Specialization + boolean equalsBoolText(boolean self, Text other) { + return false; + } + + @Specialization + boolean equalsBoolInterop( + boolean self, + Object other, + @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop) { + try { + return self == interop.asBoolean(other); + } catch (UnsupportedMessageException ex) { + return false; + } + } + + @Specialization + boolean equalsLongLong(long self, long other) { + return self == other; + } + + @Specialization + boolean equalsLongBool(long self, boolean other) { + return false; + } + + @Specialization + boolean equalsLongDouble(long self, double other) { + return (double) self == other; + } + + @Specialization + boolean equalsLongText(long self, Text other) { + return false; + } + + @Specialization + @TruffleBoundary + boolean equalsLongBigInt(long self, EnsoBigInteger other) { + if (BigIntegerOps.fitsInLong(other.getValue())) { + return BigInteger.valueOf(self).compareTo(other.getValue()) == 0; + } else { + return false; + } + } + + @Specialization + boolean equalsLongInterop( + long self, + Object other, + @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop) { + try { + return self == interop.asLong(other); + } catch (UnsupportedMessageException ex) { + return false; + } + } + + @Specialization + boolean equalsDoubleDouble(double self, double other) { + if (Double.isNaN(self) || Double.isNaN(other)) { + return false; + } else { + return self == other; + } + } + + @Specialization + boolean equalsDoubleLong(double self, long other) { + return self == (double) other; + } + + @Specialization + boolean equalsDoubleBool(double self, boolean other) { + return false; + } + + @Specialization + boolean equalsDoubleText(double self, Text other) { + return false; + } + + @Specialization + boolean equalsDoubleInterop( + double self, + Object other, + @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop) { + try { + if (interop.fitsInDouble(other)) { + return self == interop.asDouble(other); + } + var otherBig = interop.asBigInteger(other); + return self == asDouble(otherBig); + } catch (UnsupportedMessageException ex) { + return false; + } + } + + @TruffleBoundary + private static double asDouble(BigInteger big) { + return big.doubleValue(); + } + + @Specialization(guards = {"isBigInteger(iop, self)", "isBigInteger(iop, other)"}) + @TruffleBoundary + boolean other( + Object self, + Object other, + @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary iop) { + return asBigInteger(iop, self).equals(asBigInteger(iop, other)); + } + + @Specialization(guards = "isBigInteger(iop, self)") + @TruffleBoundary + boolean equalsBigIntDouble( + Object self, + double other, + @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary iop) { + return asBigInteger(iop, self).doubleValue() == other; + } + + @Specialization(guards = "isBigInteger(iop, self)") + @TruffleBoundary + boolean equalsBigIntLong( + Object self, long other, @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary iop) { + var v = asBigInteger(iop, self); + if (BigIntegerOps.fitsInLong(v)) { + return v.compareTo(BigInteger.valueOf(other)) == 0; + } else { + return false; + } + } + + @Specialization + boolean equalsBigIntBool(EnsoBigInteger self, boolean other) { + return false; + } + + @Specialization + boolean equalsBigIntText(EnsoBigInteger self, Text other) { + return false; + } + + @TruffleBoundary + @Specialization(guards = {"isBigInteger(iop, self)", "!isPrimitiveValue(other)"}) + boolean equalsBigIntInterop( + Object self, + Object other, + @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary iop) { + try { + var otherBigInteger = InteropLibrary.getUncached().asBigInteger(other); + return asBigInteger(iop, self).equals(otherBigInteger); + } catch (UnsupportedMessageException ex) { + return false; + } + } + + @Specialization(guards = {"selfText.is_normalized()", "otherText.is_normalized()"}) + boolean equalsTextText(Text selfText, Text otherText) { + return selfText.toString().compareTo(otherText.toString()) == 0; + } + + @Specialization + boolean equalsTextBool(Text self, boolean other) { + return false; + } + + @Specialization + boolean equalsTextLong(Text selfText, long otherLong) { + return false; + } + + @Specialization + boolean equalsTextDouble(Text selfText, double otherDouble) { + return false; + } + + @Specialization + boolean equalsTextBigInt(Text self, EnsoBigInteger other) { + return false; + } + + /** + * Compares interop string with other object. If the other object doesn't support conversion to + * String, it is not equal. Otherwise the two strings are compared according to the + * lexicographical order, handling Unicode normalization. See {@code Text_Utils.compare_to}. + */ + @TruffleBoundary + @Specialization( + guards = {"selfInterop.isString(selfString)"}, + limit = "3") + boolean equalsStrings( + Object selfString, + Object otherString, + @CachedLibrary("selfString") InteropLibrary selfInterop, + @CachedLibrary("otherString") InteropLibrary otherInterop) { + String selfJavaString; + String otherJavaString; + try { + selfJavaString = selfInterop.asString(selfString); + otherJavaString = otherInterop.asString(otherString); + } catch (UnsupportedMessageException e) { + return false; + } + return Core_Text_Utils.equals(selfJavaString, otherJavaString); + } + + @Specialization(guards = "isPrimitive(self, interop) != isPrimitive(other, interop)") + boolean equalsDifferent( + Object self, + Object other, + @Shared("interop") @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 equalsAtomNode, + @Shared("isSameObjectNode") @Cached IsSameObjectNode isSameObjectNode) { + return isSameObjectNode.execute(self, other) || equalsAtomNode.execute(self, other); + } + + @Specialization + boolean equalsReverseBoolean( + TruffleObject self, + boolean other, + @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop, + @Shared("reverse") @Cached EqualsNode reverse) { + return reverse.execute(other, self); + } + + @Specialization + boolean equalsReverseLong( + TruffleObject self, + long other, + @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop, + @Shared("reverse") @Cached EqualsNode reverse) { + return reverse.execute(other, self); + } + + @Specialization + boolean equalsReverseDouble( + TruffleObject self, + double other, + @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop, + @Shared("reverse") @Cached EqualsNode reverse) { + return reverse.execute(other, self); + } + + @Specialization + boolean equalsReverseBigInt( + TruffleObject self, + EnsoBigInteger other, + @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop, + @Shared("reverse") @Cached EqualsNode reverse) { + return reverse.execute(other, self); + } + + @Specialization(guards = "isNotPrimitive(self, other, interop, warnings)") + boolean equalsComplex( + Object self, + Object other, + @Cached EqualsComplexNode equalsComplex, + @Shared("isSameObjectNode") @Cached IsSameObjectNode isSameObjectNode, + @Shared("interop") @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 interop, WarningsLibrary warnings) { + if (a instanceof AtomConstructor && b instanceof AtomConstructor) { + return false; + } + if (a instanceof Atom && b instanceof Atom) { + return false; + } + if (warnings.hasWarnings(a) || warnings.hasWarnings(b)) { + return true; + } + if (a instanceof EnsoMultiValue || b instanceof EnsoMultiValue) { + return true; + } + 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}. + */ + static boolean isPrimitive(Object object, InteropLibrary interop) { + return isPrimitiveValue(object) + || object instanceof EnsoBigInteger + || object instanceof Text + || interop.isString(object) + || interop.isNumber(object) + || interop.isBoolean(object); + } + + static boolean isPrimitiveValue(Object object) { + return object instanceof Boolean || object instanceof Long || object instanceof Double; + } + + static boolean isBigInteger(InteropLibrary iop, Object v) { + if (v instanceof EnsoBigInteger) { + return true; + } else { + return !iop.fitsInDouble(v) && !iop.fitsInLong(v) && iop.fitsInBigInteger(v); + } + } + + BigInteger asBigInteger(InteropLibrary iop, Object v) { + if (v instanceof EnsoBigInteger big) { + return big.getValue(); + } else { + try { + return iop.asBigInteger(v); + } catch (UnsupportedMessageException ex) { + throw EnsoContext.get(this).raiseAssertionPanic(this, "Expecting BigInteger", ex); + } + } + } +} From b0a4e4846e5788d1d4d67b71a5021fc52155d1e9 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 15 Feb 2024 09:05:34 +0100 Subject: [PATCH 03/27] First of all compute results only then verify them --- test/Base_Tests/src/Data/Numbers_Spec.enso | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/Base_Tests/src/Data/Numbers_Spec.enso b/test/Base_Tests/src/Data/Numbers_Spec.enso index 56f7d70164fd..01809503351d 100644 --- a/test/Base_Tests/src/Data/Numbers_Spec.enso +++ b/test/Base_Tests/src/Data/Numbers_Spec.enso @@ -590,18 +590,22 @@ add_specs suite_builder = group_builder.specify "Equality of complex and number" <| v1 = (Complex.new 3) v2 = 3 + r1 = v1==v2 + r2 = v2==v1 + r1 . should_be_true + r2 . should_be_true v1 . should_equal v2 v2 . should_equal v1 - v1==v2 . should_be_true - v2==v1 . should_be_true group_builder.specify "Equality of number and complex" <| v1 = 3 v2 = (Complex.new 3) + r1 = v1==v2 + r2 = v2==v1 + r1 . should_be_true + r2 . should_be_true v1 . should_equal v2 v2 . should_equal v1 - v1==v2 . should_be_true - v2==v1 . should_be_true group_builder.specify "Greater or equal of complex and complex" <| v1 = (Complex.new 3) From 9f4cde3d9f620d31820f736a6796d4e9990ea168 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 15 Feb 2024 09:06:21 +0100 Subject: [PATCH 04/27] Number can be equal to different type. Check with == --- distribution/lib/Standard/Test/0.0.0-dev/src/Extensions.enso | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/distribution/lib/Standard/Test/0.0.0-dev/src/Extensions.enso b/distribution/lib/Standard/Test/0.0.0-dev/src/Extensions.enso index 1c7dd5f4076a..b0a4700ac1c0 100644 --- a/distribution/lib/Standard/Test/0.0.0-dev/src/Extensions.enso +++ b/distribution/lib/Standard/Test/0.0.0-dev/src/Extensions.enso @@ -296,7 +296,7 @@ Number.should_equal : Float -> Float -> Integer -> Spec_Result Number.should_equal self that epsilon=0 frames_to_skip=0 = matches = case that of _ : Number -> self.equals that epsilon - _ -> False + _ -> self==that case matches of True -> Spec_Result.Success False -> @@ -519,7 +519,7 @@ Error.should_contain_the_same_elements_as self that frames_to_skip=0 = It checks that all elements from `self` are also present in `that`. It does not require that all elements of `that` are contained in `self`. Arities of - elements are not checked, so `self` may still contain more elements than + elements are not checked, so `self` may still contain more elements than `that` by containing duplicates. It will work on any collection which supports the methods From a214c0b8d14d74b74852cf2410099669e26f4e41 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 15 Feb 2024 09:07:01 +0100 Subject: [PATCH 05/27] Supporting conversions during evaluation of == --- .../org/enso/interpreter/test/EqualsTest.java | 78 ++++---- .../enso/interpreter/test/HashCodeTest.java | 4 +- .../enso/interpreter/caches/CacheUtils.java | 52 +++-- .../interpreter/caches/ImportExportCache.java | 2 +- .../enso/interpreter/caches/ModuleCache.java | 2 +- .../builtin/meta/EqualsAtomNode.java | 18 +- .../builtin/meta/EqualsComplexNode.java | 32 +-- .../expression/builtin/meta/EqualsNode.java | 184 ++++++++++++++++-- .../builtin/meta/EqualsSimpleNode.java | 33 ++-- .../builtin/ordering/SortArrayNode.java | 12 +- .../builtin/ordering/SortVectorNode.java | 22 ++- .../runtime/data/hash/EnsoHashMap.java | 9 +- .../runtime/data/hash/EnsoHashMapBuilder.java | 39 ++-- .../runtime/data/hash/HashMapInsertNode.java | 18 +- .../runtime/data/hash/HashMapRemoveNode.java | 16 +- 15 files changed, 375 insertions(+), 146 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsTest.java index fa9c8803e0bd..6e139d044969 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsTest.java @@ -16,7 +16,6 @@ import java.util.stream.Collectors; import org.enso.interpreter.node.expression.builtin.interop.syntax.HostValueToEnsoNode; import org.enso.interpreter.node.expression.builtin.meta.EqualsNode; -import org.enso.interpreter.node.expression.builtin.meta.EqualsNodeGen; import org.enso.interpreter.runtime.callable.UnresolvedConversion; import org.enso.interpreter.runtime.number.EnsoBigInteger; import org.enso.polyglot.MethodNames; @@ -45,7 +44,7 @@ public static void initContextAndData() { context, () -> { testRootNode = new TestRootNode(); - equalsNode = EqualsNode.build(); + equalsNode = EqualsNode.create(); hostValueToEnsoNode = HostValueToEnsoNode.build(); testRootNode.insertChildren(equalsNode, hostValueToEnsoNode); return null; @@ -93,8 +92,8 @@ public void equalsOperatorShouldBeSymmetric(Object firstValue, Object secondValu executeInContext( context, () -> { - boolean firstResult = equalsNode.execute(firstValue, secondValue); - boolean secondResult = equalsNode.execute(secondValue, firstValue); + boolean firstResult = equalsNode.execute(null, firstValue, secondValue); + boolean secondResult = equalsNode.execute(null, secondValue, firstValue); assertEquals("equals should be symmetric", firstResult, secondResult); return null; }); @@ -105,8 +104,8 @@ public void equalsOperatorShouldBeConsistent(Object value) { executeInContext( context, () -> { - Object firstResult = equalsNode.execute(value, value); - Object secondResult = equalsNode.execute(value, value); + Object firstResult = equalsNode.execute(null, value, value); + Object secondResult = equalsNode.execute(null, value, value); assertEquals("equals should be consistent", firstResult, secondResult); return null; }); @@ -117,8 +116,8 @@ public void equalsNodeCachedIsConsistentWithUncached(Object firstVal, Object sec executeInContext( context, () -> { - Object uncachedRes = EqualsNodeGen.getUncached().execute(firstVal, secondVal); - Object cachedRes = equalsNode.execute(firstVal, secondVal); + Object uncachedRes = EqualsNode.getUncached().execute(null, firstVal, secondVal); + Object cachedRes = equalsNode.execute(null, firstVal, secondVal); assertEquals( "Result from uncached EqualsNode should be the same as result from its cached" + " variant", @@ -140,7 +139,7 @@ public void testDateEquality() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(ensoDate, javaDate)); + assertTrue(equalsNode.execute(null, ensoDate, javaDate)); return null; }); } @@ -158,7 +157,7 @@ public void testTimeEquality() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(ensoTime, javaDate)); + assertTrue(equalsNode.execute(null, ensoTime, javaDate)); return null; }); } @@ -181,7 +180,7 @@ public void testDateTimeEquality() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(ensoDateTime, javaDateTime)); + assertTrue(equalsNode.execute(null, ensoDateTime, javaDateTime)); return null; }); } @@ -194,7 +193,8 @@ public void testDoubleEqualsEnsoBigInteger() { executeInContext( context, () -> { - assertTrue(javaNumber + " == " + ensoNumber, equalsNode.execute(javaNumber, ensoNumber)); + assertTrue( + javaNumber + " == " + ensoNumber, equalsNode.execute(null, javaNumber, ensoNumber)); return null; }); } @@ -207,7 +207,8 @@ public void testEnsoBigIntegerEqualsDoubleEquals() { executeInContext( context, () -> { - assertTrue(ensoNumber + " == " + javaNumber, equalsNode.execute(ensoNumber, javaNumber)); + assertTrue( + ensoNumber + " == " + javaNumber, equalsNode.execute(null, ensoNumber, javaNumber)); return null; }); } @@ -220,7 +221,8 @@ public void testDoubleEqualsJavaBigInteger() { executeInContext( context, () -> { - assertTrue(javaNumber + " == " + hostNumber, equalsNode.execute(javaNumber, hostNumber)); + assertTrue( + javaNumber + " == " + hostNumber, equalsNode.execute(null, javaNumber, hostNumber)); return null; }); } @@ -233,7 +235,8 @@ public void testJavaBigIntegerEqualsDoubleEquals() { executeInContext( context, () -> { - assertTrue(hostNumber + " == " + javaNumber, equalsNode.execute(hostNumber, javaNumber)); + assertTrue( + hostNumber + " == " + javaNumber, equalsNode.execute(null, hostNumber, javaNumber)); return null; }); } @@ -246,7 +249,7 @@ public void testVectorsEquality() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(ensoVector, javaVector)); + assertTrue(equalsNode.execute(null, ensoVector, javaVector)); return null; }); } @@ -258,9 +261,9 @@ public void testTruffleNumberLong() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(ensoNumber, foreignNumber.asDirect())); - assertTrue(equalsNode.execute(ensoNumber, foreignNumber)); - assertTrue(equalsNode.execute(foreignNumber, ensoNumber)); + assertTrue(equalsNode.execute(null, ensoNumber, foreignNumber.asDirect())); + assertTrue(equalsNode.execute(null, ensoNumber, foreignNumber)); + assertTrue(equalsNode.execute(null, foreignNumber, ensoNumber)); return null; }); } @@ -272,9 +275,9 @@ public void testTruffleNumberDouble() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(ensoNumber, foreignNumber.asDirect())); - assertTrue(equalsNode.execute(ensoNumber, foreignNumber)); - assertTrue(equalsNode.execute(foreignNumber, ensoNumber)); + assertTrue(equalsNode.execute(null, ensoNumber, foreignNumber.asDirect())); + assertTrue(equalsNode.execute(null, ensoNumber, foreignNumber)); + assertTrue(equalsNode.execute(null, foreignNumber, ensoNumber)); return null; }); } @@ -287,8 +290,8 @@ public void testTruffleNumberBigInt() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(ensoNumber, foreignNumber)); - assertTrue(equalsNode.execute(foreignNumber, ensoNumber)); + assertTrue(equalsNode.execute(null, ensoNumber, foreignNumber)); + assertTrue(equalsNode.execute(null, foreignNumber, ensoNumber)); return null; }); } @@ -301,9 +304,9 @@ public void testTruffleBoolean() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(ensoBoolean, foreignBoolean.asDirect())); - assertTrue(equalsNode.execute(ensoBoolean, foreignBoolean)); - assertTrue(equalsNode.execute(foreignBoolean, ensoBoolean)); + assertTrue(equalsNode.execute(null, ensoBoolean, foreignBoolean.asDirect())); + assertTrue(equalsNode.execute(null, ensoBoolean, foreignBoolean)); + assertTrue(equalsNode.execute(null, foreignBoolean, ensoBoolean)); return null; }); } @@ -315,9 +318,9 @@ public void testTruffleString() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(ensoText, foreignString.asDirect())); - assertTrue(equalsNode.execute(ensoText, foreignString)); - assertTrue(equalsNode.execute(foreignString, ensoText)); + assertTrue(equalsNode.execute(null, ensoText, foreignString.asDirect())); + assertTrue(equalsNode.execute(null, ensoText, foreignString)); + assertTrue(equalsNode.execute(null, foreignString, ensoText)); return null; }); } @@ -336,8 +339,8 @@ public void testTruffleNumberPlus() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(142L, hundred42)); - assertTrue(equalsNode.execute(hundred42, 142L)); + assertTrue(equalsNode.execute(null, 142L, hundred42)); + assertTrue(equalsNode.execute(null, hundred42, 142L)); return null; }); } @@ -376,14 +379,17 @@ && unwrapValue(context, mod2) instanceof org.enso.interpreter.runtime.Module m2) context, () -> { assertTrue( - "Conversions from same module are the same", equalsNode.execute(conv1, conv1_2)); + "Conversions from same module are the same", + equalsNode.execute(null, conv1, conv1_2)); assertTrue( - "Conversions from same module are the same", equalsNode.execute(conv2, conv2_2)); + "Conversions from same module are the same", + equalsNode.execute(null, conv2, conv2_2)); assertFalse( - "Conversions from other modules aren't the same", equalsNode.execute(conv1, conv2)); + "Conversions from other modules aren't the same", + equalsNode.execute(null, conv1, conv2)); assertFalse( "Conversions from other modueles aren't the same", - equalsNode.execute(conv2_2, conv1_2)); + equalsNode.execute(null, conv2_2, conv1_2)); return null; }); diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/HashCodeTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/HashCodeTest.java index ade0d7fda03f..cfe04819640e 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/HashCodeTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/HashCodeTest.java @@ -38,7 +38,7 @@ public static void initContextAndData() { context, () -> { hashCodeNode = HashCodeNode.build(); - equalsNode = EqualsNode.build(); + equalsNode = EqualsNode.create(); hostValueToEnsoNode = HostValueToEnsoNode.build(); testRootNode = new TestRootNode(); testRootNode.insertChildren(hashCodeNode, equalsNode, hostValueToEnsoNode); @@ -94,7 +94,7 @@ public void hashCodeContractTheory(Object firstValue, Object secondValue) { () -> { long firstHash = hashCodeNode.execute(firstValue); long secondHash = hashCodeNode.execute(secondValue); - Object valuesAreEqual = equalsNode.execute(firstValue, secondValue); + Object valuesAreEqual = equalsNode.execute(null, firstValue, secondValue); // if o1 == o2 then hash(o1) == hash(o2) if (isTrue(valuesAreEqual)) { assertEquals( diff --git a/engine/runtime/src/main/java/org/enso/interpreter/caches/CacheUtils.java b/engine/runtime/src/main/java/org/enso/interpreter/caches/CacheUtils.java index e14818113729..f90f4edfe204 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/caches/CacheUtils.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/caches/CacheUtils.java @@ -1,5 +1,6 @@ package org.enso.interpreter.caches; +import com.oracle.truffle.api.TruffleFile; import java.io.IOException; import java.nio.ByteBuffer; import java.security.MessageDigest; @@ -8,40 +9,38 @@ import java.util.List; import java.util.UUID; import java.util.function.Function; - import org.enso.compiler.context.CompilerContext; import org.enso.compiler.core.ir.ProcessingPass; import org.enso.pkg.SourceFile; import org.enso.text.Hex; -import com.oracle.truffle.api.TruffleFile; - final class CacheUtils { - private CacheUtils() { - } + private CacheUtils() {} - static Function writeReplace(CompilerContext context) { - return (obj) -> switch (obj) { - case ProcessingPass.Metadata metadata -> metadata.prepareForSerialization(context); - case UUID _ -> null; - case null -> null; - default -> obj; - }; + static Function writeReplace(CompilerContext context, boolean keepUUIDs) { + return (obj) -> + switch (obj) { + case ProcessingPass.Metadata metadata -> metadata.prepareForSerialization(context); + case UUID id -> keepUUIDs ? id : null; + case null -> null; + default -> obj; + }; } static Function readResolve(CompilerContext context) { - return (obj) -> switch (obj) { - case ProcessingPass.Metadata metadata -> { - var option = metadata.restoreFromSerialization(context); - if (option.nonEmpty()) { - yield option.get(); - } else { - throw raise(RuntimeException.class, new IOException("Cannot convert " + metadata)); - } - } - case null -> null; - default -> obj; - }; + return (obj) -> + switch (obj) { + case ProcessingPass.Metadata metadata -> { + var option = metadata.restoreFromSerialization(context); + if (option.nonEmpty()) { + yield option.get(); + } else { + throw raise(RuntimeException.class, new IOException("Cannot convert " + metadata)); + } + } + case null -> null; + default -> obj; + }; } /** @@ -75,9 +74,7 @@ static String computeDigestFromBytes(ByteBuffer bytes) { * @param pkgSources the list of package sources * @return string representation of bytes' hash */ - static final String computeDigestOfLibrarySources( - List> pkgSources - ) { + static final String computeDigestOfLibrarySources(List> pkgSources) { pkgSources.sort(Comparator.comparing(o -> o.qualifiedName().toString())); try { @@ -95,5 +92,4 @@ static final String computeDigestOfLibrarySources( static T raise(Class cls, Exception e) throws T { throw (T) e; } - } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/caches/ImportExportCache.java b/engine/runtime/src/main/java/org/enso/interpreter/caches/ImportExportCache.java index 526714353a78..7144728b00f1 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/caches/ImportExportCache.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/caches/ImportExportCache.java @@ -65,7 +65,7 @@ public byte[] metadata(String sourceDigest, String blobDigest, CachedBindings en public byte[] serialize(EnsoContext context, CachedBindings entry) throws IOException { var arr = Persistance.write( - entry.bindings(), CacheUtils.writeReplace(context.getCompiler().context())); + entry.bindings(), CacheUtils.writeReplace(context.getCompiler().context(), false)); return arr; } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/caches/ModuleCache.java b/engine/runtime/src/main/java/org/enso/interpreter/caches/ModuleCache.java index e65bb3b24476..d32c446ec29d 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/caches/ModuleCache.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/caches/ModuleCache.java @@ -60,7 +60,7 @@ public byte[] metadata(String sourceDigest, String blobDigest, CachedModule entr public byte[] serialize(EnsoContext context, CachedModule entry) throws IOException { var arr = Persistance.write( - entry.moduleIR(), CacheUtils.writeReplace(context.getCompiler().context())); + entry.moduleIR(), CacheUtils.writeReplace(context.getCompiler().context(), true)); return arr; } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsAtomNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsAtomNode.java index c65921194ab3..4c4615a0eded 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsAtomNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsAtomNode.java @@ -7,6 +7,8 @@ import com.oracle.truffle.api.dsl.Cached.Shared; import com.oracle.truffle.api.dsl.GenerateUncached; import com.oracle.truffle.api.dsl.Specialization; +import com.oracle.truffle.api.frame.MaterializedFrame; +import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.library.CachedLibrary; import com.oracle.truffle.api.nodes.ExplodeLoop; import com.oracle.truffle.api.nodes.Node; @@ -32,7 +34,7 @@ public static EqualsAtomNode build() { return EqualsAtomNodeGen.create(); } - public abstract boolean execute(Atom left, Atom right); + public abstract boolean execute(VirtualFrame frame, Atom left, Atom right); static EqualsNode[] createEqualsNodes(int size) { EqualsNode[] nodes = new EqualsNode[size]; @@ -48,6 +50,7 @@ static EqualsNode[] createEqualsNodes(int size) { limit = "10") @ExplodeLoop boolean equalsAtomsWithDefaultComparator( + VirtualFrame frame, Atom self, Atom other, @Cached("self.getConstructor()") AtomConstructor selfCtorCached, @@ -65,7 +68,7 @@ boolean equalsAtomsWithDefaultComparator( for (int i = 0; i < fieldsLenCached; i++) { var selfValue = structsLib.getField(self, i); var otherValue = structsLib.getField(other, i); - var fieldsAreEqual = fieldEqualsNodes[i].execute(selfValue, otherValue); + var fieldsAreEqual = fieldEqualsNodes[i].execute(frame, selfValue, otherValue); if (!fieldsAreEqual) { return false; } @@ -116,13 +119,18 @@ private static boolean orderingOrNull(Node where, EnsoContext ctx, Object obj, F } } - @CompilerDirectives.TruffleBoundary @Specialization( replaces = {"equalsAtomsWithDefaultComparator", "equalsAtomsWithCustomComparator"}) - boolean equalsAtomsUncached(Atom self, Atom other) { + boolean equalsAtomsUncached(VirtualFrame frame, Atom self, Atom other) { if (self.getConstructor() != other.getConstructor()) { return false; + } else { + return equalsAtomsUncached(frame.materialize(), self, other); } + } + + @CompilerDirectives.TruffleBoundary + private boolean equalsAtomsUncached(MaterializedFrame frame, Atom self, Atom other) { Type customComparator = CustomComparatorNode.getUncached().execute(self); if (customComparator != null) { Function compareFunc = findCompareMethod(customComparator); @@ -139,7 +147,7 @@ boolean equalsAtomsUncached(Atom self, Atom other) { for (int i = 0; i < self.getConstructor().getArity(); i++) { var selfField = StructsLibrary.getUncached().getField(self, i); var otherField = StructsLibrary.getUncached().getField(other, i); - boolean areFieldsSame = EqualsNodeGen.getUncached().execute(selfField, otherField); + boolean areFieldsSame = EqualsNode.getUncached().execute(frame, selfField, otherField); if (!areFieldsSame) { return false; } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsComplexNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsComplexNode.java index b7d6f85ba68d..bd1487d5c6a5 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsComplexNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsComplexNode.java @@ -6,6 +6,7 @@ import com.oracle.truffle.api.dsl.Cached.Shared; import com.oracle.truffle.api.dsl.GenerateUncached; import com.oracle.truffle.api.dsl.Specialization; +import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.interop.ArityException; import com.oracle.truffle.api.interop.InteropLibrary; import com.oracle.truffle.api.interop.InvalidArrayIndexException; @@ -18,7 +19,6 @@ import com.oracle.truffle.api.nodes.Node; import java.time.LocalDateTime; import java.time.ZonedDateTime; -import org.enso.interpreter.dsl.AcceptsError; import org.enso.interpreter.node.expression.builtin.interop.syntax.HostValueToEnsoNode; import org.enso.interpreter.node.expression.builtin.ordering.CustomComparatorNode; import org.enso.interpreter.runtime.EnsoContext; @@ -39,24 +39,26 @@ public static EqualsComplexNode build() { return EqualsComplexNodeGen.create(); } - public abstract boolean execute(@AcceptsError Object left, @AcceptsError Object right); + public abstract boolean execute(VirtualFrame frame, Object left, Object right); /** Enso specific types */ @Specialization boolean equalsUnresolvedSymbols( + VirtualFrame frame, UnresolvedSymbol self, UnresolvedSymbol otherSymbol, @Shared("equalsNode") @Cached EqualsNode equalsNode) { return self.getName().equals(otherSymbol.getName()) - && equalsNode.execute(self.getScope(), otherSymbol.getScope()); + && equalsNode.execute(frame, self.getScope(), otherSymbol.getScope()); } @Specialization boolean equalsUnresolvedConversion( + VirtualFrame frame, UnresolvedConversion selfConversion, UnresolvedConversion otherConversion, @Shared("equalsNode") @Cached EqualsNode equalsNode) { - return equalsNode.execute(selfConversion.getScope(), otherConversion.getScope()); + return equalsNode.execute(frame, selfConversion.getScope(), otherConversion.getScope()); } @Specialization @@ -71,8 +73,11 @@ boolean equalsModules(Module selfModule, Module otherModule) { @Specialization boolean equalsFiles( - EnsoFile selfFile, EnsoFile otherFile, @Shared("equalsNode") @Cached EqualsNode equalsNode) { - return equalsNode.execute(selfFile.getPath(), otherFile.getPath()); + VirtualFrame frame, + EnsoFile selfFile, + EnsoFile otherFile, + @Shared("equalsNode") @Cached EqualsNode equalsNode) { + return equalsNode.execute(frame, selfFile.getPath(), otherFile.getPath()); } /** @@ -82,12 +87,13 @@ boolean equalsFiles( */ @Specialization(guards = {"typesLib.hasType(selfType)", "typesLib.hasType(otherType)"}) boolean equalsTypes( + VirtualFrame frame, Type selfType, Type otherType, @Shared("equalsNode") @Cached EqualsNode equalsNode, @Shared("typesLib") @CachedLibrary(limit = "10") TypesLibrary typesLib) { return equalsNode.execute( - selfType.getQualifiedName().toString(), otherType.getQualifiedName().toString()); + frame, selfType.getQualifiedName().toString(), otherType.getQualifiedName().toString()); } /** @@ -99,6 +105,7 @@ boolean equalsTypes( }, limit = "3") boolean equalsWithWarnings( + VirtualFrame frame, Object selfWithWarnings, Object otherWithWarnings, @CachedLibrary("selfWithWarnings") WarningsLibrary selfWarnLib, @@ -113,7 +120,7 @@ boolean equalsWithWarnings( otherWarnLib.hasWarnings(otherWithWarnings) ? otherWarnLib.removeWarnings(otherWithWarnings) : otherWithWarnings; - return equalsNode.execute(self, other); + return equalsNode.execute(frame, self, other); } catch (UnsupportedMessageException e) { throw EnsoContext.get(this).raiseAssertionPanic(this, null, e); } @@ -278,6 +285,7 @@ boolean equalsDuration( }, limit = "3") boolean equalsArrays( + VirtualFrame frame, Object selfArray, Object otherArray, @CachedLibrary("selfArray") InteropLibrary selfInterop, @@ -293,7 +301,7 @@ boolean equalsArrays( for (long i = 0; i < selfSize; i++) { Object selfElem = valueToEnsoNode.execute(selfInterop.readArrayElement(selfArray, i)); Object otherElem = valueToEnsoNode.execute(otherInterop.readArrayElement(otherArray, i)); - boolean elemsAreEqual = equalsNode.execute(selfElem, otherElem); + boolean elemsAreEqual = equalsNode.execute(frame, selfElem, otherElem); if (!elemsAreEqual) { return false; } @@ -313,6 +321,7 @@ boolean equalsArrays( }, limit = "3") boolean equalsHashMaps( + VirtualFrame frame, Object selfHashMap, Object otherHashMap, @CachedLibrary("selfHashMap") InteropLibrary selfInterop, @@ -337,7 +346,7 @@ boolean equalsHashMaps( && otherInterop.isHashEntryReadable(otherHashMap, key)) { Object otherValue = valueToEnsoNode.execute(otherInterop.readHashValue(otherHashMap, key)); - if (!equalsNode.execute(selfValue, otherValue)) { + if (!equalsNode.execute(frame, selfValue, otherValue)) { return false; } } else { @@ -391,13 +400,14 @@ boolean equalsHostObjects( // It has well-defined equality based on the qualified name. @Specialization(guards = {"isJavaFunction(selfHostFunc)", "isJavaFunction(otherHostFunc)"}) boolean equalsHostFunctions( + VirtualFrame frame, Object selfHostFunc, Object otherHostFunc, @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop, @Shared("equalsNode") @Cached EqualsNode equalsNode) { Object selfFuncStrRepr = interop.toDisplayString(selfHostFunc); Object otherFuncStrRepr = interop.toDisplayString(otherHostFunc); - return equalsNode.execute(selfFuncStrRepr, otherFuncStrRepr); + return equalsNode.execute(frame, selfFuncStrRepr, otherFuncStrRepr); } @Specialization(guards = "fallbackGuard(left, right, interop, warningsLib)") 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 be5087862e9b..862aa3bccf48 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 @@ -1,13 +1,24 @@ package org.enso.interpreter.node.expression.builtin.meta; +import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.dsl.Cached; +import com.oracle.truffle.api.dsl.Cached.Shared; import com.oracle.truffle.api.dsl.GenerateUncached; +import com.oracle.truffle.api.dsl.NeverDefault; import com.oracle.truffle.api.dsl.Specialization; +import com.oracle.truffle.api.frame.VirtualFrame; +import com.oracle.truffle.api.interop.ArityException; import com.oracle.truffle.api.library.CachedLibrary; import com.oracle.truffle.api.nodes.Node; import org.enso.interpreter.dsl.AcceptsError; import org.enso.interpreter.dsl.BuiltinMethod; +import org.enso.interpreter.node.callable.InteropConversionCallNode; +import org.enso.interpreter.runtime.EnsoContext; +import org.enso.interpreter.runtime.callable.UnresolvedConversion; +import org.enso.interpreter.runtime.data.Type; +import org.enso.interpreter.runtime.error.PanicException; import org.enso.interpreter.runtime.library.dispatch.TypesLibrary; +import org.enso.interpreter.runtime.state.State; @BuiltinMethod( type = "Any", @@ -26,22 +37,171 @@ 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 { +public final class EqualsNode extends Node { + @Child private EqualsSimpleNode node; + @Child private TypeOfNode types; + @Child private WithConversionNode convert1; + @Child private WithConversionNode convert2; - public static EqualsNode build() { - return EqualsNodeGen.create(); + private EqualsNode(EqualsSimpleNode node, TypeOfNode types) { + this.node = node; + this.types = types; } - public abstract boolean execute(@AcceptsError Object self, @AcceptsError Object right); + @NeverDefault + static EqualsNode build() { + return create(); + } + + @NeverDefault + public static EqualsNode create() { + return new EqualsNode(EqualsSimpleNode.build(), TypeOfNode.build()); + } + + @NeverDefault + public static EqualsNode getUncached() { + return new EqualsNode(EqualsSimpleNodeGen.getUncached(), TypeOfNode.getUncached()); + } - @Specialization - boolean sameTypeCheck( - Object self, - Object other, - @CachedLibrary(limit = "3") TypesLibrary types, - @Cached EqualsSimpleNode node) { - var areEqual = node.execute(self, other); + /** + * Compares two objects for equality. If the {@link EqualsSimpleNode simple check} fails, it tries + * to convert first argument to the second one and compare again. + * + * @param frame the stack frame we are executing at + * @param self the self object + * @param other the other object + * @return {@code true} if {@code self} and {@code that} seem equal + */ + public boolean execute( + VirtualFrame frame, @AcceptsError Object self, @AcceptsError Object other) { + var areEqual = node.execute(frame, self, other); + if (!areEqual) { + var selfType = types.execute(self); + var otherType = types.execute(other); + if (selfType != otherType) { + if (convert1 == null) { + CompilerDirectives.transferToInterpreter(); + convert1 = insert(WithConversionNode.create()); + convert2 = insert(WithConversionNode.create()); + } + return convert1.executeThatConversion(frame, other, self) + || convert2.executeThatConversion(frame, self, other); + } + } return areEqual; } + + /** + * A node that checks the type of {@code that} argument, performs conversion of {@code self} to + * {@code that} type, and executes equality check again. + */ + @GenerateUncached + abstract static class WithConversionNode extends Node { + + @NeverDefault + static WithConversionNode create() { + return EqualsNodeFactory.WithConversionNodeGen.create(); + } + + /** + * @return {code false} if the conversion makes no sense or result of equality check after doing + * the conversion + */ + abstract boolean executeThatConversion(VirtualFrame frame, Object self, Object that); + + static Type findType(TypeOfNode typeOfNode, Object obj) { + var rawType = typeOfNode.execute(obj); + return rawType instanceof Type type ? type : null; + } + + static Type findTypeUncached(Object obj) { + return findType(TypeOfNode.getUncached(), obj); + } + + @Specialization( + limit = "3", + guards = { + "!types.hasSpecialDispatch(that)", + "selfType != null", + "thatType != null", + "thatType == findType(typeOfNode, that)" + }) + final boolean doThatConversionCached( + VirtualFrame frame, + Object self, + Object that, + @Shared("typeOf") @Cached TypeOfNode typeOfNode, + @CachedLibrary(limit = "3") TypesLibrary types, + @Cached(value = "findType(typeOfNode, self)", uncached = "findTypeUncached(self)") + Type selfType, + @Cached(value = "findType(typeOfNode, that)", uncached = "findTypeUncached(that)") + Type thatType, + @Shared("convert") @Cached InteropConversionCallNode convertNode, + @Shared("invoke") @Cached(allowUncached = true) EqualsSimpleNode equalityNode) { + return doDispatch(frame, self, that, thatType, convertNode, equalityNode); + } + + @Specialization(replaces = "doThatConversionCached") + final boolean doThatConversionUncached( + VirtualFrame frame, + Object self, + Object that, + @Shared("typeOf") @Cached TypeOfNode typeOfNode, + @Shared("convert") @Cached InteropConversionCallNode convertNode, + @Shared("invoke") @Cached(allowUncached = true) EqualsSimpleNode equalityNode) { + /* + if (that instanceof EnsoMultiValue multi) { + for (var thatType : multi.allTypes()) { + var fn = findSymbol(symbol, thatType); + if (fn != null) { + var result = + doDispatch( + frame, self, multi.castTo(thatType), thatType, fn, convertNode, equalityNode); + if (result != null) { + return result; + } + } + } + } else */ + { + var thatType = findType(typeOfNode, that); + if (thatType != null) { + var result = doDispatch(frame, self, that, thatType, convertNode, equalityNode); + return result; + } + } + return false; + } + + private boolean doDispatch( + VirtualFrame frame, + Object self, + Object that, + Type thatType, + InteropConversionCallNode convertNode, + EqualsSimpleNode equalityNode) + throws PanicException { + var convert = UnresolvedConversion.build(thatType.getDefinitionScope()); + + var ctx = EnsoContext.get(this); + var state = State.create(ctx); + try { + var selfAsThat = convertNode.execute(convert, state, new Object[] {thatType, self}); + var result = equalityNode.execute(frame, selfAsThat, that); + return result; + } catch (ArityException ex) { + var assertsOn = false; + assert assertsOn = true; + if (assertsOn) { + throw new AssertionError("Unexpected arity exception", ex); + } + return false; + } catch (PanicException ex) { + if (ctx.getBuiltins().error().isNoSuchConversionError(ex.getPayload())) { + return false; + } + throw ex; + } + } + } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsSimpleNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsSimpleNode.java index d684e4de8e75..d561e18bccaf 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsSimpleNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsSimpleNode.java @@ -5,6 +5,7 @@ import com.oracle.truffle.api.dsl.Cached.Shared; import com.oracle.truffle.api.dsl.GenerateUncached; import com.oracle.truffle.api.dsl.Specialization; +import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.interop.InteropLibrary; import com.oracle.truffle.api.interop.TruffleObject; import com.oracle.truffle.api.interop.UnsupportedMessageException; @@ -28,7 +29,7 @@ public static EqualsSimpleNode build() { return EqualsSimpleNodeGen.create(); } - public abstract boolean execute(Object self, Object right); + public abstract boolean execute(VirtualFrame frame, Object self, Object right); @Specialization boolean equalsBoolBool(boolean self, boolean other) { @@ -274,58 +275,54 @@ boolean equalsAtomConstructors(AtomConstructor self, AtomConstructor other) { @Specialization boolean equalsAtoms( + VirtualFrame frame, Atom self, Atom other, @Cached EqualsAtomNode equalsAtomNode, @Shared("isSameObjectNode") @Cached IsSameObjectNode isSameObjectNode) { - return isSameObjectNode.execute(self, other) || equalsAtomNode.execute(self, other); - } - - @Specialization - boolean equalsReverseBoolean( - TruffleObject self, - boolean other, - @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop, - @Shared("reverse") @Cached EqualsNode reverse) { - return reverse.execute(other, self); + return isSameObjectNode.execute(self, other) || equalsAtomNode.execute(frame, self, other); } @Specialization boolean equalsReverseLong( + VirtualFrame frame, TruffleObject self, long other, @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop, - @Shared("reverse") @Cached EqualsNode reverse) { - return reverse.execute(other, self); + @Shared("reverse") @Cached EqualsSimpleNode reverse) { + return reverse.execute(frame, other, self); } @Specialization boolean equalsReverseDouble( + VirtualFrame frame, TruffleObject self, double other, @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop, - @Shared("reverse") @Cached EqualsNode reverse) { - return reverse.execute(other, self); + @Shared("reverse") @Cached EqualsSimpleNode reverse) { + return reverse.execute(frame, other, self); } @Specialization boolean equalsReverseBigInt( + VirtualFrame frame, TruffleObject self, EnsoBigInteger other, @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop, - @Shared("reverse") @Cached EqualsNode reverse) { - return reverse.execute(other, self); + @Shared("reverse") @Cached EqualsSimpleNode reverse) { + return reverse.execute(frame, other, self); } @Specialization(guards = "isNotPrimitive(self, other, interop, warnings)") boolean equalsComplex( + VirtualFrame frame, Object self, Object other, @Cached EqualsComplexNode equalsComplex, @Shared("isSameObjectNode") @Cached IsSameObjectNode isSameObjectNode, @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop, @CachedLibrary(limit = "5") WarningsLibrary warnings) { - return isSameObjectNode.execute(self, other) || equalsComplex.execute(self, other); + return isSameObjectNode.execute(self, other) || equalsComplex.execute(frame, self, other); } static boolean isNotPrimitive( diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/ordering/SortArrayNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/ordering/SortArrayNode.java index 1ed278790603..f34cd00e8a31 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/ordering/SortArrayNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/ordering/SortArrayNode.java @@ -1,5 +1,6 @@ package org.enso.interpreter.node.expression.builtin.ordering; +import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.nodes.Node; import org.enso.interpreter.dsl.AcceptsError; import org.enso.interpreter.dsl.BuiltinMethod; @@ -11,6 +12,7 @@ public final class SortArrayNode extends Node { @Child private SortVectorNode sortVectorNode = SortVectorNode.build(); public Object execute( + VirtualFrame frame, State state, @AcceptsError Object self, long ascending, @@ -20,7 +22,15 @@ public Object execute( Object onFunc, long problemBehavior) { return sortVectorNode.execute( - state, self, ascending, comparators, compareFunctions, byFunc, onFunc, problemBehavior); + frame, + state, + self, + ascending, + comparators, + compareFunctions, + byFunc, + onFunc, + problemBehavior); } public static SortArrayNode build() { diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/ordering/SortVectorNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/ordering/SortVectorNode.java index 017aa53ae547..854e99eca2ac 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/ordering/SortVectorNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/ordering/SortVectorNode.java @@ -6,6 +6,8 @@ import com.oracle.truffle.api.dsl.Cached.Shared; import com.oracle.truffle.api.dsl.GenerateUncached; import com.oracle.truffle.api.dsl.Specialization; +import com.oracle.truffle.api.frame.MaterializedFrame; +import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.interop.InteropLibrary; import com.oracle.truffle.api.interop.InvalidArrayIndexException; import com.oracle.truffle.api.interop.UnsupportedMessageException; @@ -77,6 +79,7 @@ public static SortVectorNode build() { * @return A new, sorted vector. */ public abstract Object execute( + VirtualFrame frame, State state, @AcceptsError Object self, long ascending, @@ -101,6 +104,7 @@ public abstract Object execute( "interop.isNull(onFunc)" }) Object sortPrimitives( + VirtualFrame frame, State state, Object self, long ascending, @@ -131,7 +135,14 @@ Object sortPrimitives( } var javaComparator = createDefaultComparator( - lessThanNode, equalsNode, typeOfNode, toTextNode, ascending, problemBehavior, interop); + frame.materialize(), + lessThanNode, + equalsNode, + typeOfNode, + toTextNode, + ascending, + problemBehavior, + interop); try { return sortPrimitiveVector(elems, javaComparator); } catch (CompareException e) { @@ -142,6 +153,7 @@ Object sortPrimitives( @TruffleBoundary private DefaultSortComparator createDefaultComparator( + MaterializedFrame frame, LessThanNode lessThanNode, EqualsNode equalsNode, TypeOfNode typeOfNode, @@ -150,6 +162,7 @@ private DefaultSortComparator createDefaultComparator( long problemBehaviorNum, InteropLibrary interop) { return new DefaultSortComparator( + frame, lessThanNode, equalsNode, typeOfNode, @@ -165,6 +178,7 @@ private DefaultSortComparator createDefaultComparator( "interop.hasArrayElements(self)", }) Object sortGeneric( + MaterializedFrame frame, State state, Object self, long ascending, @@ -206,6 +220,7 @@ Object sortGeneric( if (interop.isNull(byFunc) && interop.isNull(onFunc) && isPrimitiveGroup(group)) { javaComparator = new DefaultSortComparator( + frame, lessThanNode, equalsNode, typeOfNode, @@ -557,12 +572,14 @@ public boolean hasWarnings() { */ final class DefaultSortComparator extends SortComparator { + private final MaterializedFrame frame; private final LessThanNode lessThanNode; private final EqualsNode equalsNode; private final TypeOfNode typeOfNode; private final boolean ascending; private DefaultSortComparator( + MaterializedFrame frame, LessThanNode lessThanNode, EqualsNode equalsNode, TypeOfNode typeOfNode, @@ -571,6 +588,7 @@ private DefaultSortComparator( ProblemBehavior problemBehavior, InteropLibrary interop) { super(toTextNode, problemBehavior, interop); + this.frame = frame; this.lessThanNode = lessThanNode; this.equalsNode = equalsNode; this.typeOfNode = typeOfNode; @@ -583,7 +601,7 @@ public int compare(Object x, Object y) { } int compareValuesWithDefaultComparator(Object x, Object y) { - if (equalsNode.execute(x, y)) { + if (equalsNode.execute(frame, x, y)) { return 0; } else { // Check if x < y diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/hash/EnsoHashMap.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/hash/EnsoHashMap.java index c960b5aae9c4..e2ae5ac8ed11 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/hash/EnsoHashMap.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/hash/EnsoHashMap.java @@ -3,6 +3,7 @@ import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; import com.oracle.truffle.api.dsl.Cached; import com.oracle.truffle.api.dsl.Cached.Shared; +import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.interop.InteropLibrary; import com.oracle.truffle.api.interop.UnknownKeyException; import com.oracle.truffle.api.interop.UnsupportedMessageException; @@ -55,11 +56,11 @@ static EnsoHashMap createEmpty() { } EnsoHashMapBuilder getMapBuilder( - boolean readOnly, HashCodeNode hashCodeNode, EqualsNode equalsNode) { + VirtualFrame frame, boolean readOnly, HashCodeNode hashCodeNode, EqualsNode equalsNode) { if (readOnly) { return mapBuilder; } else { - return mapBuilder.asModifiable(generation, hashCodeNode, equalsNode); + return mapBuilder.asModifiable(frame, generation, hashCodeNode, equalsNode); } } @@ -104,7 +105,7 @@ boolean isHashEntryExisting( Object key, @Shared("hash") @Cached HashCodeNode hashCodeNode, @Shared("equals") @Cached EqualsNode equalsNode) { - var entry = mapBuilder.get(key, generation, hashCodeNode, equalsNode); + var entry = mapBuilder.get(null, key, generation, hashCodeNode, equalsNode); return entry != null; } @@ -122,7 +123,7 @@ Object readHashValue( @Shared("hash") @Cached HashCodeNode hashCodeNode, @Shared("equals") @Cached EqualsNode equalsNode) throws UnknownKeyException { - StorageEntry entry = mapBuilder.get(key, generation, hashCodeNode, equalsNode); + StorageEntry entry = mapBuilder.get(null, key, generation, hashCodeNode, equalsNode); if (entry != null) { return entry.value(); } else { diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/hash/EnsoHashMapBuilder.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/hash/EnsoHashMapBuilder.java index c1208f2a5fd0..722aeb06914a 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/hash/EnsoHashMapBuilder.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/hash/EnsoHashMapBuilder.java @@ -1,6 +1,7 @@ package org.enso.interpreter.runtime.data.hash; import com.oracle.truffle.api.CompilerDirectives; +import com.oracle.truffle.api.frame.VirtualFrame; import java.util.Arrays; import org.enso.interpreter.node.expression.builtin.meta.EqualsNode; import org.enso.interpreter.node.expression.builtin.meta.HashCodeNode; @@ -87,10 +88,10 @@ public StorageEntry[] getEntries(int atGeneration, int size) { * Otherwise it may return new builder suitable for additions. */ public EnsoHashMapBuilder asModifiable( - int atGeneration, HashCodeNode hashCodeNode, EqualsNode equalsNode) { + VirtualFrame frame, int atGeneration, HashCodeNode hashCodeNode, EqualsNode equalsNode) { if (atGeneration != generation || generation * 4 > byHash.length * 3) { var newSize = Math.max(actualSize * 2, byHash.length); - return rehash(newSize, atGeneration, hashCodeNode, equalsNode); + return rehash(frame, newSize, atGeneration, hashCodeNode, equalsNode); } else { return this; } @@ -102,7 +103,12 @@ public EnsoHashMapBuilder asModifiable( * equal key, it marks it as removed, if it hasn't been removed yet. Once it finds an empty slot, * it puts there a new entry with the next generation. */ - public void put(Object key, Object value, HashCodeNode hashCodeNode, EqualsNode equalsNode) { + public void put( + VirtualFrame frame, + Object key, + Object value, + HashCodeNode hashCodeNode, + EqualsNode equalsNode) { var at = findWhereToStart(key, hashCodeNode); var nextGeneration = ++generation; var replacingExistingKey = false; @@ -114,7 +120,7 @@ public void put(Object key, Object value, HashCodeNode hashCodeNode, EqualsNode byHash[at] = new StorageEntry(key, value, nextGeneration); return; } - if (compare(equalsNode, byHash[at].key(), key)) { + if (compare(frame, equalsNode, byHash[at].key(), key)) { var invalidatedEntry = byHash[at].markRemoved(nextGeneration); if (invalidatedEntry != byHash[at]) { byHash[at] = invalidatedEntry; @@ -133,14 +139,18 @@ public void put(Object key, Object value, HashCodeNode hashCodeNode, EqualsNode * given {@code generation}. */ public StorageEntry get( - Object key, int generation, HashCodeNode hashCodeNode, EqualsNode equalsNode) { + VirtualFrame frame, + Object key, + int generation, + HashCodeNode hashCodeNode, + EqualsNode equalsNode) { var at = findWhereToStart(key, hashCodeNode); for (var i = 0; i < byHash.length; i++) { if (byHash[at] == null) { return null; } if (byHash[at].isVisible(generation)) { - if (compare(equalsNode, key, byHash[at].key())) { + if (compare(frame, equalsNode, key, byHash[at].key())) { return byHash[at]; } } @@ -164,14 +174,15 @@ private int findWhereToStart(Object key, HashCodeNode hashCodeNode) { * * @return true if the removal was successful false otherwise. */ - public boolean remove(Object key, HashCodeNode hashCodeNode, EqualsNode equalsNode) { + public boolean remove( + VirtualFrame frame, Object key, HashCodeNode hashCodeNode, EqualsNode equalsNode) { var at = findWhereToStart(key, hashCodeNode); var nextGeneration = ++generation; for (var i = 0; i < byHash.length; i++) { if (byHash[at] == null) { return false; } - if (compare(equalsNode, key, byHash[at].key())) { + if (compare(frame, equalsNode, key, byHash[at].key())) { var invalidatedEntry = byHash[at].markRemoved(nextGeneration); if (invalidatedEntry != byHash[at]) { byHash[at] = invalidatedEntry; @@ -191,12 +202,16 @@ public boolean remove(Object key, HashCodeNode hashCodeNode, EqualsNode equalsNo * atGeneration}. */ private EnsoHashMapBuilder rehash( - int size, int atGeneration, HashCodeNode hashCodeNode, EqualsNode equalsNode) { + VirtualFrame frame, + int size, + int atGeneration, + HashCodeNode hashCodeNode, + EqualsNode equalsNode) { var newBuilder = new EnsoHashMapBuilder(size); for (var i = 0; i < byHash.length; i++) { var entry = byHash[i]; if (entry != null && entry.isVisible(atGeneration)) { - newBuilder.put(entry.key(), entry.value(), hashCodeNode, equalsNode); + newBuilder.put(frame, entry.key(), entry.value(), hashCodeNode, equalsNode); } } return newBuilder; @@ -224,11 +239,11 @@ public String toString() { + "}"; } - private static boolean compare(EqualsNode equalsNode, Object a, Object b) { + private static boolean compare(VirtualFrame frame, EqualsNode equalsNode, Object a, Object b) { if (a instanceof Double aDbl && b instanceof Double bDbl && aDbl.isNaN() && bDbl.isNaN()) { return true; } else { - return equalsNode.execute(a, b); + return equalsNode.execute(frame, a, b); } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/hash/HashMapInsertNode.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/hash/HashMapInsertNode.java index 8ffc5d9e2402..6f64ed98b2cb 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/hash/HashMapInsertNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/hash/HashMapInsertNode.java @@ -4,6 +4,7 @@ import com.oracle.truffle.api.dsl.Cached; import com.oracle.truffle.api.dsl.Cached.Shared; import com.oracle.truffle.api.dsl.Specialization; +import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.interop.InteropLibrary; import com.oracle.truffle.api.interop.InvalidArrayIndexException; import com.oracle.truffle.api.interop.StopIterationException; @@ -30,17 +31,18 @@ public static HashMapInsertNode build() { return HashMapInsertNodeGen.create(); } - public abstract EnsoHashMap execute(Object self, Object key, Object value); + public abstract EnsoHashMap execute(VirtualFrame frame, Object self, Object key, Object value); @Specialization EnsoHashMap doEnsoHashMap( + VirtualFrame frame, EnsoHashMap hashMap, Object key, Object value, @Shared("hash") @Cached HashCodeNode hashCodeNode, @Shared("equals") @Cached EqualsNode equalsNode) { - var mapBuilder = hashMap.getMapBuilder(false, hashCodeNode, equalsNode); - mapBuilder.put(key, value, hashCodeNode, equalsNode); + var mapBuilder = hashMap.getMapBuilder(frame, false, hashCodeNode, equalsNode); + mapBuilder.put(frame, key, value, hashCodeNode, equalsNode); var newMap = mapBuilder.build(); return newMap; } @@ -51,6 +53,7 @@ EnsoHashMap doEnsoHashMap( */ @Specialization(guards = "mapInterop.hasHashEntries(foreignMap)", limit = "3") EnsoHashMap doForeign( + VirtualFrame frame, Object foreignMap, Object keyToInsert, Object valueToInsert, @@ -65,8 +68,9 @@ EnsoHashMap doForeign( Object keyValueArr = iteratorInterop.getIteratorNextElement(entriesIterator); Object key = iteratorInterop.readArrayElement(keyValueArr, 0); Object value = iteratorInterop.readArrayElement(keyValueArr, 1); - mapBuilder = mapBuilder.asModifiable(mapBuilder.generation(), hashCodeNode, equalsNode); - mapBuilder.put(key, value, hashCodeNode, equalsNode); + mapBuilder = + mapBuilder.asModifiable(frame, mapBuilder.generation(), hashCodeNode, equalsNode); + mapBuilder.put(frame, key, value, hashCodeNode, equalsNode); } } catch (UnsupportedMessageException | StopIterationException | InvalidArrayIndexException e) { CompilerDirectives.transferToInterpreter(); @@ -76,8 +80,8 @@ EnsoHashMap doForeign( + " has wrongly specified Interop API (hash entries iterator)"; throw new PanicException(Text.create(msg), this); } - mapBuilder = mapBuilder.asModifiable(mapBuilder.generation(), hashCodeNode, equalsNode); - mapBuilder.put(keyToInsert, valueToInsert, hashCodeNode, equalsNode); + mapBuilder = mapBuilder.asModifiable(frame, mapBuilder.generation(), hashCodeNode, equalsNode); + mapBuilder.put(frame, keyToInsert, valueToInsert, hashCodeNode, equalsNode); return EnsoHashMap.createWithBuilder(mapBuilder); } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/hash/HashMapRemoveNode.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/hash/HashMapRemoveNode.java index b19f6e915b2e..30a3541197ed 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/hash/HashMapRemoveNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/hash/HashMapRemoveNode.java @@ -5,6 +5,7 @@ import com.oracle.truffle.api.dsl.Cached.Shared; import com.oracle.truffle.api.dsl.GenerateUncached; import com.oracle.truffle.api.dsl.Specialization; +import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.interop.InteropLibrary; import com.oracle.truffle.api.interop.InvalidArrayIndexException; import com.oracle.truffle.api.interop.StopIterationException; @@ -29,16 +30,17 @@ public static HashMapRemoveNode build() { return HashMapRemoveNodeGen.create(); } - public abstract EnsoHashMap execute(Object self, Object key); + public abstract EnsoHashMap execute(VirtualFrame frame, Object self, Object key); @Specialization EnsoHashMap removeFromEnsoMap( + VirtualFrame frame, EnsoHashMap ensoMap, Object key, @Shared("hash") @Cached HashCodeNode hashCodeNode, @Shared("equals") @Cached EqualsNode equalsNode) { - var mapBuilder = ensoMap.getMapBuilder(false, hashCodeNode, equalsNode); - if (mapBuilder.remove(key, hashCodeNode, equalsNode)) { + var mapBuilder = ensoMap.getMapBuilder(frame, false, hashCodeNode, equalsNode); + if (mapBuilder.remove(frame, key, hashCodeNode, equalsNode)) { return mapBuilder.build(); } else { throw DataflowError.withoutTrace("No such key", null); @@ -47,6 +49,7 @@ EnsoHashMap removeFromEnsoMap( @Specialization(guards = "interop.hasHashEntries(map)") EnsoHashMap removeFromInteropMap( + VirtualFrame frame, Object map, Object keyToRemove, @CachedLibrary(limit = "5") InteropLibrary interop, @@ -62,7 +65,7 @@ EnsoHashMap removeFromInteropMap( while (interop.hasIteratorNextElement(entriesIterator)) { Object keyValueArr = interop.getIteratorNextElement(entriesIterator); Object key = interop.readArrayElement(keyValueArr, 0); - if ((boolean) equalsNode.execute(keyToRemove, key)) { + if (equalsNode.execute(frame, keyToRemove, key)) { if (keyToRemoveFound) { CompilerDirectives.transferToInterpreter(); var ctx = EnsoContext.get(this); @@ -72,8 +75,9 @@ EnsoHashMap removeFromInteropMap( } } else { Object value = interop.readArrayElement(keyValueArr, 1); - mapBuilder = mapBuilder.asModifiable(mapBuilder.generation(), hashCodeNode, equalsNode); - mapBuilder.put(key, value, hashCodeNode, equalsNode); + mapBuilder = + mapBuilder.asModifiable(frame, mapBuilder.generation(), hashCodeNode, equalsNode); + mapBuilder.put(frame, key, value, hashCodeNode, equalsNode); } } } catch (UnsupportedMessageException | StopIterationException | InvalidArrayIndexException e) { From a2bfb88085864892cd29410c37db9b988cf836c7 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 15 Feb 2024 09:27:04 +0100 Subject: [PATCH 06/27] Removing accidentally committed fix from other PR --- .../enso/interpreter/caches/CacheUtils.java | 52 ++++++++++--------- .../interpreter/caches/ImportExportCache.java | 2 +- .../enso/interpreter/caches/ModuleCache.java | 2 +- 3 files changed, 30 insertions(+), 26 deletions(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/caches/CacheUtils.java b/engine/runtime/src/main/java/org/enso/interpreter/caches/CacheUtils.java index f90f4edfe204..e14818113729 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/caches/CacheUtils.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/caches/CacheUtils.java @@ -1,6 +1,5 @@ package org.enso.interpreter.caches; -import com.oracle.truffle.api.TruffleFile; import java.io.IOException; import java.nio.ByteBuffer; import java.security.MessageDigest; @@ -9,38 +8,40 @@ import java.util.List; import java.util.UUID; import java.util.function.Function; + import org.enso.compiler.context.CompilerContext; import org.enso.compiler.core.ir.ProcessingPass; import org.enso.pkg.SourceFile; import org.enso.text.Hex; +import com.oracle.truffle.api.TruffleFile; + final class CacheUtils { - private CacheUtils() {} + private CacheUtils() { + } - static Function writeReplace(CompilerContext context, boolean keepUUIDs) { - return (obj) -> - switch (obj) { - case ProcessingPass.Metadata metadata -> metadata.prepareForSerialization(context); - case UUID id -> keepUUIDs ? id : null; - case null -> null; - default -> obj; - }; + static Function writeReplace(CompilerContext context) { + return (obj) -> switch (obj) { + case ProcessingPass.Metadata metadata -> metadata.prepareForSerialization(context); + case UUID _ -> null; + case null -> null; + default -> obj; + }; } static Function readResolve(CompilerContext context) { - return (obj) -> - switch (obj) { - case ProcessingPass.Metadata metadata -> { - var option = metadata.restoreFromSerialization(context); - if (option.nonEmpty()) { - yield option.get(); - } else { - throw raise(RuntimeException.class, new IOException("Cannot convert " + metadata)); - } - } - case null -> null; - default -> obj; - }; + return (obj) -> switch (obj) { + case ProcessingPass.Metadata metadata -> { + var option = metadata.restoreFromSerialization(context); + if (option.nonEmpty()) { + yield option.get(); + } else { + throw raise(RuntimeException.class, new IOException("Cannot convert " + metadata)); + } + } + case null -> null; + default -> obj; + }; } /** @@ -74,7 +75,9 @@ static String computeDigestFromBytes(ByteBuffer bytes) { * @param pkgSources the list of package sources * @return string representation of bytes' hash */ - static final String computeDigestOfLibrarySources(List> pkgSources) { + static final String computeDigestOfLibrarySources( + List> pkgSources + ) { pkgSources.sort(Comparator.comparing(o -> o.qualifiedName().toString())); try { @@ -92,4 +95,5 @@ static final String computeDigestOfLibrarySources(List> static T raise(Class cls, Exception e) throws T { throw (T) e; } + } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/caches/ImportExportCache.java b/engine/runtime/src/main/java/org/enso/interpreter/caches/ImportExportCache.java index 7144728b00f1..526714353a78 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/caches/ImportExportCache.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/caches/ImportExportCache.java @@ -65,7 +65,7 @@ public byte[] metadata(String sourceDigest, String blobDigest, CachedBindings en public byte[] serialize(EnsoContext context, CachedBindings entry) throws IOException { var arr = Persistance.write( - entry.bindings(), CacheUtils.writeReplace(context.getCompiler().context(), false)); + entry.bindings(), CacheUtils.writeReplace(context.getCompiler().context())); return arr; } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/caches/ModuleCache.java b/engine/runtime/src/main/java/org/enso/interpreter/caches/ModuleCache.java index d32c446ec29d..e65bb3b24476 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/caches/ModuleCache.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/caches/ModuleCache.java @@ -60,7 +60,7 @@ public byte[] metadata(String sourceDigest, String blobDigest, CachedModule entr public byte[] serialize(EnsoContext context, CachedModule entry) throws IOException { var arr = Persistance.write( - entry.moduleIR(), CacheUtils.writeReplace(context.getCompiler().context(), true)); + entry.moduleIR(), CacheUtils.writeReplace(context.getCompiler().context())); return arr; } From 10f7941714e9621f139ff275e757fb78306d5730 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 15 Feb 2024 09:47:24 +0100 Subject: [PATCH 07/27] TruffleObject vs. boolean specialization --- .../builtin/meta/EqualsSimpleNode.java | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsSimpleNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsSimpleNode.java index d561e18bccaf..78b2369f431a 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsSimpleNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsSimpleNode.java @@ -68,6 +68,18 @@ boolean equalsBoolInterop( } } + @Specialization + boolean equalsInteropBool( + TruffleObject self, + boolean other, + @Shared("interop") @CachedLibrary(limit = "10") InteropLibrary interop) { + try { + return other == interop.asBoolean(self); + } catch (UnsupportedMessageException ex) { + return false; + } + } + @Specialization boolean equalsLongLong(long self, long other) { return self == other; @@ -185,11 +197,6 @@ boolean equalsBigIntLong( } } - @Specialization - boolean equalsBigIntBool(EnsoBigInteger self, boolean other) { - return false; - } - @Specialization boolean equalsBigIntText(EnsoBigInteger self, Text other) { return false; @@ -214,11 +221,6 @@ boolean equalsTextText(Text selfText, Text otherText) { return selfText.toString().compareTo(otherText.toString()) == 0; } - @Specialization - boolean equalsTextBool(Text self, boolean other) { - return false; - } - @Specialization boolean equalsTextLong(Text selfText, long otherLong) { return false; From b30801485fddc7a73aeed484b023d21853aa8b9e Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 15 Feb 2024 09:56:46 +0100 Subject: [PATCH 08/27] Encapsulating in equalityCheck method --- .../org/enso/interpreter/test/EqualsTest.java | 80 +++++++++---------- 1 file changed, 37 insertions(+), 43 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsTest.java index 6e139d044969..e76b645de263 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsTest.java @@ -87,13 +87,17 @@ private static Object[] fetchAllUnwrappedValues() { } } + private boolean equalityCheck(Object first, Object second) { + return equalsNode.execute(null, first, second); + } + @Theory public void equalsOperatorShouldBeSymmetric(Object firstValue, Object secondValue) { executeInContext( context, () -> { - boolean firstResult = equalsNode.execute(null, firstValue, secondValue); - boolean secondResult = equalsNode.execute(null, secondValue, firstValue); + boolean firstResult = equalityCheck(firstValue, secondValue); + boolean secondResult = equalityCheck(secondValue, firstValue); assertEquals("equals should be symmetric", firstResult, secondResult); return null; }); @@ -104,8 +108,8 @@ public void equalsOperatorShouldBeConsistent(Object value) { executeInContext( context, () -> { - Object firstResult = equalsNode.execute(null, value, value); - Object secondResult = equalsNode.execute(null, value, value); + Object firstResult = equalityCheck(value, value); + Object secondResult = equalityCheck(value, value); assertEquals("equals should be consistent", firstResult, secondResult); return null; }); @@ -117,7 +121,7 @@ public void equalsNodeCachedIsConsistentWithUncached(Object firstVal, Object sec context, () -> { Object uncachedRes = EqualsNode.getUncached().execute(null, firstVal, secondVal); - Object cachedRes = equalsNode.execute(null, firstVal, secondVal); + Object cachedRes = equalityCheck(firstVal, secondVal); assertEquals( "Result from uncached EqualsNode should be the same as result from its cached" + " variant", @@ -139,7 +143,7 @@ public void testDateEquality() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(null, ensoDate, javaDate)); + assertTrue(equalityCheck(ensoDate, javaDate)); return null; }); } @@ -157,7 +161,7 @@ public void testTimeEquality() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(null, ensoTime, javaDate)); + assertTrue(equalityCheck(ensoTime, javaDate)); return null; }); } @@ -180,7 +184,7 @@ public void testDateTimeEquality() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(null, ensoDateTime, javaDateTime)); + assertTrue(equalityCheck(ensoDateTime, javaDateTime)); return null; }); } @@ -193,8 +197,7 @@ public void testDoubleEqualsEnsoBigInteger() { executeInContext( context, () -> { - assertTrue( - javaNumber + " == " + ensoNumber, equalsNode.execute(null, javaNumber, ensoNumber)); + assertTrue(javaNumber + " == " + ensoNumber, equalityCheck(javaNumber, ensoNumber)); return null; }); } @@ -207,8 +210,7 @@ public void testEnsoBigIntegerEqualsDoubleEquals() { executeInContext( context, () -> { - assertTrue( - ensoNumber + " == " + javaNumber, equalsNode.execute(null, ensoNumber, javaNumber)); + assertTrue(ensoNumber + " == " + javaNumber, equalityCheck(ensoNumber, javaNumber)); return null; }); } @@ -221,8 +223,7 @@ public void testDoubleEqualsJavaBigInteger() { executeInContext( context, () -> { - assertTrue( - javaNumber + " == " + hostNumber, equalsNode.execute(null, javaNumber, hostNumber)); + assertTrue(javaNumber + " == " + hostNumber, equalityCheck(javaNumber, hostNumber)); return null; }); } @@ -235,8 +236,7 @@ public void testJavaBigIntegerEqualsDoubleEquals() { executeInContext( context, () -> { - assertTrue( - hostNumber + " == " + javaNumber, equalsNode.execute(null, hostNumber, javaNumber)); + assertTrue(hostNumber + " == " + javaNumber, equalityCheck(hostNumber, javaNumber)); return null; }); } @@ -249,7 +249,7 @@ public void testVectorsEquality() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(null, ensoVector, javaVector)); + assertTrue(equalityCheck(ensoVector, javaVector)); return null; }); } @@ -261,9 +261,9 @@ public void testTruffleNumberLong() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(null, ensoNumber, foreignNumber.asDirect())); - assertTrue(equalsNode.execute(null, ensoNumber, foreignNumber)); - assertTrue(equalsNode.execute(null, foreignNumber, ensoNumber)); + assertTrue(equalityCheck(ensoNumber, foreignNumber.asDirect())); + assertTrue(equalityCheck(ensoNumber, foreignNumber)); + assertTrue(equalityCheck(foreignNumber, ensoNumber)); return null; }); } @@ -275,9 +275,9 @@ public void testTruffleNumberDouble() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(null, ensoNumber, foreignNumber.asDirect())); - assertTrue(equalsNode.execute(null, ensoNumber, foreignNumber)); - assertTrue(equalsNode.execute(null, foreignNumber, ensoNumber)); + assertTrue(equalityCheck(ensoNumber, foreignNumber.asDirect())); + assertTrue(equalityCheck(ensoNumber, foreignNumber)); + assertTrue(equalityCheck(foreignNumber, ensoNumber)); return null; }); } @@ -290,8 +290,8 @@ public void testTruffleNumberBigInt() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(null, ensoNumber, foreignNumber)); - assertTrue(equalsNode.execute(null, foreignNumber, ensoNumber)); + assertTrue(equalityCheck(ensoNumber, foreignNumber)); + assertTrue(equalityCheck(foreignNumber, ensoNumber)); return null; }); } @@ -304,9 +304,9 @@ public void testTruffleBoolean() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(null, ensoBoolean, foreignBoolean.asDirect())); - assertTrue(equalsNode.execute(null, ensoBoolean, foreignBoolean)); - assertTrue(equalsNode.execute(null, foreignBoolean, ensoBoolean)); + assertTrue(equalityCheck(ensoBoolean, foreignBoolean.asDirect())); + assertTrue(equalityCheck(ensoBoolean, foreignBoolean)); + assertTrue(equalityCheck(foreignBoolean, ensoBoolean)); return null; }); } @@ -318,9 +318,9 @@ public void testTruffleString() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(null, ensoText, foreignString.asDirect())); - assertTrue(equalsNode.execute(null, ensoText, foreignString)); - assertTrue(equalsNode.execute(null, foreignString, ensoText)); + assertTrue(equalityCheck(ensoText, foreignString.asDirect())); + assertTrue(equalityCheck(ensoText, foreignString)); + assertTrue(equalityCheck(foreignString, ensoText)); return null; }); } @@ -339,8 +339,8 @@ public void testTruffleNumberPlus() { executeInContext( context, () -> { - assertTrue(equalsNode.execute(null, 142L, hundred42)); - assertTrue(equalsNode.execute(null, hundred42, 142L)); + assertTrue(equalityCheck(142L, hundred42)); + assertTrue(equalityCheck(hundred42, 142L)); return null; }); } @@ -378,18 +378,12 @@ && unwrapValue(context, mod2) instanceof org.enso.interpreter.runtime.Module m2) executeInContext( context, () -> { - assertTrue( - "Conversions from same module are the same", - equalsNode.execute(null, conv1, conv1_2)); - assertTrue( - "Conversions from same module are the same", - equalsNode.execute(null, conv2, conv2_2)); + assertTrue("Conversions from same module are the same", equalityCheck(conv1, conv1_2)); + assertTrue("Conversions from same module are the same", equalityCheck(conv2, conv2_2)); assertFalse( - "Conversions from other modules aren't the same", - equalsNode.execute(null, conv1, conv2)); + "Conversions from other modules aren't the same", equalityCheck(conv1, conv2)); assertFalse( - "Conversions from other modueles aren't the same", - equalsNode.execute(null, conv2_2, conv1_2)); + "Conversions from other modueles aren't the same", equalityCheck(conv2_2, conv1_2)); return null; }); From 290b63b47019ec9e9ff920896b303571bd36cc31 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 15 Feb 2024 10:11:12 +0100 Subject: [PATCH 09/27] Uncached version of the node may not use cached libraries --- .../java/org/enso/interpreter/test/EqualsTest.java | 10 ++++++++-- .../java/org/enso/interpreter/test/TestBase.java | 14 +++++++++++++- .../expression/builtin/meta/EqualsAtomNode.java | 2 +- .../node/expression/builtin/meta/EqualsNode.java | 13 ++++++++++--- .../enso/interpreter/runtime/error/Warning.java | 2 +- 5 files changed, 33 insertions(+), 8 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsTest.java index e76b645de263..7ea7b87feb6f 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsTest.java @@ -6,6 +6,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import com.oracle.truffle.api.frame.VirtualFrame; import java.math.BigInteger; import java.time.LocalDate; import java.time.LocalTime; @@ -43,7 +44,7 @@ public static void initContextAndData() { executeInContext( context, () -> { - testRootNode = new TestRootNode(); + testRootNode = new TestRootNode(EqualsTest::equalityCheck); equalsNode = EqualsNode.create(); hostValueToEnsoNode = HostValueToEnsoNode.build(); testRootNode.insertChildren(equalsNode, hostValueToEnsoNode); @@ -87,8 +88,13 @@ private static Object[] fetchAllUnwrappedValues() { } } + private static boolean equalityCheck(VirtualFrame frame) { + var args = frame.getArguments(); + return equalsNode.execute(frame, args[0], args[1]); + } + private boolean equalityCheck(Object first, Object second) { - return equalsNode.execute(null, first, second); + return (Boolean) testRootNode.getCallTarget().call(first, second); } @Theory diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/TestBase.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/TestBase.java index 34e2e475270e..5c45398415e2 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/TestBase.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/TestBase.java @@ -13,6 +13,7 @@ import java.nio.file.Paths; import java.util.Map; import java.util.concurrent.Callable; +import java.util.function.Function; import java.util.logging.Level; import org.enso.interpreter.EnsoLanguage; import org.enso.polyglot.MethodNames.Module; @@ -152,8 +153,15 @@ protected static Value getMethodFromModule(Context ctx, String moduleSrc, String * #insertChildren(Node...)}. */ static class TestRootNode extends RootNode { + private final Function callback; + TestRootNode() { + this(null); + } + + TestRootNode(Function callback) { super(EnsoLanguage.get(null)); + this.callback = callback; } void insertChildren(Node... children) { @@ -165,7 +173,11 @@ void insertChildren(Node... children) { /** In the tests, do not execute this root node, but execute directly the child nodes. */ @Override public Object execute(VirtualFrame frame) { - throw new AssertionError("should not reach here"); + if (callback == null) { + throw new AssertionError("should not reach here"); + } else { + return callback.apply(frame); + } } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsAtomNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsAtomNode.java index 4c4615a0eded..8914feaabb42 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsAtomNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsAtomNode.java @@ -125,7 +125,7 @@ boolean equalsAtomsUncached(VirtualFrame frame, Atom self, Atom other) { if (self.getConstructor() != other.getConstructor()) { return false; } else { - return equalsAtomsUncached(frame.materialize(), self, other); + return equalsAtomsUncached(frame == null ? null : frame.materialize(), self, other); } } 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 862aa3bccf48..edfab276e9a1 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 @@ -43,9 +43,16 @@ public final class EqualsNode extends Node { @Child private WithConversionNode convert1; @Child private WithConversionNode convert2; - private EqualsNode(EqualsSimpleNode node, TypeOfNode types) { + private static final EqualsNode UNCACHED = + new EqualsNode(EqualsSimpleNodeGen.getUncached(), TypeOfNode.getUncached(), true); + + private EqualsNode(EqualsSimpleNode node, TypeOfNode types, boolean uncached) { this.node = node; this.types = types; + if (uncached) { + convert1 = EqualsNodeFactory.WithConversionNodeGen.getUncached(); + convert2 = convert1; + } } @NeverDefault @@ -55,12 +62,12 @@ static EqualsNode build() { @NeverDefault public static EqualsNode create() { - return new EqualsNode(EqualsSimpleNode.build(), TypeOfNode.build()); + return new EqualsNode(EqualsSimpleNode.build(), TypeOfNode.build(), false); } @NeverDefault public static EqualsNode getUncached() { - return new EqualsNode(EqualsSimpleNodeGen.getUncached(), TypeOfNode.getUncached()); + return UNCACHED; } /** diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/error/Warning.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/error/Warning.java index 0d530d724fb4..9945a6d877de 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/error/Warning.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/error/Warning.java @@ -231,7 +231,7 @@ public long getSequenceId() { public Warning reassign(Node location) { RootNode root = location.getRootNode(); SourceSection section = location.getEncapsulatingSourceSection(); - Reassignment reassignment = new Reassignment(root.getName(), section); + Reassignment reassignment = new Reassignment(root == null ? "" : root.getName(), section); return new Warning(value, origin, sequenceId, reassignments.prepend(reassignment)); } From bbbbf73731d2e808e1c161a296a36aec8baf1b1c Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 15 Feb 2024 11:12:28 +0100 Subject: [PATCH 10/27] Fool is now equal to anything as there is a Fool.from (that:Any) conversion --- .../expression/builtin/meta/EqualsNode.java | 24 +++----------- .../src/Semantic/Conversion_Spec.enso | 32 +++++++++---------- 2 files changed, 20 insertions(+), 36 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 edfab276e9a1..32d82823e54c 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 @@ -156,26 +156,10 @@ final boolean doThatConversionUncached( @Shared("typeOf") @Cached TypeOfNode typeOfNode, @Shared("convert") @Cached InteropConversionCallNode convertNode, @Shared("invoke") @Cached(allowUncached = true) EqualsSimpleNode equalityNode) { - /* - if (that instanceof EnsoMultiValue multi) { - for (var thatType : multi.allTypes()) { - var fn = findSymbol(symbol, thatType); - if (fn != null) { - var result = - doDispatch( - frame, self, multi.castTo(thatType), thatType, fn, convertNode, equalityNode); - if (result != null) { - return result; - } - } - } - } else */ - { - var thatType = findType(typeOfNode, that); - if (thatType != null) { - var result = doDispatch(frame, self, that, thatType, convertNode, equalityNode); - return result; - } + var thatType = findType(typeOfNode, that); + if (thatType != null) { + var result = doDispatch(frame, self, that, thatType, convertNode, equalityNode); + return result; } return false; } diff --git a/test/Base_Tests/src/Semantic/Conversion_Spec.enso b/test/Base_Tests/src/Semantic/Conversion_Spec.enso index ab436844ea73..06239dcb0a0b 100644 --- a/test/Base_Tests/src/Semantic/Conversion_Spec.enso +++ b/test/Base_Tests/src/Semantic/Conversion_Spec.enso @@ -291,10 +291,10 @@ add_specs suite_builder = x.fool . should_equal 42 x==x . should_be_true (x:Integer)==42 . should_be_true - (x:Fool)==42 . should_be_false + (x:Fool)==42 . should_be_true # as 42 can be converted to Fool 42! x==42 . should_be_true 42==(x.to Integer) . should_be_true - 42==(x.to Fool) . should_be_false + 42==(x.to Fool) . should_be_true # as 42 can be converted to Fool 42! 42==x . should_be_true 100+(x:Integer) . should_equal 142 (x:Integer)+100 . should_equal 142 @@ -312,10 +312,10 @@ add_specs suite_builder = x.fool . should_equal 42.3 x==x . should_be_true (x:Float)==42.3 . should_be_true - (x:Fool)==42.3 . should_be_false + (x:Fool)==42.3 . should_be_true # as 42.3 can be converted to Fool 42.3! x==42.3 . should_be_true 42.3==(x.to Float) . should_be_true - 42.3==(x.to Fool) . should_be_false + 42.3==(x.to Fool) . should_be_true # as 42.3 can be converted to Fool 42.3! 42.3==x . should_be_true 100+(x:Float) . should_equal 142.3 (x:Float)+100 . should_equal 142.3 @@ -332,10 +332,10 @@ add_specs suite_builder = x.fool . should_equal True x==x . should_be_true (x:Boolean) . should_be_true - (x:Fool)==True . should_be_false + (x:Fool)==True . should_be_true # as True can be converted to Fool True! x==True . should_be_true True==(x:Boolean) . should_be_true - True==(x:Fool) . should_be_false + True==(x:Fool) . should_be_true # as True can be converted to Fool True! True==x . should_be_true x.to_text . should_equal "{FOOL True}" (x:Fool).to_text . should_equal "{FOOL True}" @@ -349,10 +349,10 @@ add_specs suite_builder = x.fool . should_equal "Hello" x==x . should_be_true (x:Text)=="Hello" . should_be_true - (x:Fool)=="Hello" . should_be_false + (x:Fool)=="Hello" . should_be_true # as "Hello" can be converted to Fool "Hello"! x=="Hello" . should_be_true "Hello"==(x:Text) . should_be_true - "Hello"==(x:Fool) . should_be_false + "Hello"==(x:Fool) . should_be_true # as "Hello" can be converted to Fool "Hello"! "Hello"==x . should_be_true x.to_text . should_equal "Hello" (x:Fool).to_text . should_equal "{FOOL Hello}" @@ -368,10 +368,10 @@ add_specs suite_builder = x.fool . should_equal now x==x . should_be_true (x:Time_Of_Day)==now . should_be_true - (x:Fool)==now . should_be_false + (x:Fool)==now . should_be_true # as now can be converted to Fool now! x==now . should_be_true now==(x:Time_Of_Day) . should_be_true - now==(x:Fool) . should_be_false + now==(x:Fool) . should_be_true # as now can be converted to Fool now! now==x . should_be_true x.to_text . should_equal now.to_text @@ -384,10 +384,10 @@ add_specs suite_builder = x.fool . should_equal now x==x . should_be_true (x:Date)==now . should_be_true - (x:Fool)==now . should_be_false + (x:Fool)==now . should_be_true # as now can be converted to Fool now! x==now . should_be_true now==(x:Date) . should_be_true - now==(x:Fool) . should_be_false + now==(x:Fool) . should_be_true # as now can be converted to Fool now! now==x . should_be_true x.to_text . should_equal "{FOOL "+now.to_text+"}" @@ -400,10 +400,10 @@ add_specs suite_builder = x.fool . should_equal now x==x . should_be_true (x:Date_Time)==now . should_be_true - (x:Fool)==now . should_be_false + (x:Fool)==now . should_be_true # as now can be converted to Fool now! x==now . should_be_true now==(x:Date_Time) . should_be_true - now==(x:Fool) . should_be_false + now==(x:Fool) . should_be_true # as now can be converted to Fool now! now==x . should_be_true x.to_text . should_equal now.to_text @@ -416,10 +416,10 @@ add_specs suite_builder = x.fool . should_equal now x==x . should_be_true (x:Duration)==now . should_be_true - (x:Fool)==now . should_be_false + (x:Fool)==now . should_be_true # as now can be converted to Fool now! x==now . should_be_true now==(x:Duration) . should_be_true - now==(x:Fool) . should_be_false + now==(x:Fool) . should_be_true # as now can be converted to Fool now! now==x . should_be_true x.to_text . should_equal "{FOOL "+now.to_text+"}" From 3fde985866860dccd2cc07d884ade933e5128ac2 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 15 Feb 2024 15:56:35 +0100 Subject: [PATCH 11/27] Don't serialize caches in unit tests --- .../src/test/java/org/enso/interpreter/test/TestBase.java | 1 + 1 file changed, 1 insertion(+) diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/TestBase.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/TestBase.java index 5c45398415e2..b80e06e218c1 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/TestBase.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/TestBase.java @@ -46,6 +46,7 @@ protected static Context.Builder defaultContextBuilder(String... languages) { .allowIO(IOAccess.ALL) .allowAllAccess(true) .option(RuntimeOptions.LOG_LEVEL, Level.WARNING.getName()) + .option(RuntimeOptions.DISABLE_IR_CACHES, "true") .logHandler(System.err) .option(RuntimeOptions.STRICT_ERRORS, "true") .option( From ecd4b90b834766f33a27d6d721ab1d1173feaa80 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 15 Feb 2024 15:57:56 +0100 Subject: [PATCH 12/27] Enforce consistency of == and hash for converted objects --- .../expression/builtin/meta/EqualsNode.java | 22 +++++++++++++++++++ .../expression/builtin/meta/HashCodeNode.java | 4 ++++ test/Base_Tests/src/Data/Numbers_Spec.enso | 3 ++- .../src/Semantic/Conversion_Spec.enso | 6 +++++ 4 files changed, 34 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 32d82823e54c..28e4d2373041 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 @@ -179,6 +179,7 @@ private boolean doDispatch( try { var selfAsThat = convertNode.execute(convert, state, new Object[] {thatType, self}); var result = equalityNode.execute(frame, selfAsThat, that); + assert !result || assertHashCodeIsTheSame(self, selfAsThat); return result; } catch (ArityException ex) { var assertsOn = false; @@ -194,5 +195,26 @@ private boolean doDispatch( throw ex; } } + + private boolean assertHashCodeIsTheSame(Object self, Object converted) { + var selfHash = HashCodeNode.getUncached().execute(self); + var convertedHash = HashCodeNode.getUncached().execute(converted); + var ok = selfHash == convertedHash; + var msg = + "Different hash code! Original " + + self + + "[#" + + Long.toHexString(selfHash) + + "] got converted to " + + converted + + "[#" + + Long.toHexString(convertedHash) + + "]"; + if (!ok) { + var ctx = EnsoContext.get(this); + throw ctx.raiseAssertionPanic(this, msg, new AssertionError(msg)); + } + return ok; + } } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/HashCodeNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/HashCodeNode.java index 1b4dd2192aac..159a5a0ea409 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/HashCodeNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/HashCodeNode.java @@ -83,6 +83,10 @@ public static HashCodeNode build() { return HashCodeNodeGen.create(); } + static HashCodeNode getUncached() { + return HashCodeNodeGen.getUncached(); + } + public abstract long execute(@AcceptsError Object object); /** Specializations for primitive values * */ diff --git a/test/Base_Tests/src/Data/Numbers_Spec.enso b/test/Base_Tests/src/Data/Numbers_Spec.enso index 01809503351d..e8e843b67c66 100644 --- a/test/Base_Tests/src/Data/Numbers_Spec.enso +++ b/test/Base_Tests/src/Data/Numbers_Spec.enso @@ -33,7 +33,8 @@ type Complex_Comparator compare x:Complex y:Complex = if x.re==y.re && x.im==y.im then Ordering.Equal else if x.im==0 && y.im==0 then Ordering.compare x.re y.re else Nothing - hash x:Complex = 7*x.re + 11*x.im + hash x:Complex = if x.im == 0 then Default_Comparator.hash x.re else + 7*x.re + 11*x.im Comparable.from (_:Complex) = Complex_Comparator diff --git a/test/Base_Tests/src/Semantic/Conversion_Spec.enso b/test/Base_Tests/src/Semantic/Conversion_Spec.enso index 06239dcb0a0b..20deee602d5e 100644 --- a/test/Base_Tests/src/Semantic/Conversion_Spec.enso +++ b/test/Base_Tests/src/Semantic/Conversion_Spec.enso @@ -82,6 +82,12 @@ type Fool Fool.from (that : Any) = Fool.Value that +type Fool_Comparator + compare x:Fool y:Fool = Ordering.compare x.fool y.fool + hash x:Fool = Default_Comparator.hash x.fool + +Comparable.from (_:Fool) = Fool_Comparator + type Blob Text c:Text Binary b:File From 69510cd6a9a6a55bdf1ce6fe19873ffb29fcb8e6 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Thu, 15 Feb 2024 17:41:35 +0100 Subject: [PATCH 13/27] Fool shall not be == unless it defines additional conversions --- .../src/Semantic/Conversion_Spec.enso | 38 ++++++++----------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/test/Base_Tests/src/Semantic/Conversion_Spec.enso b/test/Base_Tests/src/Semantic/Conversion_Spec.enso index 20deee602d5e..ab436844ea73 100644 --- a/test/Base_Tests/src/Semantic/Conversion_Spec.enso +++ b/test/Base_Tests/src/Semantic/Conversion_Spec.enso @@ -82,12 +82,6 @@ type Fool Fool.from (that : Any) = Fool.Value that -type Fool_Comparator - compare x:Fool y:Fool = Ordering.compare x.fool y.fool - hash x:Fool = Default_Comparator.hash x.fool - -Comparable.from (_:Fool) = Fool_Comparator - type Blob Text c:Text Binary b:File @@ -297,10 +291,10 @@ add_specs suite_builder = x.fool . should_equal 42 x==x . should_be_true (x:Integer)==42 . should_be_true - (x:Fool)==42 . should_be_true # as 42 can be converted to Fool 42! + (x:Fool)==42 . should_be_false x==42 . should_be_true 42==(x.to Integer) . should_be_true - 42==(x.to Fool) . should_be_true # as 42 can be converted to Fool 42! + 42==(x.to Fool) . should_be_false 42==x . should_be_true 100+(x:Integer) . should_equal 142 (x:Integer)+100 . should_equal 142 @@ -318,10 +312,10 @@ add_specs suite_builder = x.fool . should_equal 42.3 x==x . should_be_true (x:Float)==42.3 . should_be_true - (x:Fool)==42.3 . should_be_true # as 42.3 can be converted to Fool 42.3! + (x:Fool)==42.3 . should_be_false x==42.3 . should_be_true 42.3==(x.to Float) . should_be_true - 42.3==(x.to Fool) . should_be_true # as 42.3 can be converted to Fool 42.3! + 42.3==(x.to Fool) . should_be_false 42.3==x . should_be_true 100+(x:Float) . should_equal 142.3 (x:Float)+100 . should_equal 142.3 @@ -338,10 +332,10 @@ add_specs suite_builder = x.fool . should_equal True x==x . should_be_true (x:Boolean) . should_be_true - (x:Fool)==True . should_be_true # as True can be converted to Fool True! + (x:Fool)==True . should_be_false x==True . should_be_true True==(x:Boolean) . should_be_true - True==(x:Fool) . should_be_true # as True can be converted to Fool True! + True==(x:Fool) . should_be_false True==x . should_be_true x.to_text . should_equal "{FOOL True}" (x:Fool).to_text . should_equal "{FOOL True}" @@ -355,10 +349,10 @@ add_specs suite_builder = x.fool . should_equal "Hello" x==x . should_be_true (x:Text)=="Hello" . should_be_true - (x:Fool)=="Hello" . should_be_true # as "Hello" can be converted to Fool "Hello"! + (x:Fool)=="Hello" . should_be_false x=="Hello" . should_be_true "Hello"==(x:Text) . should_be_true - "Hello"==(x:Fool) . should_be_true # as "Hello" can be converted to Fool "Hello"! + "Hello"==(x:Fool) . should_be_false "Hello"==x . should_be_true x.to_text . should_equal "Hello" (x:Fool).to_text . should_equal "{FOOL Hello}" @@ -374,10 +368,10 @@ add_specs suite_builder = x.fool . should_equal now x==x . should_be_true (x:Time_Of_Day)==now . should_be_true - (x:Fool)==now . should_be_true # as now can be converted to Fool now! + (x:Fool)==now . should_be_false x==now . should_be_true now==(x:Time_Of_Day) . should_be_true - now==(x:Fool) . should_be_true # as now can be converted to Fool now! + now==(x:Fool) . should_be_false now==x . should_be_true x.to_text . should_equal now.to_text @@ -390,10 +384,10 @@ add_specs suite_builder = x.fool . should_equal now x==x . should_be_true (x:Date)==now . should_be_true - (x:Fool)==now . should_be_true # as now can be converted to Fool now! + (x:Fool)==now . should_be_false x==now . should_be_true now==(x:Date) . should_be_true - now==(x:Fool) . should_be_true # as now can be converted to Fool now! + now==(x:Fool) . should_be_false now==x . should_be_true x.to_text . should_equal "{FOOL "+now.to_text+"}" @@ -406,10 +400,10 @@ add_specs suite_builder = x.fool . should_equal now x==x . should_be_true (x:Date_Time)==now . should_be_true - (x:Fool)==now . should_be_true # as now can be converted to Fool now! + (x:Fool)==now . should_be_false x==now . should_be_true now==(x:Date_Time) . should_be_true - now==(x:Fool) . should_be_true # as now can be converted to Fool now! + now==(x:Fool) . should_be_false now==x . should_be_true x.to_text . should_equal now.to_text @@ -422,10 +416,10 @@ add_specs suite_builder = x.fool . should_equal now x==x . should_be_true (x:Duration)==now . should_be_true - (x:Fool)==now . should_be_true # as now can be converted to Fool now! + (x:Fool)==now . should_be_false x==now . should_be_true now==(x:Duration) . should_be_true - now==(x:Fool) . should_be_true # as now can be converted to Fool now! + now==(x:Fool) . should_be_false now==x . should_be_true x.to_text . should_equal "{FOOL "+now.to_text+"}" From 0e249c00ab9ef32dc64d970973d772802acbcd00 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Fri, 16 Feb 2024 08:37:36 +0100 Subject: [PATCH 14/27] Single letter typo fix --- .../java/org/enso/interpreter/runtime/data/EnsoMultiValue.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/EnsoMultiValue.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/EnsoMultiValue.java index 1b3550f2aaa9..1eb2c679251d 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/EnsoMultiValue.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/EnsoMultiValue.java @@ -369,7 +369,7 @@ Object getMembers( try { var members = iop.getMembers(values[i]); var len = iop.getArraySize(members); - for (var j = 0L; j < len; i++) { + for (var j = 0L; j < len; j++) { var name = iop.readArrayElement(members, j); names.add(iop.asString(name)); } From a5c0674c3b99d9f9d04fd88792581996ebe7e299 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Fri, 16 Feb 2024 08:38:58 +0100 Subject: [PATCH 15/27] Both from conversions must be present in the module defining Complex type --- .../expression/builtin/meta/EqualsNode.java | 72 +++++++++++++++---- test/Base_Tests/src/Data/Numbers_Spec.enso | 1 + .../src/Semantic/Conversion_Spec.enso | 2 + 3 files changed, 60 insertions(+), 15 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 28e4d2373041..08a627c42a61 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 @@ -12,12 +12,15 @@ import com.oracle.truffle.api.nodes.Node; import org.enso.interpreter.dsl.AcceptsError; import org.enso.interpreter.dsl.BuiltinMethod; +import org.enso.interpreter.node.EnsoRootNode; import org.enso.interpreter.node.callable.InteropConversionCallNode; import org.enso.interpreter.runtime.EnsoContext; import org.enso.interpreter.runtime.callable.UnresolvedConversion; +import org.enso.interpreter.runtime.callable.function.Function; import org.enso.interpreter.runtime.data.Type; import org.enso.interpreter.runtime.error.PanicException; import org.enso.interpreter.runtime.library.dispatch.TypesLibrary; +import org.enso.interpreter.runtime.scope.ModuleScope; import org.enso.interpreter.runtime.state.State; @BuiltinMethod( @@ -91,8 +94,8 @@ public boolean execute( convert1 = insert(WithConversionNode.create()); convert2 = insert(WithConversionNode.create()); } - return convert1.executeThatConversion(frame, other, self) - || convert2.executeThatConversion(frame, self, other); + return convert1.executeWithConversion(frame, other, self) + || convert2.executeWithConversion(frame, self, other); } } return areEqual; @@ -114,7 +117,7 @@ static WithConversionNode create() { * @return {code false} if the conversion makes no sense or result of equality check after doing * the conversion */ - abstract boolean executeThatConversion(VirtualFrame frame, Object self, Object that); + abstract boolean executeWithConversion(VirtualFrame frame, Object self, Object that); static Type findType(TypeOfNode typeOfNode, Object obj) { var rawType = typeOfNode.execute(obj); @@ -125,15 +128,49 @@ static Type findTypeUncached(Object obj) { return findType(TypeOfNode.getUncached(), obj); } + record Convert(Function f1, Function f2, Function f3) {} + + private static boolean isDefinedIn(ModuleScope scope, Function fn) { + if (fn.getCallTarget().getRootNode() instanceof EnsoRootNode ensoRoot) { + return ensoRoot.getModuleScope() == scope; + } else { + return false; + } + } + + Convert findConversions(Type selfType, Type thatType) { + if (selfType == null || thatType == null) { + return null; + } + var ctx = EnsoContext.get(this); + var comparableType = ctx.getBuiltins().comparable().getType(); + + var selfScope = selfType.getDefinitionScope(); + + var fromSelfType = + UnresolvedConversion.build(selfScope).resolveFor(ctx, comparableType, selfType); + var fromThatType = + UnresolvedConversion.build(selfScope).resolveFor(ctx, comparableType, thatType); + var betweenBoth = UnresolvedConversion.build(selfScope).resolveFor(ctx, selfType, thatType); + + if (isDefinedIn(selfScope, fromSelfType) + && isDefinedIn(selfScope, fromThatType) + && betweenBoth != null) { + return new Convert(fromSelfType, fromThatType, betweenBoth); + } else { + return null; + } + } + @Specialization( limit = "3", guards = { - "!types.hasSpecialDispatch(that)", "selfType != null", "thatType != null", + "selfType == findType(typeOfNode, self)", "thatType == findType(typeOfNode, that)" }) - final boolean doThatConversionCached( + final boolean doConversionCached( VirtualFrame frame, Object self, Object that, @@ -143,22 +180,27 @@ final boolean doThatConversionCached( Type selfType, @Cached(value = "findType(typeOfNode, that)", uncached = "findTypeUncached(that)") Type thatType, + @Cached("findConversions(selfType, thatType)") Convert convert, @Shared("convert") @Cached InteropConversionCallNode convertNode, @Shared("invoke") @Cached(allowUncached = true) EqualsSimpleNode equalityNode) { - return doDispatch(frame, self, that, thatType, convertNode, equalityNode); + if (convert == null) { + return false; + } + return doDispatch(frame, self, that, selfType, convertNode, equalityNode); } - @Specialization(replaces = "doThatConversionCached") - final boolean doThatConversionUncached( + @Specialization(replaces = "doConversionCached") + final boolean doConversionUncached( VirtualFrame frame, Object self, Object that, @Shared("typeOf") @Cached TypeOfNode typeOfNode, @Shared("convert") @Cached InteropConversionCallNode convertNode, @Shared("invoke") @Cached(allowUncached = true) EqualsSimpleNode equalityNode) { + var selfType = findType(typeOfNode, self); var thatType = findType(typeOfNode, that); - if (thatType != null) { - var result = doDispatch(frame, self, that, thatType, convertNode, equalityNode); + if (findConversions(selfType, thatType) != null) { + var result = doDispatch(frame, self, that, selfType, convertNode, equalityNode); return result; } return false; @@ -168,18 +210,18 @@ private boolean doDispatch( VirtualFrame frame, Object self, Object that, - Type thatType, + Type selfType, InteropConversionCallNode convertNode, EqualsSimpleNode equalityNode) throws PanicException { - var convert = UnresolvedConversion.build(thatType.getDefinitionScope()); + var convert = UnresolvedConversion.build(selfType.getDefinitionScope()); var ctx = EnsoContext.get(this); var state = State.create(ctx); try { - var selfAsThat = convertNode.execute(convert, state, new Object[] {thatType, self}); - var result = equalityNode.execute(frame, selfAsThat, that); - assert !result || assertHashCodeIsTheSame(self, selfAsThat); + var thatAsSelf = convertNode.execute(convert, state, new Object[] {selfType, that}); + var result = equalityNode.execute(frame, self, thatAsSelf); + assert !result || assertHashCodeIsTheSame(self, thatAsSelf); return result; } catch (ArityException ex) { var assertsOn = false; diff --git a/test/Base_Tests/src/Data/Numbers_Spec.enso b/test/Base_Tests/src/Data/Numbers_Spec.enso index e8e843b67c66..66adce3d6210 100644 --- a/test/Base_Tests/src/Data/Numbers_Spec.enso +++ b/test/Base_Tests/src/Data/Numbers_Spec.enso @@ -37,6 +37,7 @@ type Complex_Comparator 7*x.re + 11*x.im Comparable.from (_:Complex) = Complex_Comparator +Comparable.from (_:Number) = Complex_Comparator add_specs suite_builder = diff --git a/test/Base_Tests/src/Semantic/Conversion_Spec.enso b/test/Base_Tests/src/Semantic/Conversion_Spec.enso index ab436844ea73..4e33503de7e7 100644 --- a/test/Base_Tests/src/Semantic/Conversion_Spec.enso +++ b/test/Base_Tests/src/Semantic/Conversion_Spec.enso @@ -235,6 +235,8 @@ add_specs suite_builder = # no conversions needed when calling `exchange` methods nine . should_equal one + suite_builder.group "MultiValue Conversions" group_builder-> + group_builder.specify "Requesting Text & Foo" <| check a (n : Text & Foo) = case a of 0 -> n.foo From 145fc55300109787f9c3a0569383468ea1afa761 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 17 Feb 2024 06:36:37 +0100 Subject: [PATCH 16/27] Equalities (and inequalities) with conversions test --- .../test/EqualsConversionsTest.java | 133 ++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsConversionsTest.java diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsConversionsTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsConversionsTest.java new file mode 100644 index 000000000000..bfe82d828419 --- /dev/null +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsConversionsTest.java @@ -0,0 +1,133 @@ +package org.enso.interpreter.test; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.List; +import org.graalvm.polyglot.Context; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +public class EqualsConversionsTest extends TestBase { + private static Context context; + + @BeforeClass + public static void initContextAndData() { + context = createDefaultContext(); + } + + @AfterClass + public static void disposeContext() { + context.close(); + } + + @Test + public void testBasicInequalities() { + var results = + TestBase.evalModule( + context, + """ + from Standard.Base import all + + Text.from (that:Number) = that.to_text + + main = + r0 = "4"+"2" == "42" + r1 = 42 == "42" + r2 = "42" == 42 + [ r0, r1, r2 ] + """) + .as(List.class); + + assertTrue("strings are equal: " + results, (boolean) results.get(0)); + assertFalse("string is not equal to number: " + results, (boolean) results.get(1)); + assertFalse("number is not equal to string: " + results, (boolean) results.get(2)); + } + + @Test + public void testNumWrapperAroundIntegerIsEqualToInteger() { + var gen = new DefineComparableWrapper(); + gen.intNumConversion = true; + gen.intComparator = true; + gen.numComparator = true; + assertTrue("Num.Value equal to Integer: ", gen.evaluate()); + } + + @Test + public void testMissingIntegerNumConversion() { + var gen = new DefineComparableWrapper(); + gen.intNumConversion = false; + gen.intComparator = true; + gen.numComparator = true; + assertFalse("Num.Value not equal to Integer: ", gen.evaluate()); + } + + @Test + public void testMissingIntComparator() { + var gen = new DefineComparableWrapper(); + gen.intNumConversion = true; + gen.intComparator = false; + gen.numComparator = true; + assertFalse("Num.Value not equal to Integer: ", gen.evaluate()); + } + + @Test + public void testMissingNumComparator() { + var gen = new DefineComparableWrapper(); + gen.intNumConversion = true; + gen.intComparator = true; + gen.numComparator = false; + assertFalse("Num.Value not equal to Integer: ", gen.evaluate()); + } + + private static final class DefineComparableWrapper { + boolean intNumConversion; + boolean numComparator; + boolean intComparator; + + boolean evaluate() { + var block0 = + """ + from Standard.Base import all + + type Num + Value n:Integer + + type Num_Comparator + compare x:Num y:Num = Ordering.compare x.n y.n + hash x:Num = x.n + """; + + var block1 = + !intNumConversion + ? "" + : """ + Num.from (that:Integer) = Num.Value that + """; + + var block2 = + !numComparator + ? "" + : """ + Comparable.from (_:Num) = Num_Comparator + """; + + var block3 = + !intComparator ? "" : """ + Comparable.from (_:Integer) = Num_Comparator + """; + + var mainBlock = + """ + main = + num42 = Num.Value 42 + + r0 = 42 == num42 + r0 + """; + var res = TestBase.evalModule(context, block0 + block1 + block2 + block3 + mainBlock); + return res.asBoolean(); + } + } +} From 6fac9616425bc37b1aa7df0ca50acd2846d54fe0 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 17 Feb 2024 06:53:23 +0100 Subject: [PATCH 17/27] Ensure inconsistency of hash is reported when assert is on --- .../test/EqualsConversionsTest.java | 26 +++++++++++++++++-- .../expression/builtin/meta/EqualsNode.java | 22 ++++++++-------- 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsConversionsTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsConversionsTest.java index bfe82d828419..39ad2d9e751f 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsConversionsTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsConversionsTest.java @@ -2,9 +2,11 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.util.List; import org.graalvm.polyglot.Context; +import org.graalvm.polyglot.PolyglotException; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; @@ -81,10 +83,29 @@ public void testMissingNumComparator() { assertFalse("Num.Value not equal to Integer: ", gen.evaluate()); } + @Test + public void testInconsistentHashFunction() { + var gen = new DefineComparableWrapper(); + gen.intNumConversion = true; + gen.intComparator = true; + gen.numComparator = true; + gen.hashFn = "x.n*31"; + + boolean r; + try { + r = gen.evaluate(); + } catch (PolyglotException ex) { + assertTrue(ex.getMessage(), ex.getMessage().contains("Different hash code!")); + return; + } + fail("Expecting assertion error, not: " + r); + } + private static final class DefineComparableWrapper { boolean intNumConversion; boolean numComparator; boolean intComparator; + String hashFn = "Default_Comparator.hash x.n"; boolean evaluate() { var block0 = @@ -96,8 +117,9 @@ boolean evaluate() { type Num_Comparator compare x:Num y:Num = Ordering.compare x.n y.n - hash x:Num = x.n - """; + hash x:Num = {hashFn} + """ + .replace("{hashFn}", hashFn); var block1 = !intNumConversion 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 08a627c42a61..4825f97efa8e 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 @@ -221,7 +221,7 @@ private boolean doDispatch( try { var thatAsSelf = convertNode.execute(convert, state, new Object[] {selfType, that}); var result = equalityNode.execute(frame, self, thatAsSelf); - assert !result || assertHashCodeIsTheSame(self, thatAsSelf); + assert !result || assertHashCodeIsTheSame(that, thatAsSelf); return result; } catch (ArityException ex) { var assertsOn = false; @@ -242,17 +242,17 @@ private boolean assertHashCodeIsTheSame(Object self, Object converted) { var selfHash = HashCodeNode.getUncached().execute(self); var convertedHash = HashCodeNode.getUncached().execute(converted); var ok = selfHash == convertedHash; - var msg = - "Different hash code! Original " - + self - + "[#" - + Long.toHexString(selfHash) - + "] got converted to " - + converted - + "[#" - + Long.toHexString(convertedHash) - + "]"; if (!ok) { + var msg = + "Different hash code! Original " + + self + + "[#" + + Long.toHexString(selfHash) + + "] got converted to " + + converted + + "[#" + + Long.toHexString(convertedHash) + + "]"; var ctx = EnsoContext.get(this); throw ctx.raiseAssertionPanic(this, msg, new AssertionError(msg)); } From 1d16e4004165d915818c3dd028331b46f7f5a8cf Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 17 Feb 2024 07:35:10 +0100 Subject: [PATCH 18/27] Benchmark equality with conversion --- .../benchmarks/semantic/EqualsBenchmarks.java | 50 +++++++++++++++++-- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/engine/runtime-benchmarks/src/main/java/org/enso/interpreter/bench/benchmarks/semantic/EqualsBenchmarks.java b/engine/runtime-benchmarks/src/main/java/org/enso/interpreter/bench/benchmarks/semantic/EqualsBenchmarks.java index 7b7c9d8e1300..2fc41e55bfba 100644 --- a/engine/runtime-benchmarks/src/main/java/org/enso/interpreter/bench/benchmarks/semantic/EqualsBenchmarks.java +++ b/engine/runtime-benchmarks/src/main/java/org/enso/interpreter/bench/benchmarks/semantic/EqualsBenchmarks.java @@ -75,6 +75,21 @@ public void initializeBenchmark(BenchmarkParams params) throws Exception { new StringBuilder( """ import Standard.Base.Data.Range.Extensions + import Standard.Base.Data.Numbers.Number + import Standard.Base.Data.Ordering.Ordering + import Standard.Base.Data.Ordering.Comparable + import Standard.Base.Data.Ordering.Default_Comparator + + type Num + Value n + + Num.from (that:Number) = Num.Value that + Comparable.from (_:Number) = Num_Comparator + Comparable.from (_:Num) = Num_Comparator + + type Num_Comparator + compare x:Num y:Num = Ordering.compare x.n y.n + hash x:Num = Default_Comparator.hash x.n type Node C1 f1 @@ -108,10 +123,31 @@ public void initializeBenchmark(BenchmarkParams params) throws Exception { primitiveVectorSize / 64); codeBuilder .append( - generateVectorOfPrimitives(primitiveVectorSize, "vec1", 42, trueExpectedAt, random)) + generateVectorOfPrimitives( + primitiveVectorSize, "vec1", 42, trueExpectedAt, random, "%f")) + .append("\n") + .append( + generateVectorOfPrimitives( + primitiveVectorSize, "vec2", 42, trueExpectedAt, random, "%f")) + .append("\n"); + } + case "equalsWithConversion" -> { + trueExpectedAt = + Set.of( + primitiveVectorSize / 2, + primitiveVectorSize / 4, + primitiveVectorSize / 8, + primitiveVectorSize / 16, + primitiveVectorSize / 32, + primitiveVectorSize / 64); + codeBuilder + .append( + generateVectorOfPrimitives( + primitiveVectorSize, "vec1", 42, trueExpectedAt, random, "%f")) .append("\n") .append( - generateVectorOfPrimitives(primitiveVectorSize, "vec2", 42, trueExpectedAt, random)) + generateVectorOfPrimitives( + primitiveVectorSize, "vec2", 42, trueExpectedAt, random, "Num.Value %f")) .append("\n"); } case "equalsStrings" -> { @@ -171,6 +207,11 @@ public void equalsPrimitives(Blackhole blackHole) { performBenchmark(blackHole); } + @Benchmark + public void equalsWithConversion(Blackhole blackHole) { + performBenchmark(blackHole); + } + @Benchmark public void equalsStrings(Blackhole blackhole) { performBenchmark(blackhole); @@ -207,7 +248,8 @@ private static String generateVectorOfPrimitives( String vecName, Object identityElem, Collection constantIdxs, - Random random) { + Random random, + String format) { var partSize = totalSize / 2; List primitiveValues = new ArrayList<>(); random.ints(partSize).forEach(primitiveValues::add); @@ -221,7 +263,7 @@ private static String generateVectorOfPrimitives( sb.append(vecName).append(" = ["); for (Object primitiveValue : primitiveValues) { if (primitiveValue instanceof Double dbl) { - sb.append(String.format("%f", dbl)).append(","); + sb.append(String.format(format, dbl)).append(","); } else { sb.append(primitiveValue).append(","); } From 866425c13a33f17eb56ab04bb3c40bcd39920633 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 17 Feb 2024 07:57:27 +0100 Subject: [PATCH 19/27] Wrap also non-double values --- .../benchmarks/semantic/EqualsBenchmarks.java | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/engine/runtime-benchmarks/src/main/java/org/enso/interpreter/bench/benchmarks/semantic/EqualsBenchmarks.java b/engine/runtime-benchmarks/src/main/java/org/enso/interpreter/bench/benchmarks/semantic/EqualsBenchmarks.java index 2fc41e55bfba..363819fef5ee 100644 --- a/engine/runtime-benchmarks/src/main/java/org/enso/interpreter/bench/benchmarks/semantic/EqualsBenchmarks.java +++ b/engine/runtime-benchmarks/src/main/java/org/enso/interpreter/bench/benchmarks/semantic/EqualsBenchmarks.java @@ -124,11 +124,11 @@ public void initializeBenchmark(BenchmarkParams params) throws Exception { codeBuilder .append( generateVectorOfPrimitives( - primitiveVectorSize, "vec1", 42, trueExpectedAt, random, "%f")) + primitiveVectorSize, "vec1", 42, trueExpectedAt, random, "%d", "%f")) .append("\n") .append( generateVectorOfPrimitives( - primitiveVectorSize, "vec2", 42, trueExpectedAt, random, "%f")) + primitiveVectorSize, "vec2", 42, trueExpectedAt, random, "%d", "%f")) .append("\n"); } case "equalsWithConversion" -> { @@ -143,11 +143,17 @@ public void initializeBenchmark(BenchmarkParams params) throws Exception { codeBuilder .append( generateVectorOfPrimitives( - primitiveVectorSize, "vec1", 42, trueExpectedAt, random, "%f")) + primitiveVectorSize, "vec1", 42, trueExpectedAt, random, "%d", "%f")) .append("\n") .append( generateVectorOfPrimitives( - primitiveVectorSize, "vec2", 42, trueExpectedAt, random, "Num.Value %f")) + primitiveVectorSize, + "vec2", + 42, + trueExpectedAt, + random, + "Num.Value %d", + "Num.Value %f")) .append("\n"); } case "equalsStrings" -> { @@ -177,7 +183,7 @@ public void initializeBenchmark(BenchmarkParams params) throws Exception { } codeBuilder.append(""" - bench x = eq_vec vec1 vec2 + bench _ = eq_vec vec1 vec2 """); module = ctx.eval(SrcUtil.source(benchmarkName, codeBuilder.toString())); @@ -249,7 +255,8 @@ private static String generateVectorOfPrimitives( Object identityElem, Collection constantIdxs, Random random, - String format) { + String intFormat, + String doubleFormat) { var partSize = totalSize / 2; List primitiveValues = new ArrayList<>(); random.ints(partSize).forEach(primitiveValues::add); @@ -263,9 +270,9 @@ private static String generateVectorOfPrimitives( sb.append(vecName).append(" = ["); for (Object primitiveValue : primitiveValues) { if (primitiveValue instanceof Double dbl) { - sb.append(String.format(format, dbl)).append(","); + sb.append(String.format(doubleFormat, dbl)).append(","); } else { - sb.append(primitiveValue).append(","); + sb.append(String.format(intFormat, primitiveValue)).append(","); } } // Replace last comma From 3524bd9cc095ba487a24e0142728b736e1c19637 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 17 Feb 2024 08:16:49 +0100 Subject: [PATCH 20/27] Place the == on a separate line to make it more easily visible in IGV --- .../bench/benchmarks/semantic/EqualsBenchmarks.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/engine/runtime-benchmarks/src/main/java/org/enso/interpreter/bench/benchmarks/semantic/EqualsBenchmarks.java b/engine/runtime-benchmarks/src/main/java/org/enso/interpreter/bench/benchmarks/semantic/EqualsBenchmarks.java index 363819fef5ee..0b739c96731d 100644 --- a/engine/runtime-benchmarks/src/main/java/org/enso/interpreter/bench/benchmarks/semantic/EqualsBenchmarks.java +++ b/engine/runtime-benchmarks/src/main/java/org/enso/interpreter/bench/benchmarks/semantic/EqualsBenchmarks.java @@ -102,7 +102,9 @@ public void initializeBenchmark(BenchmarkParams params) throws Exception { eq_vec vec1 vec2 = (0.up_to vec1.length).map idx-> - (vec1.at idx) == (vec2.at idx) + v1 = vec1.at idx + v2 = vec2.at idx + v1 == v2 eq x y = x == y """); From 2d708de2878ac70c57e4604e853a5608af65f9ef Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Mon, 19 Feb 2024 09:58:49 +0100 Subject: [PATCH 21/27] Reference to binary operator multiple dispatch --- docs/types/dynamic-dispatch.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/types/dynamic-dispatch.md b/docs/types/dynamic-dispatch.md index f715b8d9a27f..3c0172b3922b 100644 --- a/docs/types/dynamic-dispatch.md +++ b/docs/types/dynamic-dispatch.md @@ -63,10 +63,13 @@ another. ## Multiple Dispatch -It is an open question as to whether we want to support proper multiple dispatch -in Enso. Multiple dispatch refers to the dynamic dispatch target being -determined based not only on the type of the `this` argument, but the types of -the other arguments to the function. +Multiple dispatch is currently used for +[binary operators](../syntax/functions.md#type-ascriptions-and-operator-resolution). + +Supporting for general functions remains an open question as to whether we want +to support proper multiple dispatch in Enso. Multiple dispatch refers to the +dynamic dispatch target being determined based not only on the type of the +`this` argument, but the types of the other arguments to the function. To do multiple dispatch properly, it is very important to get a rigorous specification of the specificity algorithm. It must account for: From 5d52d518fe18dd6ed150d444d0f1225e8e462358 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Mon, 19 Feb 2024 12:19:59 +0100 Subject: [PATCH 22/27] Describing multi-type custom equality --- docs/syntax/functions.md | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/docs/syntax/functions.md b/docs/syntax/functions.md index a4e332bb53cf..7554d395871f 100644 --- a/docs/syntax/functions.md +++ b/docs/syntax/functions.md @@ -304,6 +304,44 @@ the addition by invoking `Num.+`. This behavior allows one to write libraries that extend existing number types with `Complex_Number`, `Rational_Number` and make them behave as first class citizen numbers. +### Custom Equality + +The `==` operator is special. A consistency with hash code is necessary to make +any Enso object behave correctly and work effectively in `Set` and `Map` +implementations. To guarantee such level of consistency there is a `Any.==` +definition providing _universal equality_ that **shall not be overriden**. + +The `==` behavior is predefined for builtin types, atoms and other Enso objects. +In addition to that it remains possible to define own _comparators_, including a +comparator capable to work with already existing types. To create such +comparator define: + +- conversion between existing type and the new type (as described in + [previous section](#type-ascriptions-and-operator-resolution)) +- comparator (see documentation of `Ordering` type) +- define **two conversion method** that return the same comparator + +To extend the previous definition of `Num` also for equality one might do for +example: + +```ruby +type Num_Comparator + compare a:Num b:Num = # compare somehow + hash a:Num = # hash somehow + +Num.from (that:Integer) = # convert somehow +Comparable.from (_:Num) = Num_Comparator +Comparable.from (_:Integer) = Num_Comparator +``` + +with such a structure the internal implementation of `Any.==` performs necessary +conversions of `Integer` argument in case the other argument is `Num` and +invokes the `Num_Comparator.compare` to handle the comparision. + +A care must be taken to keep consistency between `hash` values of original and +converted types - e.g. hash of `n:Integer` and hash of `Num.from n` must be the +same (otherwise consistency required for `Set` and `Map` would be compromised). + ### Precedence Operator precedence in Enso is a collection of rules that reflect conventions From 0b7f55c93c7619f29f4efb66db495ef73cb581d4 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Mon, 19 Feb 2024 13:17:26 +0100 Subject: [PATCH 23/27] Verify Comparator for both types is the same --- .../test/EqualsConversionsTest.java | 27 +++++++++++++++---- .../builtin/meta/EqualsComplexNode.java | 2 -- .../expression/builtin/meta/EqualsNode.java | 24 ++++++++++++----- 3 files changed, 40 insertions(+), 13 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsConversionsTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsConversionsTest.java index 39ad2d9e751f..05c0dcfddfc4 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsConversionsTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsConversionsTest.java @@ -1,13 +1,13 @@ package org.enso.interpreter.test; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - import java.util.List; + import org.graalvm.polyglot.Context; import org.graalvm.polyglot.PolyglotException; import org.junit.AfterClass; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import org.junit.BeforeClass; import org.junit.Test; @@ -83,6 +83,22 @@ public void testMissingNumComparator() { assertFalse("Num.Value not equal to Integer: ", gen.evaluate()); } + @Test + public void testDifferentComparators() { + var gen = new DefineComparableWrapper(); + gen.intNumConversion = true; + gen.intComparator = true; + gen.numComparator = false; + gen.extraBlock = """ + type Second_Comparator + compare a:Num b:Num = Num_Comparator.compare a b + hash a:Num = Num_Comparator.hash a + + Comparable.from (_:Num) = Second_Comparator + """; + assertFalse("Num.Value not equal to Integer: ", gen.evaluate()); + } + @Test public void testInconsistentHashFunction() { var gen = new DefineComparableWrapper(); @@ -106,6 +122,7 @@ private static final class DefineComparableWrapper { boolean numComparator; boolean intComparator; String hashFn = "Default_Comparator.hash x.n"; + String extraBlock = ""; boolean evaluate() { var block0 = @@ -148,7 +165,7 @@ boolean evaluate() { r0 = 42 == num42 r0 """; - var res = TestBase.evalModule(context, block0 + block1 + block2 + block3 + mainBlock); + var res = TestBase.evalModule(context, block0 + block1 + block2 + block3 + mainBlock + extraBlock); return res.asBoolean(); } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsComplexNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsComplexNode.java index bd1487d5c6a5..bada651826ad 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsComplexNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/EqualsComplexNode.java @@ -20,7 +20,6 @@ import java.time.LocalDateTime; import java.time.ZonedDateTime; import org.enso.interpreter.node.expression.builtin.interop.syntax.HostValueToEnsoNode; -import org.enso.interpreter.node.expression.builtin.ordering.CustomComparatorNode; import org.enso.interpreter.runtime.EnsoContext; import org.enso.interpreter.runtime.Module; import org.enso.interpreter.runtime.callable.UnresolvedConversion; @@ -291,7 +290,6 @@ boolean equalsArrays( @CachedLibrary("selfArray") InteropLibrary selfInterop, @CachedLibrary("otherArray") InteropLibrary otherInterop, @Shared("equalsNode") @Cached EqualsNode equalsNode, - @Cached CustomComparatorNode hasCustomComparatorNode, @Shared("hostValueToEnsoNode") @Cached HostValueToEnsoNode valueToEnsoNode) { try { long selfSize = selfInterop.getArraySize(selfArray); 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 4825f97efa8e..12792ce3e707 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 @@ -8,18 +8,20 @@ import com.oracle.truffle.api.dsl.Specialization; import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.interop.ArityException; -import com.oracle.truffle.api.library.CachedLibrary; import com.oracle.truffle.api.nodes.Node; import org.enso.interpreter.dsl.AcceptsError; import org.enso.interpreter.dsl.BuiltinMethod; import org.enso.interpreter.node.EnsoRootNode; import org.enso.interpreter.node.callable.InteropConversionCallNode; +import org.enso.interpreter.node.callable.InvokeCallableNode.ArgumentsExecutionMode; +import org.enso.interpreter.node.callable.InvokeCallableNode.DefaultsExecutionMode; +import org.enso.interpreter.node.callable.dispatch.InvokeFunctionNode; import org.enso.interpreter.runtime.EnsoContext; import org.enso.interpreter.runtime.callable.UnresolvedConversion; +import org.enso.interpreter.runtime.callable.argument.CallArgumentInfo; import org.enso.interpreter.runtime.callable.function.Function; import org.enso.interpreter.runtime.data.Type; import org.enso.interpreter.runtime.error.PanicException; -import org.enso.interpreter.runtime.library.dispatch.TypesLibrary; import org.enso.interpreter.runtime.scope.ModuleScope; import org.enso.interpreter.runtime.state.State; @@ -138,7 +140,17 @@ private static boolean isDefinedIn(ModuleScope scope, Function fn) { } } - Convert findConversions(Type selfType, Type thatType) { + private static Object convertor(EnsoContext ctx, Function convFn, Object value) { + var argSchema = new CallArgumentInfo[] {new CallArgumentInfo(), new CallArgumentInfo()}; + var node = + InvokeFunctionNode.build( + argSchema, DefaultsExecutionMode.EXECUTE, ArgumentsExecutionMode.EXECUTE); + var state = State.create(ctx); + return node.execute( + convFn, null, state, new Object[] {ctx.getBuiltins().comparable(), value}); + } + + Convert findConversions(Type selfType, Type thatType, Object self, Object that) { if (selfType == null || thatType == null) { return null; } @@ -155,6 +167,7 @@ Convert findConversions(Type selfType, Type thatType) { if (isDefinedIn(selfScope, fromSelfType) && isDefinedIn(selfScope, fromThatType) + && convertor(ctx, fromSelfType, self) == convertor(ctx, fromThatType, that) && betweenBoth != null) { return new Convert(fromSelfType, fromThatType, betweenBoth); } else { @@ -175,12 +188,11 @@ final boolean doConversionCached( Object self, Object that, @Shared("typeOf") @Cached TypeOfNode typeOfNode, - @CachedLibrary(limit = "3") TypesLibrary types, @Cached(value = "findType(typeOfNode, self)", uncached = "findTypeUncached(self)") Type selfType, @Cached(value = "findType(typeOfNode, that)", uncached = "findTypeUncached(that)") Type thatType, - @Cached("findConversions(selfType, thatType)") Convert convert, + @Cached("findConversions(selfType, thatType, self, that)") Convert convert, @Shared("convert") @Cached InteropConversionCallNode convertNode, @Shared("invoke") @Cached(allowUncached = true) EqualsSimpleNode equalityNode) { if (convert == null) { @@ -199,7 +211,7 @@ final boolean doConversionUncached( @Shared("invoke") @Cached(allowUncached = true) EqualsSimpleNode equalityNode) { var selfType = findType(typeOfNode, self); var thatType = findType(typeOfNode, that); - if (findConversions(selfType, thatType) != null) { + if (findConversions(selfType, thatType, self, that) != null) { var result = doDispatch(frame, self, that, selfType, convertNode, equalityNode); return result; } From 8c88cc1d2811fc1dcbb09f8bcfc5b06d1a87c83e Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Mon, 19 Feb 2024 13:32:31 +0100 Subject: [PATCH 24/27] Use only one WithConversion node --- .../expression/builtin/meta/EqualsNode.java | 45 ++++++++++++------- 1 file changed, 30 insertions(+), 15 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 12792ce3e707..883e71ad89dd 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,8 +45,7 @@ public final class EqualsNode extends Node { @Child private EqualsSimpleNode node; @Child private TypeOfNode types; - @Child private WithConversionNode convert1; - @Child private WithConversionNode convert2; + @Child private WithConversionNode convert; private static final EqualsNode UNCACHED = new EqualsNode(EqualsSimpleNodeGen.getUncached(), TypeOfNode.getUncached(), true); @@ -55,8 +54,7 @@ private EqualsNode(EqualsSimpleNode node, TypeOfNode types, boolean uncached) { this.node = node; this.types = types; if (uncached) { - convert1 = EqualsNodeFactory.WithConversionNodeGen.getUncached(); - convert2 = convert1; + convert = EqualsNodeFactory.WithConversionNodeGen.getUncached(); } } @@ -91,13 +89,11 @@ public boolean execute( var selfType = types.execute(self); var otherType = types.execute(other); if (selfType != otherType) { - if (convert1 == null) { + if (convert == null) { CompilerDirectives.transferToInterpreter(); - convert1 = insert(WithConversionNode.create()); - convert2 = insert(WithConversionNode.create()); + convert = insert(WithConversionNode.create()); } - return convert1.executeWithConversion(frame, other, self) - || convert2.executeWithConversion(frame, self, other); + return convert.executeWithConversion(frame, other, self); } } return areEqual; @@ -130,7 +126,7 @@ static Type findTypeUncached(Object obj) { return findType(TypeOfNode.getUncached(), obj); } - record Convert(Function f1, Function f2, Function f3) {} + record Convert(boolean flip, Function f1, Function f2, Function f3) {} private static boolean isDefinedIn(ModuleScope scope, Function fn) { if (fn.getCallTarget().getRootNode() instanceof EnsoRootNode ensoRoot) { @@ -155,9 +151,20 @@ Convert findConversions(Type selfType, Type thatType, Object self, Object that) return null; } var ctx = EnsoContext.get(this); - var comparableType = ctx.getBuiltins().comparable().getType(); + var c1 = findConversionImpl(ctx, false, selfType, thatType, self, that); + if (c1 != null) { + return c1; + } else { + var c2 = findConversionImpl(ctx, true, thatType, selfType, that, self); + return c2; + } + } + + private static Convert findConversionImpl( + EnsoContext ctx, boolean flip, Type selfType, Type thatType, Object self, Object that) { var selfScope = selfType.getDefinitionScope(); + var comparableType = ctx.getBuiltins().comparable().getType(); var fromSelfType = UnresolvedConversion.build(selfScope).resolveFor(ctx, comparableType, selfType); @@ -169,7 +176,7 @@ Convert findConversions(Type selfType, Type thatType, Object self, Object that) && isDefinedIn(selfScope, fromThatType) && convertor(ctx, fromSelfType, self) == convertor(ctx, fromThatType, that) && betweenBoth != null) { - return new Convert(fromSelfType, fromThatType, betweenBoth); + return new Convert(flip, fromSelfType, fromThatType, betweenBoth); } else { return null; } @@ -198,7 +205,11 @@ final boolean doConversionCached( if (convert == null) { return false; } - return doDispatch(frame, self, that, selfType, convertNode, equalityNode); + if (convert.flip) { + return doDispatch(frame, that, self, thatType, convertNode, equalityNode); + } else { + return doDispatch(frame, self, that, selfType, convertNode, equalityNode); + } } @Specialization(replaces = "doConversionCached") @@ -211,8 +222,12 @@ final boolean doConversionUncached( @Shared("invoke") @Cached(allowUncached = true) EqualsSimpleNode equalityNode) { var selfType = findType(typeOfNode, self); var thatType = findType(typeOfNode, that); - if (findConversions(selfType, thatType, self, that) != null) { - var result = doDispatch(frame, self, that, selfType, convertNode, equalityNode); + var conv = findConversions(selfType, thatType, self, that); + if (conv != null) { + var result = + conv.flip + ? doDispatch(frame, that, self, thatType, convertNode, equalityNode) + : doDispatch(frame, self, that, selfType, convertNode, equalityNode); return result; } return false; From 12fd92536a03730b2fa59dc55ba5ae1ff87cd07b Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Mon, 19 Feb 2024 14:00:53 +0100 Subject: [PATCH 25/27] No need for Convert record. Boolean is enough. --- .../test/EqualsConversionsTest.java | 14 ++++---- .../expression/builtin/meta/EqualsNode.java | 35 ++++++++++--------- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsConversionsTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsConversionsTest.java index 05c0dcfddfc4..c47144e646c7 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsConversionsTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/EqualsConversionsTest.java @@ -1,13 +1,13 @@ package org.enso.interpreter.test; -import java.util.List; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import java.util.List; import org.graalvm.polyglot.Context; import org.graalvm.polyglot.PolyglotException; import org.junit.AfterClass; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import org.junit.BeforeClass; import org.junit.Test; @@ -89,7 +89,8 @@ public void testDifferentComparators() { gen.intNumConversion = true; gen.intComparator = true; gen.numComparator = false; - gen.extraBlock = """ + gen.extraBlock = + """ type Second_Comparator compare a:Num b:Num = Num_Comparator.compare a b hash a:Num = Num_Comparator.hash a @@ -165,7 +166,8 @@ boolean evaluate() { r0 = 42 == num42 r0 """; - var res = TestBase.evalModule(context, block0 + block1 + block2 + block3 + mainBlock + extraBlock); + var res = + TestBase.evalModule(context, block0 + block1 + block2 + block3 + mainBlock + extraBlock); return res.asBoolean(); } } 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 883e71ad89dd..8a56ab67181a 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 @@ -126,8 +126,6 @@ static Type findTypeUncached(Object obj) { return findType(TypeOfNode.getUncached(), obj); } - record Convert(boolean flip, Function f1, Function f2, Function f3) {} - private static boolean isDefinedIn(ModuleScope scope, Function fn) { if (fn.getCallTarget().getRootNode() instanceof EnsoRootNode ensoRoot) { return ensoRoot.getModuleScope() == scope; @@ -146,23 +144,28 @@ private static Object convertor(EnsoContext ctx, Function convFn, Object value) convFn, null, state, new Object[] {ctx.getBuiltins().comparable(), value}); } - Convert findConversions(Type selfType, Type thatType, Object self, Object that) { + /** + * @return {@code null} if no conversion found + */ + Boolean findConversions(Type selfType, Type thatType, Object self, Object that) { if (selfType == null || thatType == null) { return null; } var ctx = EnsoContext.get(this); - var c1 = findConversionImpl(ctx, false, selfType, thatType, self, that); - if (c1 != null) { - return c1; + if (findConversionImpl(ctx, selfType, thatType, self, that)) { + return false; } else { - var c2 = findConversionImpl(ctx, true, thatType, selfType, that, self); - return c2; + if (findConversionImpl(ctx, thatType, selfType, that, self)) { + return true; + } else { + return null; + } } } - private static Convert findConversionImpl( - EnsoContext ctx, boolean flip, Type selfType, Type thatType, Object self, Object that) { + private static boolean findConversionImpl( + EnsoContext ctx, Type selfType, Type thatType, Object self, Object that) { var selfScope = selfType.getDefinitionScope(); var comparableType = ctx.getBuiltins().comparable().getType(); @@ -176,14 +179,14 @@ private static Convert findConversionImpl( && isDefinedIn(selfScope, fromThatType) && convertor(ctx, fromSelfType, self) == convertor(ctx, fromThatType, that) && betweenBoth != null) { - return new Convert(flip, fromSelfType, fromThatType, betweenBoth); + return true; } else { - return null; + return false; } } @Specialization( - limit = "3", + limit = "10", guards = { "selfType != null", "thatType != null", @@ -199,13 +202,13 @@ final boolean doConversionCached( Type selfType, @Cached(value = "findType(typeOfNode, that)", uncached = "findTypeUncached(that)") Type thatType, - @Cached("findConversions(selfType, thatType, self, that)") Convert convert, + @Cached("findConversions(selfType, thatType, self, that)") Boolean convert, @Shared("convert") @Cached InteropConversionCallNode convertNode, @Shared("invoke") @Cached(allowUncached = true) EqualsSimpleNode equalityNode) { if (convert == null) { return false; } - if (convert.flip) { + if (convert) { return doDispatch(frame, that, self, thatType, convertNode, equalityNode); } else { return doDispatch(frame, self, that, selfType, convertNode, equalityNode); @@ -225,7 +228,7 @@ final boolean doConversionUncached( var conv = findConversions(selfType, thatType, self, that); if (conv != null) { var result = - conv.flip + conv ? doDispatch(frame, that, self, thatType, convertNode, equalityNode) : doDispatch(frame, self, that, selfType, convertNode, equalityNode); return result; From f9af3ad00f75c64930afa59077688fcdfe2c3ae5 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Mon, 19 Feb 2024 14:16:46 +0100 Subject: [PATCH 26/27] Fixing typo --- docs/types/dynamic-dispatch.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/types/dynamic-dispatch.md b/docs/types/dynamic-dispatch.md index 3c0172b3922b..013ddfc42fa8 100644 --- a/docs/types/dynamic-dispatch.md +++ b/docs/types/dynamic-dispatch.md @@ -66,9 +66,9 @@ another. Multiple dispatch is currently used for [binary operators](../syntax/functions.md#type-ascriptions-and-operator-resolution). -Supporting for general functions remains an open question as to whether we want -to support proper multiple dispatch in Enso. Multiple dispatch refers to the -dynamic dispatch target being determined based not only on the type of the +Supporting it for general functions remains an open question as to whether we +want to support proper multiple dispatch in Enso. Multiple dispatch refers to +the dynamic dispatch target being determined based not only on the type of the `this` argument, but the types of the other arguments to the function. To do multiple dispatch properly, it is very important to get a rigorous From d3510a84c8678deeec2102ae16892d4ee9de0371 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Mon, 19 Feb 2024 15:30:43 +0100 Subject: [PATCH 27/27] Hide InvokeFunctionNode.build behind @TruffleBoundary --- .../interpreter/node/expression/builtin/meta/EqualsNode.java | 1 + 1 file changed, 1 insertion(+) 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 8a56ab67181a..b368fb7fe802 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 @@ -134,6 +134,7 @@ private static boolean isDefinedIn(ModuleScope scope, Function fn) { } } + @CompilerDirectives.TruffleBoundary private static Object convertor(EnsoContext ctx, Function convFn, Object value) { var argSchema = new CallArgumentInfo[] {new CallArgumentInfo(), new CallArgumentInfo()}; var node =