From 6b3362957f9db6d26899b1a341ecc6f89189d4f3 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Fri, 23 Aug 2024 17:33:46 +0200 Subject: [PATCH 1/3] Return value from a typed method stays uncoverted if its type fits --- .../argument/ReadArgumentCheckNode.java | 31 +++++++++++-- .../interpreter/runtime/IrToTruffle.scala | 31 ++++++++----- .../src/Semantic/Conversion_Spec.enso | 46 +++++++++++++++++++ 3 files changed, 92 insertions(+), 16 deletions(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/argument/ReadArgumentCheckNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/argument/ReadArgumentCheckNode.java index d61186db70f6..f90ce7419ca4 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/argument/ReadArgumentCheckNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/argument/ReadArgumentCheckNode.java @@ -55,9 +55,26 @@ public abstract class ReadArgumentCheckNode extends Node { this.comment = comment; } - /** */ - public static ExpressionNode wrap(ExpressionNode original, ReadArgumentCheckNode check) { - return new TypeCheckExpressionNode(original, check); + /** + * Wraps {@code original} node with a check for specified type. One can specify whether it is OK + * to accept {@link EnsoMultiValue} without conversion or not. Currently the explicit conversions: + * + * 
+   * x:Integer
+   *
+ * + * extract the Integer component from a multi value (if it is there). However the implicit checks + * like method return type allow multi value to propagage as a whole (if it has the right + * component requested by the type). + * + * @param original node computing the original expression value + * @param check node that performs the type check + * @param acceptWholeMultiValue is it OK to accept multi value? + * @return + */ + public static ExpressionNode wrapWithTypeCheck( + ExpressionNode original, ReadArgumentCheckNode check, boolean acceptWholeMultiValue) { + return new TypeCheckExpressionNode(original, check, acceptWholeMultiValue); } /** @@ -553,16 +570,22 @@ public Object execute(VirtualFrame frame) { private static final class TypeCheckExpressionNode extends ExpressionNode { @Child private ExpressionNode original; @Child private ReadArgumentCheckNode check; + private final boolean acceptWholeMultiValue; - TypeCheckExpressionNode(ExpressionNode original, ReadArgumentCheckNode check) { + private TypeCheckExpressionNode( + ExpressionNode original, ReadArgumentCheckNode check, boolean acceptWholeMultiValue) { this.check = check; this.original = original; + this.acceptWholeMultiValue = acceptWholeMultiValue; } @Override public Object executeGeneric(VirtualFrame frame) { var value = original.executeGeneric(frame); var result = check.handleCheckOrConversion(frame, value); + if (acceptWholeMultiValue && value instanceof EnsoMultiValue) { + return value; + } return result; } diff --git a/engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala b/engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala index 38a8000e4b17..3aca6b7885fc 100644 --- a/engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala +++ b/engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala @@ -1303,19 +1303,25 @@ class IrToTruffle( } runtimeExpression.setTailStatus(getTailStatus(ir)) + def attachTypes(allowWholeMultiValue: Boolean) = { + val types: Option[TypeSignatures.Signature] = + ir.getMetadata(TypeSignatures) + types.foreach { tpe => + val checkNode = + extractAscribedType(tpe.comment.orNull, tpe.signature); + if (checkNode != null) { + runtimeExpression = ReadArgumentCheckNode.wrapWithTypeCheck( + runtimeExpression, + checkNode, + allowWholeMultiValue + ) + } + } + } ir match { case _: Expression.Binding => - case _ => - val types: Option[TypeSignatures.Signature] = - ir.getMetadata(TypeSignatures) - types.foreach { tpe => - val checkNode = - extractAscribedType(tpe.comment.orNull, tpe.signature); - if (checkNode != null) { - runtimeExpression = - ReadArgumentCheckNode.wrap(runtimeExpression, checkNode) - } - } + case _: Expression.Block => attachTypes(true) + case _ => attachTypes(false) } runtimeExpression } @@ -2204,7 +2210,8 @@ class IrToTruffle( if (typeCheck == null) { (argExpressions.toArray, bodyExpr) } else { - val bodyWithCheck = ReadArgumentCheckNode.wrap(bodyExpr, typeCheck) + val bodyWithCheck = + ReadArgumentCheckNode.wrapWithTypeCheck(bodyExpr, typeCheck, true) (argExpressions.toArray, bodyWithCheck) } } diff --git a/test/Base_Tests/src/Semantic/Conversion_Spec.enso b/test/Base_Tests/src/Semantic/Conversion_Spec.enso index c0d7a560ec9e..14269b88f33f 100644 --- a/test/Base_Tests/src/Semantic/Conversion_Spec.enso +++ b/test/Base_Tests/src/Semantic/Conversion_Spec.enso @@ -98,6 +98,20 @@ type Fool Fool.from (that : Any) = Fool.Value that +type Mixin + Value mix:Text + + text_and_mixin t:Text = + mixin = t:(Text & Mixin) + mixin + + text_and_mixin_with_check t:Text -> Mixin = + v = Mixin.text_and_mixin t + v + +Mixin.from (that:Text) = Mixin.Value that +Mixin.from (that:Any) = Mixin.text_and_mixin that.to_text + type Blob Text c:Text Binary b:File @@ -450,6 +464,38 @@ add_specs suite_builder = do_duration now + group_builder.specify "Returning Text & Mixin from method" <| + m = Mixin.text_and_mixin "hi there" + + m:Text . should_equal "hi there" + (m:Mixin).mix . should_equal "hi there" + + Meta.type_of (m:Text) . should_equal Text + Meta.type_of (m:Mixin) . should_equal Mixin + Meta.type_of m . should_equal Text + + group_builder.specify "Returning Text & Mixin from method with check" <| + m = Mixin.text_and_mixin_with_check "hi there" + + m:Text . should_equal "hi there" + (m:Mixin).mix . should_equal "hi there" + + Meta.type_of (m:Text) . should_equal Text + Meta.type_of (m:Mixin) . should_equal Mixin + Meta.type_of m . should_equal Text + + group_builder.specify "Returning Text & Mixin from Conversion method" <| + v = 42 + m = v:Mixin + + m:Text . should_equal "42" + (m:Mixin).mix . should_equal "42" + + Meta.type_of (m:Text) . should_equal Text + Meta.type_of (m:Mixin) . should_equal Mixin + Meta.type_of m . should_equal Text + + suite_builder.group "Autoscoped Constructors" group_builder-> group_builder.specify "Foo.Value as autoscoped" <| From 8d7ab15bb6af929f47ce0201a06b946d913dfc95 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Fri, 23 Aug 2024 18:21:37 +0200 Subject: [PATCH 2/3] Example that mixes in Snowflake_Connection and Connection into a single object --- test/Base_Tests/src/Main.enso | 2 + .../Semantic/Conversion_Database_Spec.enso | 82 +++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 test/Base_Tests/src/Semantic/Conversion_Database_Spec.enso diff --git a/test/Base_Tests/src/Main.enso b/test/Base_Tests/src/Main.enso index a12834f9fdb6..0a53f1f68e67 100644 --- a/test/Base_Tests/src/Main.enso +++ b/test/Base_Tests/src/Main.enso @@ -5,6 +5,7 @@ from Standard.Test import all import project.Semantic.Any_Spec import project.Semantic.Case_Spec import project.Semantic.Conversion_Spec +import project.Semantic.Conversion_Database_Spec import project.Semantic.Default_Args_Spec import project.Semantic.Error_Spec import project.Semantic.Import_Loop.Spec as Import_Loop_Spec @@ -103,6 +104,7 @@ main filter=Nothing = Function_Spec.add_specs suite_builder Case_Spec.add_specs suite_builder Conversion_Spec.add_specs suite_builder + Conversion_Database_Spec.add_specs suite_builder Default_Args_Spec.add_specs suite_builder Error_Spec.add_specs suite_builder Environment_Spec.add_specs suite_builder diff --git a/test/Base_Tests/src/Semantic/Conversion_Database_Spec.enso b/test/Base_Tests/src/Semantic/Conversion_Database_Spec.enso new file mode 100644 index 000000000000..319980099872 --- /dev/null +++ b/test/Base_Tests/src/Semantic/Conversion_Database_Spec.enso @@ -0,0 +1,82 @@ +from Standard.Base import all +from Standard.Test import all + + +#class Connection a where +# get_tables :: a -> [String] + +type Connection + private By value dict + + get_tables self = self.dict.get_tables self.value + + new value dict = + Connection.By value dict + +type Snowflake_Connection + private Details details:Snowflake_Connection_Details + + get_warehouses self -> Vector Text = ["SNOWFLAKE_WAREHOUSE"] + +# database_connect :: Connection c => Connection_Details d c => d -> c +database_connect details:Any -> Connection = + Connection.from details + +# data Snowflake_Connection_Details = Snowflake_Connection_Details String String String -- username, password etc. +type Snowflake_Connection_Details + Value username:Text password:Text etc:Text + +two_roles_of_snowflake_connection -> Pair Text = + # let snowflake_connection = database_connect (Snowflake_Connection_Details "foo" "bar" "baz") + details = Snowflake_Connection_Details.Value "foo" "bar" "baz" + snowflake_connection = database_connect details + + # get_tables is methods from Connection type + # it is there because `database_connect` returns Connection + r1 = snowflake_connection.get_tables + # get_warehouses is a method from Snowflake_Connection type + # it has been "mixed in" in Connection.from (that:Snowflake_Connection) + r2 = snowflake_connection.get_warehouses + + Pair.new r1.to_text r2.to_text + +# data Snowflake_Connection = Snowflake_Connection_Impl -- here we'd have internals of the connection +#instance Connection Snowflake_Connection where +# get_tables c = ["some Snowflake table"] -- we'd call JDBC here ofc. + +type Snowflake_Connection_Impl + get_tables self c:Snowflake_Connection = + ["some Snowflake table for "+c.to_text] + +Connection.from (that:Snowflake_Connection_Details) = + c = Snowflake_Connection.Details that + mixin = c:(Snowflake_Connection & Connection) + mixin + +Connection.from (that:Snowflake_Connection) = + Connection.new that Snowflake_Connection_Impl + +# +# running +# + +main = + pair = two_roles_of_snowflake_connection + IO.println pair.first + IO.println pair.second + +# +# testing +# + +add_specs suite_builder = + suite_builder.group "Conversion Database Example" group_builder-> + group_builder.specify "snowflake_connection plays two roles" <| + pair = two_roles_of_snowflake_connection + + pair.first . should_contain "some Snowflake table for" + pair.first . should_contain "Snowflake_Connection_Details" + pair.first . should_contain "foo" + pair.first . should_contain "bar" + pair.first . should_contain "baz" + pair.second . should_equal "[SNOWFLAKE_WAREHOUSE]" From 0034567b09f4a270252c36180c8460501429556f Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Sat, 24 Aug 2024 11:22:58 +0200 Subject: [PATCH 3/3] Note in changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e4597bbf042..aa3e9ebe2bb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,8 +39,10 @@ #### Enso Language & Runtime - [Print out warnings associated with local variables][10842] +- [Method return type checks avoid conversion when the type fits][10882] [10842]: https://github.com/enso-org/enso/pull/10842 +[10882]: https://github.com/enso-org/enso/pull/10882 # Enso 2024.3