Type ascriptions not accepted in tail position #7841

radeusgd · 2023-09-19T12:02:33Z

The PR #7796 gave us the ability to check / trigger conversions of types by a type ascription (a : Xyz).

Apparently, this is not accepted by the parser in tail position.

from Standard.Base import all

f1 v = v.each x->
    y = x : Text
    y

f2 v = v.each x->
    x : Text

main =
    IO.println (f1 ["foo"])
    IO.println (f2 ["foo"])

yields

C:\NBO\repr\type-check-parse.enso:7:15: warning: Unused function argument x.
    7 | f2 v = v.each x->
      |               ^
C:\NBO\repr\type-check-parse.enso:8:5: error: Unexpected type signature.
    8 |     x : Text
      |     ^~~~~~~~
Aborting due to 1 errors and 1 warnings.
Execution finished with an error: Compilation aborted due to errors.

So f1 is happily accepted but f2 is not.

The repro itself shows how to work-around this - we just need to add an intermediate variable. Thus it's rather low priority, but still - it seems there is no reason to disallow a check in there.

The text was updated successfully, but these errors were encountered:

kazcw · 2023-09-19T14:22:45Z

Currently the parser allows type signatures anywhere in a block. We could treat the last line of a block specially, since a type ascription is clearly more likely to be the intent in that case. In the meantime another option for a workaround would be (x : Text), which is a type ascription because type signatures are not allowed inside expressions.

radeusgd · 2023-09-19T14:59:55Z

In the meantime another option for a workaround would be (x : Text), which is a type ascription because type signatures are not allowed inside expressions.

Actually, I checked (x : Text) and it does not work either:

from Standard.Base import all

f1 v = v.each x->
    y = x : Text
    y

f2 v = v.each x->
    (x : Text)

main =
    IO.println (f1 ["foo"])
    IO.println (f2 ["foo"])

still yields

C:\NBO\repr\type-check-parse.enso:7:15: warning: Unused function argument x.
    7 | f2 v = v.each x->
      |               ^
C:\NBO\repr\type-check-parse.enso:8:6: error: Unexpected type signature.
    8 |     (x : Text)
      |      ^~~~~~~~
Aborting due to 1 errors and 1 warnings.
Execution finished with an error: Compilation aborted due to errors.

kazcw · 2023-09-19T15:11:25Z

Huh. That is actually a separate bug, in TreeToIr; in the AST, that's a TypeAnnotated so it should not be treated as a type signature:

$ echo -n 'foo = x->    (y : Text)' | target/rust/release/enso-parser-debug
(BodyBlock #((Assignment (Ident foo) "=" (OprApp (Ident x) (Ok "->") (Group (TypeAnnotated (Ident y) ":" (Ident Text)))))))

JaroslavTulach · 2024-10-07T14:55:52Z

Investigating. Following diff seems to enable the proper behavior:

diff --git engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala
index a524f70ebf..3270abef23 100644
--- engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala
+++ engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/FramePointerAnalysis.scala
@@ -12,7 +12,8 @@ import org.enso.compiler.core.ir.{
   Function,
   Module,
   Name,
-  Pattern
+  Pattern,
+  Type
 }
 import org.enso.compiler.core.ir.module.scope.Definition
 import org.enso.compiler.core.ir.module.scope.definition.Method
@@ -163,6 +164,9 @@ case object FramePointerAnalysis extends IRPass {
         caseExpr.branches.foreach { branch =>
           processCaseBranch(branch)
         }
+      case asc: Type.Ascription =>
+        processExpression(asc.typed, graph)
+        processExpression(asc.signature, graph)
       case _ => ()
     }
     if (updateSymbols) {
diff --git engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/TypeSignatures.scala engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/TypeSignatures.scala
index 10f52ce9d5..fc01693c85 100644
--- engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/TypeSignatures.scala
+++ engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/resolve/TypeSignatures.scala
@@ -315,7 +315,13 @@ case object TypeSignatures extends IRPass {
         lastSignature = None
         res
       case a => Some(resolveExpression(a))
-    } ::: lastSignature.map(errors.Unexpected.TypeSignature(_)).toList
+    } ::: lastSignature.map({
+      case asc @ Type.Ascription(_:Name, sig, comment, _, _) =>
+        asc.updateMetadata(
+          new MetadataPair(this, Signature(sig, comment))
+        )
+      case any => errors.Unexpected.TypeSignature(any)
+    }).toList
 
     block.copy(
       expressions = newExpressions.init,
diff --git engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala
index c7b47140b4..5c7a306bb6 100644
--- engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala
+++ engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala
@@ -878,7 +878,8 @@ class IrToTruffle(
       // Type contexts aren't currently really used. But we should still check the base type.
       extractAscribedType(comment, typeInContext.typed)
     case t => {
-      t.getMetadata(TypeNames) match {
+      val res = t.getMetadata(TypeNames)
+      res match {
         case Some(
               BindingsMap
                 .Resolution(binding @ BindingsMap.ResolvedType(_, _))
@@ -1287,7 +1288,24 @@ class IrToTruffle(
         case binding: Expression.Binding => processBinding(binding)
         case caseExpr: Case =>
           processCase(caseExpr, subjectToInstrumentation)
-        case typ: Tpe => processType(typ)
+        case asc : Tpe.Ascription =>
+            val checkNode =
+              extractAscribedType(asc.comment.orNull, asc.signature)
+            if (checkNode != null) {
+              val body = run(asc.typed, binding, subjectToInstrumentation)
+              ReadArgumentCheckNode.wrap(body, checkNode)
+            } else {
+              processType(asc)
+            }
+        case typ: Tpe =>
+            val checkNode =
+              extractAscribedType("ascribed", typ)
+            if (checkNode != null) {
+              //val body = run(typ.typed(), binding, subjectToInstrumentation)
+              ReadArgumentCheckNode.wrap(null, checkNode)
+            } else {
+              processType(typ)
+            }
         case _: Empty =>
           processEmpty()
         case _: Comment =>

kazcw · 2024-10-07T17:46:32Z

In the Tree we have a distinction between two different expression : Type syntaxes:

TypeSignature, which declares the type of expression and is used in statement context (e.g. in the older out-of-band-types method definition syntax)
TypeAnnotated, which evaluates to expression and is used in a value context

I think this is an important syntactic distinction because it corresponds to a semantic distinction in whether the form causes expression to be evaluated. So I'm wondering, why not reflect this distinction in the IR representation?

JaroslavTulach · 2024-10-08T03:43:48Z

* `TypeSignature`, which _declares_ the type of `expression` and
* `TypeAnnotated`, which _evaluates to_ `expression` and is used in a value context
I think this is an important syntactic distinction because it corresponds to a semantic distinction ... So I'm wondering, why not reflect this distinction in the IR representation?

I was surprised the x: Text is processed by TypesSignatures's lastSignature, because it is not signature, but an expression. The diff addresses that directly in TypesSignatures, but I'll try to avoid this confusion in TreeToIr.

kazcw · 2024-10-08T14:38:50Z

* `TypeSignature`, which _declares_ the type of `expression` and
* `TypeAnnotated`, which _evaluates to_ `expression` and is used in a value context
I think this is an important syntactic distinction because it corresponds to a semantic distinction ... So I'm wondering, why not reflect this distinction in the IR representation?
I was surprised the x: Text is processed by TypesSignatures's lastSignature, because it is not signature, but an expression.

Yes, the problem is that IR doesn't distinguish type declaration statements (Tree.TypeSignature) from type-annotated expressions (Tree.TypeAnnotated)--in TreeToIr they are currently both translated to the same type ir.Type.Ascription.

enso-bot · 2024-10-09T04:01:20Z

Jaroslav Tulach reports a new STANDUP for yesterday (2024-10-08):

Progress: .

mini pass with mapExpressions: Migrate some passes to Mini passes #11191 (comment)
design serde: Design serde for Enso objects #11221
postfix operator: Operator-block syntax pitfall #11203 (comment)
fixing unit tests: 6388881
review scala.Option: Eliminate scala.Some allocations #11259 (review)
review exec env: Run node in a different execution environment #11173 (review)
planning meeting It should be finished by 2024-10-11.

JaroslavTulach · 2024-10-09T15:28:40Z

There are signatures like this is_wanted : Node -> Boolean and then the signatures like

                plusChecked a b =
                    x = a + b
                    x:Integer

so far I have problems to differentiate them. This second case is still TypeSignature[" ", "x:Integer", [], Ident["", "x", [], Ident["", "x", 1, 1, false, 0, false, false]], TypeAnnotationOperator["", ":", 1, 1], Ident["", "Integer", [], Ident["", "Integer", 7, 7, false, 0, true, false]]]. I cannot differentiate from the case:

               plusChecked a b =
                    x:Integer
                    x = a + b
                    x

in this example, it is again TypeSignature[" ", "x:Integer", [], Ident["", "x", [], Ident["", "x", 1, 1, false, 0, false, false]], TypeAnnotationOperator["", ":", 1, 1], Ident["", "Integer", [], Ident["", "Integer", 7, 7, false, 0, true, false]]].

Those are both currently parsed as declarations, which I think we should change (in the parser):

If you want, please do so. I find way easier to just process the lastSignature as I did in this patch.

kazcw · 2024-10-09T15:32:01Z

Those are both currently parsed as declarations, which I think we should change (in the parser):

Currently the parser allows type signatures anywhere in a block. We could treat the last line of a block specially, since a type ascription is clearly more likely to be the intent in that case. In the meantime another option for a workaround would be (x : Text), which is a type ascription because type signatures are not allowed inside expressions.

enso-bot · 2024-10-10T08:19:44Z

Jaroslav Tulach reports a new STANDUP for yesterday (2024-10-09):

Progress: .

trying ascriptions: Type ascriptions not accepted in tail position #7841 (comment)
bench-processor/test: 2bf4df1
removing Python and JavaScript from NI launcher: https://github.com/orgs/enso-org/discussions/10121
partial deserialization? https://discord.com/channels/@me/948519891961544714/1293552320415793212
fixed Python&JS removal licensing
meetings and standups It should be finished by 2024-10-11.

GitHub
Launching Enso programs instantly · enso-org · Discussion #10121
Enso is dynamically compiled language built on top of JVM - e.g. GraalVM - and its Truffle framework as such programs written in Enso can run pretty fast - once the dynamic compiler does its job. T...

Discord
Discord - Group Chat That’s All Fun & Games
Discord is great for playing games and chilling with friends, or even building a worldwide community. Customize your own space to talk, play, and hang out.

kazcw · 2024-10-10T15:17:03Z

There signatures like this is_wanted : Node -> Boolean and then the signatures like
                plusChecked a b =
                    x = a + b
                    x:Integer
so far I have problems to differentiate them. This second case is still TypeSignature[" ", "x:Integer", [], Ident["", "x", [], Ident["", "x", 1, 1, false, 0, false, false]], TypeAnnotationOperator["", ":", 1, 1], Ident["", "Integer", [], Ident["", "Integer", 7, 7, false, 0, true, false]]]. I cannot differentiate from the case:
               plusChecked a b =
                    x:Integer
                    x = a + b
                    x
in this example, it is again TypeSignature[" ", "x:Integer", [], Ident["", "x", [], Ident["", "x", 1, 1, false, 0, false, false]], TypeAnnotationOperator["", ":", 1, 1], Ident["", "Integer", [], Ident["", "Integer", 7, 7, false, 0, true, false]]].

Those are both currently parsed as declarations, which I think we should change (in the parser):

If you want, please do so. I find way easier to just process the lastSignature as I did in this patch.

With the patch, the code runs correctly, so the patch solves the problem from a compiler perspective. From a broader tooling perspective, if the AST is wrong, and the compiler output is "right", that shows we have two bugs. #11289 fixes the immediate issue; I've opened separate issues for fixing the parser and then the compiler:

enso-bot · 2024-10-11T04:24:46Z

Jaroslav Tulach reports a new STANDUP for yesterday (2024-10-10):

Progress: .

addressing review comments: 147492f
fix by Type ascriptions not accepted in tail position #7841 by Special handling of last signature in a block #11289
tea with Radek It should be finished by 2024-10-11.

github-project-automation bot added this to Issues Board Sep 19, 2023

github-project-automation bot moved this to ❓New in Issues Board Sep 19, 2023

radeusgd added p-lowest Should be completed at some point --bug Type: bug -compiler -parser labels Sep 19, 2023

kazcw self-assigned this Sep 19, 2023

kazcw removed the -compiler label Sep 19, 2023

Akirathan assigned JaroslavTulach Sep 22, 2023

JaroslavTulach moved this from ❓New to ⚙️ Design in Issues Board Oct 7, 2024

enso-bot bot mentioned this issue Oct 8, 2024

AssertionError with payload Error Error.type UnresolvedSymbol<==> when executing Base_Tests #11127

Closed

JaroslavTulach mentioned this issue Oct 10, 2024

Special handling of last signature in a block #11289

Merged

2 tasks

JaroslavTulach moved this from ⚙️ Design to 👁️ Code review in Issues Board Oct 10, 2024

kazcw mentioned this issue Oct 10, 2024

Parse tail of block as expression #11293

Closed

JaroslavTulach closed this as completed in #11289 Oct 11, 2024

github-project-automation bot moved this from 👁️ Code review to 🟢 Accepted in Issues Board Oct 11, 2024

JaroslavTulach mentioned this issue Nov 19, 2024

Unexpected (42 : Text & Integer) signature at the end of a function #11574

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Type ascriptions not accepted in tail position #7841

Type ascriptions not accepted in tail position #7841

radeusgd commented Sep 19, 2023

kazcw commented Sep 19, 2023 •

edited

Loading

radeusgd commented Sep 19, 2023 •

edited

Loading

kazcw commented Sep 19, 2023

JaroslavTulach commented Oct 7, 2024 •

edited

Loading

kazcw commented Oct 7, 2024

JaroslavTulach commented Oct 8, 2024

kazcw commented Oct 8, 2024

enso-bot bot commented Oct 9, 2024

JaroslavTulach commented Oct 9, 2024 •

edited

Loading

kazcw commented Oct 9, 2024

enso-bot bot commented Oct 10, 2024 •

edited by unfurl-links bot

Loading

kazcw commented Oct 10, 2024

enso-bot bot commented Oct 11, 2024

Type ascriptions not accepted in tail position #7841

Type ascriptions not accepted in tail position #7841

Comments

radeusgd commented Sep 19, 2023

kazcw commented Sep 19, 2023 • edited Loading

radeusgd commented Sep 19, 2023 • edited Loading

kazcw commented Sep 19, 2023

JaroslavTulach commented Oct 7, 2024 • edited Loading

kazcw commented Oct 7, 2024

JaroslavTulach commented Oct 8, 2024

kazcw commented Oct 8, 2024

enso-bot bot commented Oct 9, 2024

JaroslavTulach commented Oct 9, 2024 • edited Loading

kazcw commented Oct 9, 2024

enso-bot bot commented Oct 10, 2024 • edited by unfurl-links bot Loading

kazcw commented Oct 10, 2024

enso-bot bot commented Oct 11, 2024

kazcw commented Sep 19, 2023 •

edited

Loading

radeusgd commented Sep 19, 2023 •

edited

Loading

JaroslavTulach commented Oct 7, 2024 •

edited

Loading

JaroslavTulach commented Oct 9, 2024 •

edited

Loading

enso-bot bot commented Oct 10, 2024 •

edited by unfurl-links bot

Loading