Mixed Decimal/Float operations throw error or attach warning (#10725)

CHANGELOG.md

@@ -14,11 +14,14 @@
   Cloud.][10660]
 - [Added Newline option to Text_Cleanse/Text_Replace.][10761]
 - [Support for reading from Tableau Hyper files.][10733]
+- [Mixed Decimal/Float arithmetic now throws an error; mixed comparisons now
+  attach warnings.][10725]
 
 [10614]: https://github.com/enso-org/enso/pull/10614
 [10660]: https://github.com/enso-org/enso/pull/10660
 [10761]: https://github.com/enso-org/enso/pull/10761
 [10733]: https://github.com/enso-org/enso/pull/10733
+[10725]: https://github.com/enso-org/enso/pull/10725
 
 # Enso 2023.3
 

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Numeric/Internal/Decimal_Internal.enso

@@ -3,6 +3,8 @@ private
 import project.Data.Decimal.Decimal
 import project.Data.Numbers.Number
 import project.Data.Text.Text
+from project.Errors.Common import Loss_Of_Numeric_Precision
+import project.Warning.Warning
 from project.Data.Ordering import Comparable, Ordering
 
 polyglot java import org.enso.base.numeric.Decimal_Utils
@@ -11,4 +13,5 @@ polyglot java import org.enso.base.numeric.Decimal_Utils
 type Decimal_Comparator
     compare (x : Decimal) (y : Decimal) =
         Ordering.from_sign (x.big_decimal.compareTo y.big_decimal)
+
     hash x:Decimal = Decimal_Utils.hashCodeOf x.big_decimal

distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Storage.enso

@@ -2,6 +2,7 @@ from Standard.Base import all
 import Standard.Base.Errors.Common.Index_Out_Of_Bounds
 import Standard.Base.Errors.Illegal_Argument.Illegal_Argument
 import Standard.Base.Errors.Illegal_State.Illegal_State
+from Standard.Base.Data.Decimal import from_big_decimal, get_big_decimal
 
 import project.Column.Column
 import project.Value_Type.Bits
@@ -78,7 +79,7 @@ closest_storage_type value_type = case value_type of
    This step is unnecessary for primitive and builtin values, but necessary for
    values such as `Decimal`/`BigDecimal`.
 enso_to_java x = case x of
-    Decimal.Value big_decimal -> big_decimal
+    _ : Decimal -> get_big_decimal x
     _ -> x
 
 ## PRIVATE
@@ -88,7 +89,7 @@ enso_to_java x = case x of
    necessary for values such as `Decimal`/`BigDecimal`.
 java_to_enso x = case x of
     _ : Nothing -> Nothing
-    _ : BigDecimal -> Decimal.Value x
+    _ : BigDecimal -> from_big_decimal x
     _ -> x
 
 ## PRIVATE

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

@@ -9,6 +9,7 @@
 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.UnsupportedMessageException;
 import com.oracle.truffle.api.library.CachedLibrary;
 import com.oracle.truffle.api.nodes.ExplodeLoop;
 import com.oracle.truffle.api.nodes.Node;
@@ -27,6 +28,7 @@
 import org.enso.interpreter.runtime.data.atom.StructsLibrary;
 import org.enso.interpreter.runtime.library.dispatch.TypeOfNode;
 import org.enso.interpreter.runtime.state.State;
+import org.enso.interpreter.runtime.warning.WarningsLibrary;
 
 @GenerateUncached
 public abstract class EqualsAtomNode extends Node {
@@ -91,16 +93,24 @@ boolean equalsAtomsWithCustomComparator(
       @Cached(value = "customComparatorNode.execute(self)") Type cachedComparator,
       @Cached(value = "findCompareMethod(cachedComparator)", allowUncached = true)
           Function compareFn,
-      @Cached(value = "invokeCompareNode(compareFn)") InvokeFunctionNode invokeNode) {
-    var otherComparator = customComparatorNode.execute(other);
-    if (cachedComparator != otherComparator) {
-      return false;
+      @Cached(value = "invokeCompareNode(compareFn)") InvokeFunctionNode invokeNode,
+      @Shared @CachedLibrary(limit = "10") WarningsLibrary warnings) {
+    try {
+      var otherComparator = customComparatorNode.execute(other);
+      if (cachedComparator != otherComparator) {
+        return false;
+      }
+      var ctx = EnsoContext.get(this);
+      var args = new Object[] {cachedComparator, self, other};
+      var result = invokeNode.execute(compareFn, null, State.create(ctx), args);
+      assert orderingOrNullOrError(this, ctx, result, compareFn);
+      if (warnings.hasWarnings(result)) {
+        result = warnings.removeWarnings(result);
+      }
+      return ctx.getBuiltins().ordering().newEqual() == result;
+    } catch (UnsupportedMessageException e) {
+      throw EnsoContext.get(this).raiseAssertionPanic(this, null, e);
     }
-    var ctx = EnsoContext.get(this);
-    var args = new Object[] {cachedComparator, self, other};
-    var result = invokeNode.execute(compareFn, null, State.create(ctx), args);
-    assert orderingOrNullOrError(this, ctx, result, compareFn);
-    return ctx.getBuiltins().ordering().newEqual() == result;
   }
 
   @TruffleBoundary
@@ -123,16 +133,21 @@ private static boolean orderingOrNullOrError(
 
   @Specialization(
       replaces = {"equalsAtomsWithDefaultComparator", "equalsAtomsWithCustomComparator"})
-  boolean equalsAtomsUncached(VirtualFrame frame, Atom self, Atom other) {
+  boolean equalsAtomsUncached(
+      VirtualFrame frame,
+      Atom self,
+      Atom other,
+      @Shared @CachedLibrary(limit = "10") WarningsLibrary warnings) {
     if (self.getConstructor() != other.getConstructor()) {
       return false;
     } else {
-      return equalsAtomsUncached(frame == null ? null : frame.materialize(), self, other);
+      return equalsAtomsUncached(frame == null ? null : frame.materialize(), self, other, warnings);
     }
   }
 
   @CompilerDirectives.TruffleBoundary
-  private boolean equalsAtomsUncached(MaterializedFrame frame, Atom self, Atom other) {
+  private boolean equalsAtomsUncached(
+      MaterializedFrame frame, Atom self, Atom other, WarningsLibrary warnings) {
     Type customComparator = CustomComparatorNode.getUncached().execute(self);
     if (customComparator != null) {
       Function compareFunc = findCompareMethod(customComparator);
@@ -144,7 +159,8 @@ private boolean equalsAtomsUncached(MaterializedFrame frame, Atom self, Atom oth
           CustomComparatorNode.getUncached(),
           customComparator,
           compareFunc,
-          invokeFuncNode);
+          invokeFuncNode,
+          warnings);
     }
     for (int i = 0; i < self.getConstructor().getArity(); i++) {
       var selfField = StructsLibrary.getUncached().getField(self, i);

test/Base_Internal_Tests/src/Comparator_Spec.enso

@@ -1,5 +1,6 @@
 from Standard.Base import all
 import Standard.Base.Errors.Common.Incomparable_Values
+import Standard.Base.Errors.Illegal_Argument.Illegal_Argument
 
 from Standard.Test import all
 
@@ -34,6 +35,19 @@ type No_Ord_Comparator
 
 Comparable.from (that:No_Ord) = Comparable.new that No_Ord_Comparator
 
+type Attach_Warning
+    Value (n : Integer)
+    Value2 (n : Integer)
+
+type Attach_Warning_Comparator
+    compare (x : Attach_Warning) (y : Attach_Warning) =
+        r = Ordering.compare x.n y.n
+        Warning.attach (Illegal_Argument.Error "warning") r
+
+    hash x:Attach_Warning = Ordering.hash x.n
+
+Comparable.from (that : Attach_Warning) = Comparable.new that Attach_Warning_Comparator
+
 # Tests
 
 add_specs suite_builder = suite_builder.group "Object Comparator" group_builder->
@@ -79,6 +93,11 @@ add_specs suite_builder = suite_builder.group "Object Comparator" group_builder-
         (default_comparator 1 True) . should_fail_with Incomparable_Values
         (default_comparator (No_Ord.Value 1) (No_Ord.Value 2)).should_fail_with Incomparable_Values
 
+    group_builder.specify "warnings attached to the return value of .compare should not affect the boolean value of the comparison operator" <|
+        ((Attach_Warning.Value 1) < (Attach_Warning.Value 2)) . should_be_true
+        ((Attach_Warning.Value 1) == (Attach_Warning.Value 1)) . should_be_true
+        ((Attach_Warning.Value 1) == (Attach_Warning.Value2 1)) . should_be_true
+
 main filter=Nothing =
     suite = Test.build suite_builder->
         add_specs suite_builder

test/Base_Internal_Tests/src/Decimal_Constructor_Spec.enso

@@ -0,0 +1,27 @@
+from Standard.Base import all
+
+from Standard.Test import all
+
+id_decimal (x : Decimal) -> Decimal = x
+
+is_value_constructor d = case d of
+    Decimal.Value _ -> True
+    Decimal.From_Float _ _ -> False
+
+add_specs suite_builder =
+    suite_builder.group "(Decimal_Constructor_Spec) conversions" group_builder->
+        group_builder.specify "Conversion from float to decimal should use the correct constructor" <|
+            is_value_constructor (dec 0.1) . should_be_true
+            is_value_constructor (0.1 . to_decimal) . should_be_true
+            is_value_constructor (0.1 . to_decimal) . should_be_true
+            is_value_constructor (Decimal.from_float 0.1) . should_be_true
+
+            is_value_constructor (Decimal.new 0.1) . should_be_false
+            is_value_constructor (Decimal.from 0.1) . should_be_false
+            is_value_constructor (Decimal.from_float 0.1 explicit=False) . should_be_false
+            is_value_constructor (id_decimal 0.1) . should_be_false
+
+main filter=Nothing =
+    suite = Test.build suite_builder->
+        add_specs suite_builder
+    suite.run_with_filter filter

test/Base_Internal_Tests/src/Main.enso

@@ -4,12 +4,14 @@ from Standard.Test import all
 
 import project.Input_Output_Spec
 import project.Comparator_Spec
+import project.Decimal_Constructor_Spec
 import project.Grapheme_Spec
 
 main filter=Nothing =
     suite = Test.build suite_builder->
-        Input_Output_Spec.add_specs suite_builder
         Comparator_Spec.add_specs suite_builder
+        Decimal_Constructor_Spec.add_specs suite_builder
         Grapheme_Spec.add_specs suite_builder
+        Input_Output_Spec.add_specs suite_builder
 
     suite.run_with_filter filter