Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x : Text is valid syntax, but doesn't check runtime type of the expression #6848

Closed
2 tasks done
JaroslavTulach opened this issue May 25, 2023 · 4 comments · Fixed by #7796
Closed
2 tasks done

x : Text is valid syntax, but doesn't check runtime type of the expression #6848

JaroslavTulach opened this issue May 25, 2023 · 4 comments · Fixed by #7796
Assignees

Comments

@JaroslavTulach
Copy link
Member

JaroslavTulach commented May 25, 2023

Following code shall compile, but then yield a runtime error:

from Standard.Base import Text

main =
    x = 10
    y = x : Text
    y

however it doesn't even compile and produces strange syntax error:

t.enso[4:5-4:5]: Unused variable x.
Compiler encountered errors:
t.enso[5:5-5:10]: Syntax is not supported yet: qualifiedNameSegment.
t.enso[6:5-6:5]: The name `y` could not be found.
Aborting due to 2 errors and 1 warnings.

Related to #6682 and @radeusgd comment #6682 (comment) to allow us to verify elements of types like Vector Number, etc.

Tasks

Preview Give feedback
@JaroslavTulach
Copy link
Member Author

Surprisingly following syntax parser fine (but doesn't do any type checking yet):

            ok = 0 : Integer
            bad = "fail" : Integer

@JaroslavTulach
Copy link
Member Author

Built on top of #7769 I seem to be able to check the above and fail on the bad line:

diff --git engine/runtime/src/main/java/org/enso/interpreter/node/callable/argument/ReadArgumentCheckNode.java engine/runtime/src/main/java/org/enso/interpreter/node/callable/argument/ReadArgumentCheckNode.java
index 6cf8603368..3cc8056a16 100644
--- engine/runtime/src/main/java/org/enso/interpreter/node/callable/argument/ReadArgumentCheckNode.java
+++ engine/runtime/src/main/java/org/enso/interpreter/node/callable/argument/ReadArgumentCheckNode.java
@@ -9,6 +9,7 @@ import org.enso.compiler.core.ir.Name;
 import org.enso.interpreter.EnsoLanguage;
 import org.enso.interpreter.node.BaseNode.TailStatus;
 import org.enso.interpreter.node.EnsoRootNode;
+import org.enso.interpreter.node.ExpressionNode;
 import org.enso.interpreter.node.callable.ApplicationNode;
 import org.enso.interpreter.node.callable.InvokeCallableNode.DefaultsExecutionMode;
 import org.enso.interpreter.node.callable.thunk.ThunkExecutorNode;
@@ -55,6 +56,26 @@ public abstract class ReadArgumentCheckNode extends Node {
     this.name = name;
   }
 
+  public static ExpressionNode wrap(ExpressionNode original, ReadArgumentCheckNode check) {
+    class TypeCheckNode extends ExpressionNode {
+      @Child
+      private ExpressionNode orig = original;
+
+      @Override
+      public Object executeGeneric(VirtualFrame frame) {
+        var value = orig.executeGeneric(frame);
+        var result = check.handleCheckOrConversion(frame, value);
+        return result;
+      }
+
+      @Override
+      public boolean isInstrumentable() {
+        return false;
+      }
+    }
+    return new TypeCheckNode();
+  }
+
   /** Executes check or conversion of the value.abstract
    * @param frame frame requesting the conversion
    * @param value the value to convert
@@ -77,7 +98,8 @@ public abstract class ReadArgumentCheckNode extends Node {
       expectedTypeMessage = expectedTypeMessage();
     }
     var ctx = EnsoContext.get(this);
-    var err = ctx.getBuiltins().error().makeTypeError(expectedTypeMessage, v, name);
+    var msg = name == null ? "expression" : name;
+    var err = ctx.getBuiltins().error().makeTypeError(expectedTypeMessage, v, msg);
     throw new PanicException(err, this);
   }
 
@@ -102,7 +124,7 @@ public abstract class ReadArgumentCheckNode extends Node {
   }
 
   public static ReadArgumentCheckNode build(Name argumentName, Type expectedType) {
-    var n = argumentName.name();
+    var n = argumentName == null ? null : argumentName.name();
     return ReadArgumentCheckNodeFactory.TypeCheckNodeGen.create(n, expectedType);
   }
 
diff --git engine/runtime/src/main/scala/org/enso/compiler/codegen/IrToTruffle.scala engine/runtime/src/main/scala/org/enso/compiler/codegen/IrToTruffle.scala
index 1cfb1be38a..41d5e89f79 100644
--- engine/runtime/src/main/scala/org/enso/compiler/codegen/IrToTruffle.scala
+++ engine/runtime/src/main/scala/org/enso/compiler/codegen/IrToTruffle.scala
@@ -680,43 +680,46 @@ class IrToTruffle(
   // === Utility Functions ====================================================
   // ==========================================================================
 
-  private def checkRuntimeTypes(
-    arg: DefinitionArgument
-  ): ReadArgumentCheckNode = {
-    def extractAscribedType(t: Expression): ReadArgumentCheckNode = t match {
-      case u: `type`.Set.Union =>
-        ReadArgumentCheckNode.oneOf(
-          arg.name,
-          u.operands.map(extractAscribedType).asJava
-        )
-      case i: `type`.Set.Intersection =>
-        ReadArgumentCheckNode.allOf(
-          arg.name,
-          extractAscribedType(i.left),
-          extractAscribedType(i.right)
-        )
-      case p: Application.Prefix => extractAscribedType(p.function)
-      case _: Tpe.Function =>
-        ReadArgumentCheckNode.build(
-          arg.name,
-          context.getTopScope().getBuiltins().function()
-        )
-      case t => {
-        t.getMetadata(TypeNames) match {
-          case Some(
-                BindingsMap
-                  .Resolution(BindingsMap.ResolvedType(mod, tpe))
-              ) =>
-            ReadArgumentCheckNode.build(
-              arg.name,
-              mod.unsafeAsModule().getScope.getTypes.get(tpe.name)
-            )
-          case _ => null
-        }
+  private def extractAscribedType(
+    name: Name,
+    t: Expression
+  ): ReadArgumentCheckNode = t match {
+    case u: `type`.Set.Union =>
+      ReadArgumentCheckNode.oneOf(
+        name,
+        u.operands.map(extractAscribedType(name, _)).asJava
+      )
+    case i: `type`.Set.Intersection =>
+      ReadArgumentCheckNode.allOf(
+        name,
+        extractAscribedType(name, i.left),
+        extractAscribedType(name, i.right)
+      )
+    case p: Application.Prefix => extractAscribedType(name, p.function)
+    case _: Tpe.Function =>
+      ReadArgumentCheckNode.build(
+        name,
+        context.getTopScope().getBuiltins().function()
+      )
+    case t => {
+      t.getMetadata(TypeNames) match {
+        case Some(
+              BindingsMap
+                .Resolution(BindingsMap.ResolvedType(mod, tpe))
+            ) =>
+          ReadArgumentCheckNode.build(
+            name,
+            mod.unsafeAsModule().getScope.getTypes.get(tpe.name)
+          )
+        case _ => null
       }
     }
+  }
 
-    arg.ascribedType.map(extractAscribedType).getOrElse(null)
+  private def checkRuntimeTypes(
+    arg: DefinitionArgument
+  ): ReadArgumentCheckNode = {
+    arg.ascribedType.map(extractAscribedType(arg.name, _)).getOrElse(null)
   }
 
   /** Checks if the expression has a @Builtin_Method annotation
@@ -982,7 +985,7 @@ class IrToTruffle(
       binding: Boolean,
       subjectToInstrumentation: Boolean
     ): RuntimeExpression = {
-      val runtimeExpression = ir match {
+      var runtimeExpression = ir match {
         case block: Expression.Block => processBlock(block)
         case literal: Literal        => processLiteral(literal)
         case app: Application =>
@@ -1007,8 +1010,18 @@ class IrToTruffle(
             s"Foreign expressions not yet implemented: $ir."
           )
       }
-
       runtimeExpression.setTailStatus(getTailStatus(ir))
+
+      val types = ir.getMetadata(TypeSignatures)
+      if (types.isDefined) {
+        System.out.println("types: " + types.get)
+        val checkNode = extractAscribedType(null, types.get.signature);
+        if (checkNode != null) {
+          runtimeExpression =
+            ReadArgumentCheckNode.wrap(runtimeExpression, checkNode)
+        }
+      }
+
       runtimeExpression
     }
 

@JaroslavTulach JaroslavTulach moved this from 📤 Backlog to 🔧 Implementation in Issues Board Sep 8, 2023
@JaroslavTulach JaroslavTulach changed the title Cannot check runtime type of local variable x : Text is valid syntax, but doesn't check runtime type of the expression Sep 8, 2023
@JaroslavTulach JaroslavTulach moved this from 🔧 Implementation to 👁️ Code review in Issues Board Sep 13, 2023
@enso-bot
Copy link

enso-bot bot commented Sep 14, 2023

Jaroslav Tulach reports a new STANDUP for yesterday (2023-09-13):

Progress: - many fixes for multi value: #7796 (comment)

  • test to verify integer arithmetic: 4614be2
  • meeting, talking private modules, talking parser It should be finished by 2023-09-18.

Next Day: Finish x:Text ascription

@github-project-automation github-project-automation bot moved this from 👁️ Code review to 🟢 Accepted in Issues Board Sep 14, 2023
@enso-bot
Copy link

enso-bot bot commented Sep 15, 2023

Jaroslav Tulach reports a new STANDUP for yesterday (2023-09-14):

Progress: - Merged Runtime checking of ascribed expressions: #7796

Next Day: Continue with IdExecutionInstrument rewrite

Discord
Discord is the easiest way to communicate over voice, video, and text. Chat, hang out, and stay close with your friends and communities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants