Kindless on the Surface, but kind(ed) in the Core, symbolically: Better, recoverable error messages for wrong kinds (#947)

Resolves #667 (see there for context)

Also see #949 which constituted the second part of this.

Types in surface.Tree are now kindless, not distinguished by
Value/BlockType (we keep these just for docs).
This means that the parser for types is now exceedingly liberal, yet
unkind(ed).

The Namer then resolves surface.Tree.Type to symbol.Value/BlockType.
The only changes there are explicit `resolveValue/BlockType` operators.

Author: jiribenes

effekt/jvm/src/test/scala/effekt/RecursiveDescentTests.scala

@@ -260,6 +260,8 @@ class RecursiveDescentTests extends munit.FunSuite {
         |""".stripMargin)
 
     parseStmts("val (left, right) = list; return left")
+
+    parseStmts("val g: () => Unit / Exc at {exc} = fun() { closure() }; ()")
   }
 
   test("Semicolon insertion") {
@@ -325,7 +327,7 @@ class RecursiveDescentTests extends munit.FunSuite {
   test("Value types") {
     assertEquals(
       parseValueType("Int"),
-      ValueTypeRef(IdRef(Nil, "Int"), Nil))
+      TypeRef(IdRef(Nil, "Int"), Nil))
 
     parseValueType("List[Int]")
     parseValueType("list::List[Int]")
@@ -342,8 +344,8 @@ class RecursiveDescentTests extends munit.FunSuite {
 
     assertEquals(
       parseBlockType("(Int, String) => Int"),
-      FunctionType(Nil, List(ValueTypeRef(IdRef(Nil,"Int"), Nil),
-        ValueTypeRef(IdRef(Nil,"String"), Nil)), Nil, ValueTypeRef(IdRef(Nil, "Int"), Nil), Effects(Nil)))
+      FunctionType(Nil, List(TypeRef(IdRef(Nil,"Int"), Nil),
+        TypeRef(IdRef(Nil,"String"), Nil)), Nil, TypeRef(IdRef(Nil, "Int"), Nil), Effects(Nil)))
 
     parseBlockType("(Int, String) => Int / Exc")
     parseBlockType("[T](Int, String) => Int / { Exc, State[T] }")
@@ -356,6 +358,9 @@ class RecursiveDescentTests extends munit.FunSuite {
     parseBlockType("[T] => T") // Not sure we want this...
 
     parseValueType("Exc at { a, b, c }")
+    intercept[Throwable] { parseBlockType("Exc / Eff") }
+    intercept[Throwable] { parseBlockType("Exc / {}") }
+
     parseValueType("() => (Exc at {}) / {} at { a, b, c }")
 
     assertEquals(
@@ -368,6 +373,8 @@ class RecursiveDescentTests extends munit.FunSuite {
     parseValueType("() => (Int at { a, b, c }) at {}")
     parseValueType("(() => Int) at { a, b, c }")
     parseValueType("(() => Int at {}) => Int at { a, b, c }")
+
+    parseValueType("() => Unit / Socket at {io, async, global}")
   }
 
   test("Params") {
@@ -413,7 +420,7 @@ class RecursiveDescentTests extends munit.FunSuite {
   test("Implementations") {
     assertEquals(
       parseImplementation("Foo {}"),
-      Implementation(BlockTypeRef(IdRef(Nil, "Foo"), Nil), Nil))
+      Implementation(TypeRef(IdRef(Nil, "Foo"), Nil), Nil))
 
     parseImplementation("Foo[T] {}")
     parseImplementation("Foo[T] { def bar() = 42 }")
@@ -431,7 +438,7 @@ class RecursiveDescentTests extends munit.FunSuite {
     assertEquals(
       parseImplementation("Foo { 43 }"),
       Implementation(
-        BlockTypeRef(IdRef(Nil, "Foo"), Nil),
+        TypeRef(IdRef(Nil, "Foo"), Nil),
         List(OpClause(IdRef(Nil, "Foo"), Nil, Nil, Nil, None,
           Return(Literal(43, symbols.builtins.TInt)), IdDef("resume")))))
   }
@@ -563,6 +570,11 @@ class RecursiveDescentTests extends munit.FunSuite {
     parseDefinition(
         """def foo[T](x: Int): String / {} = e
           |""".stripMargin)
+
+    parseDefinition(
+        """def op(): Int => Int at {greeter} / Greet = f
+          |""".stripMargin
+    )
   }
 
   test("Toplevel definitions") {

effekt/shared/src/main/scala/effekt/Namer.scala

@@ -702,11 +702,11 @@ class RecursiveDescent(positions: Positions, tokens: Seq[Token], source: Source)
   def implementation(): Implementation =
     nonterminal:
       // Interface[...] {}
-      def emptyImplementation() = backtrack { Implementation(interfaceType(), `{` ~> Nil <~ `}`) }
+      def emptyImplementation() = backtrack { Implementation(blockTypeRef(), `{` ~> Nil <~ `}`) }
 
       // Interface[...] { def <NAME> = ... }
       def interfaceImplementation() = backtrack {
-        val tpe = interfaceType()
+        val tpe = blockTypeRef()
         consume(`{`)
         if !peek(`def`) then fail("Expected at least one operation definition to implement this interface.")
         tpe
@@ -719,7 +719,7 @@ class RecursiveDescent(positions: Positions, tokens: Seq[Token], source: Source)
       def operationImplementation() = idRef() ~ maybeTypeArgs() ~ implicitResume ~ functionArg() match {
         case (id ~ tps ~ k ~ BlockLiteral(_, vps, bps, body)) =>
           val synthesizedId = IdRef(Nil, id.name).withPositionOf(id)
-          val interface = BlockTypeRef(id, tps).withPositionOf(id): BlockTypeRef
+          val interface = TypeRef(id, tps).withPositionOf(id)
           val operation = OpClause(synthesizedId, Nil, vps, bps, None, body, k).withRangeOf(id, body)
           Implementation(interface, List(operation))
       }
@@ -866,9 +866,8 @@ class RecursiveDescent(positions: Positions, tokens: Seq[Token], source: Source)
     case _ => sys.error(s"Internal compiler error: not a valid operator ${op}")
   }
 
-  private def TupleTypeTree(tps: List[ValueType]): ValueType =
-    ValueTypeRef(IdRef(List("effekt"), s"Tuple${tps.size}"), tps)
-    // TODO positions!
+  def TypeTuple(tps: List[Type]): Type =
+    TypeRef(IdRef(List("effekt"), s"Tuple${tps.size}"), tps)
 
   /**
    * This is a compound production for
@@ -1100,78 +1099,109 @@ class RecursiveDescent(positions: Positions, tokens: Seq[Token], source: Source)
         case _ => fail(s"Expected identifier")
       }
 
-
-  /**
-   * Types
-   */
-
-  def valueType(): ValueType = valueType2(true)
-
-  /**
-   * Uses backtracking!
+  /*
+   * Type grammar by precedence:
+   *
+   * Type ::= Id ('[' Type ',' ... ']')?                                           refType
+   *
+   *       | '(' Type ',' ... ')'                                                  atomicType
+   *       | '(' Type ')'
    *
-   * This is not very efficient. To parse a value type, we first parse a block type,
-   * just to see that it either is no blocktype or it is not followed by an `at`
-   * and just "looked" like a block type...
+   *       | Type '=>' Type ('/' SetType)?                                         functionType
+   *       | '(' Type ',' ... ')' ('{' Type '}')* '=>' Type ('/' SetType)?
    *
-   * The parameter [[boxedAllowed]] controls whether on the right a dangling `at`
-   * can occur. This way we prevent parsing `() => S at {} at {}` and force users
-   * to manually parenthesize.
+   *       | Type 'at' Id                                                          boxedType
+   *       | Type 'at' '{' Id ',' ... '}'
+   *       | Type ('/' SetType)?
+   *
+   * SetType ::= Type
+   *        | '{' Type ',' ... '}'
    */
-  private def valueType2(boxedAllowed: Boolean): ValueType = nonterminal {
-    def boxedBlock = backtrack {
-      BoxedType(blockType2(false), `at` ~> captureSet())
-    }
-    if (boxedAllowed) { boxedBlock getOrElse atomicValueType() }
-    else atomicValueType()
+  def effects(): Effects =
+    nonterminal:
+      if peek(`{`) then Effects(many(refType, `{`, `,`, `}`))
+      else Effects(refType())
+
+  def maybeEffects(): Effects = {
+    nonterminal:
+      when(`/`) {
+        effects()
+      } {
+        Effects.Pure
+      }
   }
 
-  def atomicValueType(): ValueType =
+  def refType(): TypeRef =
+    nonterminal:
+      TypeRef(idRef(), maybeTypeArgs())
+
+  // Parse atomic types: Tuples, parenthesized types, type references (highest precedence)
+  private def atomicType(): Type =
     nonterminal:
       peek.kind match {
-        case `(` => some(valueType, `(`, `,`, `)`) match {
-          case tpe :: Nil => tpe
-          case tpes => TupleTypeTree(tpes)
-        }
-        case _ => ValueTypeRef(idRef(), maybeTypeArgs())
+        case `(` =>
+          some(boxedType, `(`, `,`, `)`) match {
+            case tpe :: Nil => tpe
+            case tpes => TypeTuple(tpes)
+          }
+        case _ => refType()
+      }
+
+  // Parse function types (middle precedence)
+  private def functionType(): Type = {
+    // Complex function type: [T]*(Int, String)*{Exc} => Int / {Effect}
+    def functionTypeComplex = backtrack {
+      maybeTypeParams() ~ maybeValueTypes() ~ (maybeBlockTypeParams() <~ `=>`) ~ atomicType() ~ maybeEffects() match {
+        case tparams ~ vparams ~ bparams ~ t ~ effs => FunctionType(tparams, vparams, bparams, t, effs)
       }
+    }
 
+    // Simple function type: Int => Int
+    def functionTypeSimple = backtrack {
+      refType() <~ `=>`
+    } map { tpe =>
+      FunctionType(Nil, List(tpe), Nil, atomicType(), maybeEffects())
+    }
 
-  /**
-   * Uses backtracking!
-   *
-   * TODO improve errors
-   *   i.e. fail("Expected either a function type (e.g., (A) => B / {E} or => B) or an interface type (e.g., State[T]).")
-   */
-  def blockType(): BlockType = blockType2(true)
-  private def blockType2(boxedAllowed: Boolean): BlockType =
+    // Try to parse each function type variant, fall back to basic type if none match
     nonterminal:
+      functionTypeSimple orElse functionTypeComplex getOrElse atomicType()
+  }
 
-      def simpleFunType = backtrack {
-        ValueTypeRef(idRef(), maybeTypeArgs()) <~ `=>`
-      } map { tpe =>
-        FunctionType(Nil, List(tpe), Nil, valueType2(boxedAllowed), maybeEffects())
+  // Parse boxed types and effectfuls (lowest precedence)
+  // "Top-level" parser for a generic type.
+  private def boxedType(): Type = {
+    nonterminal:
+      // Parse the function type first
+      val tpe = functionType()
+
+      // TODO: these should probably be in a loop to parse as many `at`s and `\`s as possible?
+      val boxed = when(`at`) {
+        BoxedType(tpe, captureSet())
+      } {
+        tpe
       }
 
-      def funType = backtrack {
-        maybeTypeParams() ~ maybeValueTypes() ~ (maybeBlockTypeParams() <~ `=>`) ~ valueType2(boxedAllowed) ~ maybeEffects() match {
-          case tparams ~ vparams ~ bparams ~ t ~ effs => FunctionType(tparams, vparams, bparams, t, effs)
-        }
+      if (peek(`/`)) {
+        Effectful(boxed, maybeEffects())
+      } else {
+        boxed
       }
-      def parenthesized = backtrack { parens { blockType() } }
+  }
 
-      def interface() =
-        val res = interfaceType()
-        if peek(`/`) then
-          fail("Effects not allowed here. Maybe you mean to use a function type `() => T / E`?")
-        else res
+  // NOTE: ValueType, BlockType are just aliases for Type.
+  def blockType(): BlockType = boxedType()
+  def valueType(): ValueType = boxedType()
 
-      simpleFunType orElse funType orElse parenthesized getOrElse interface()
+  // Completely specialized for TypeRef: we only parse `refType` here, we don't go through the whole hierarchy.
+  // This results in slightly worse errors, but massively simplifies the design.
+  inline def blockTypeRef(): TypeRef = refType()
 
-  def interfaceType(): BlockTypeRef =
-    nonterminal:
-      BlockTypeRef(idRef(), maybeTypeArgs()): BlockTypeRef
-    // TODO error "Expected an interface type"
+  // Somewhat specialized: we parse a normal type, if it's not a ${tpe} / ${effs},
+  // then pretend the effect set is empty. This seems to work out fine :)
+  def effectful(): Effectful = boxedType() match
+    case eff: Effectful => eff
+    case tpe => Effectful(tpe, Effects.Pure)
 
   def maybeTypeParams(): List[Id] =
     nonterminal:
@@ -1181,15 +1211,15 @@ class RecursiveDescent(positions: Positions, tokens: Seq[Token], source: Source)
     nonterminal:
       some(idDef, `[`, `,`, `]`)
 
-  def maybeBlockTypeParams(): List[(Option[IdDef], BlockType)] =
+  def maybeBlockTypeParams(): List[(Option[IdDef], Type)] =
     nonterminal:
       if peek(`{`) then blockTypeParams() else Nil
 
-  def blockTypeParams(): List[(Option[IdDef], BlockType)] =
+  def blockTypeParams(): List[(Option[IdDef], Type)] =
     nonterminal:
       someWhile(blockTypeParam(), `{`)
 
-  def blockTypeParam(): (Option[IdDef], BlockType) =
+  def blockTypeParam(): (Option[IdDef], Type) =
     nonterminal:
       braces { (backtrack { idDef() <~ `:` }, blockType()) }
 
@@ -1259,34 +1289,18 @@ class RecursiveDescent(positions: Positions, tokens: Seq[Token], source: Source)
     nonterminal:
       BlockParam(idDef(), when(`:`)(Some(blockType()))(None))
 
-
-  def maybeValueTypes(): List[ValueType] =
+  def maybeValueTypes(): List[Type] =
     nonterminal:
       if peek(`(`) then valueTypes() else Nil
 
-  def valueTypes(): List[ValueType] =
+  def valueTypes(): List[Type] =
     nonterminal:
       many(valueType, `(`, `,`, `)`)
 
   def captureSet(): CaptureSet =
     nonterminal:
       CaptureSet(many(idRef, `{`, `,` , `}`))
 
-  def effectful(): Effectful =
-    nonterminal:
-      Effectful(valueType(), maybeEffects())
-
-  def maybeEffects(): Effects =
-    nonterminal:
-      when(`/`) { effects() } { Effects.Pure }
-
-  // TODO error "Expected an effect set"
-  def effects(): Effects =
-    nonterminal:
-      if peek(`{`) then Effects(many(interfaceType, `{`, `,`, `}`))
-      else Effects(interfaceType())
-
-
   // Generic utility functions
   // -------------------------
   // ... for writing parsers.

effekt/shared/src/main/scala/effekt/RecursiveDescent.scala

@@ -6,7 +6,7 @@ package typer
  */
 import effekt.context.{Annotation, Annotations, Context, ContextOps}
 import effekt.context.assertions.*
-import effekt.source.{AnyPattern, Def, Effectful, IgnorePattern, MatchGuard, MatchPattern, ModuleDecl, OpClause, Stmt, TagPattern, Term, Tree, resolve, symbol}
+import effekt.source.{AnyPattern, Def, Effectful, IgnorePattern, MatchGuard, MatchPattern, ModuleDecl, OpClause, Stmt, TagPattern, Term, Tree, resolve, resolveBlockRef, resolveValueType, resolveBlockType, symbol}
 import effekt.source.Term.BlockLiteral
 import effekt.symbols.*
 import effekt.symbols.builtins.*
@@ -150,7 +150,7 @@ object Typer extends Phase[NameResolved, Typechecked] {
 
       case c @ source.Do(effect, op, targs, vargs, bargs) =>
         // (1) first check the call
-        val Result(tpe, effs) = checkOverloadedFunctionCall(c, op, targs map { _.resolve }, vargs, bargs, expected)
+        val Result(tpe, effs) = checkOverloadedFunctionCall(c, op, targs map { _.resolveValueType }, vargs, bargs, expected)
         // (2) now we need to find a capability as the receiver of this effect operation
         // (2a) compute substitution for inferred type arguments
         val typeArgs = Context.annotatedTypeArgs(c)
@@ -168,21 +168,21 @@ object Typer extends Phase[NameResolved, Typechecked] {
         Result(tpe, effs ++ ConcreteEffects(List(effect)))
 
       case c @ source.Call(t: source.IdTarget, targs, vargs, bargs) =>
-        checkOverloadedFunctionCall(c, t.id, targs map { _.resolve }, vargs, bargs, expected)
+        checkOverloadedFunctionCall(c, t.id, targs map { _.resolveValueType }, vargs, bargs, expected)
 
       case c @ source.Call(source.ExprTarget(e), targs, vargs, bargs) =>
         val Result(tpe, funEffs) = checkExprAsBlock(e, None) match {
           case Result(b: FunctionType, capt) => Result(b, capt)
           case _ => Context.abort("Cannot infer function type for callee.")
         }
 
-        val Result(t, eff) = checkCallTo(c, "function", tpe, targs map { _.resolve }, vargs, bargs, expected)
+        val Result(t, eff) = checkCallTo(c, "function", tpe, targs map { _.resolveValueType }, vargs, bargs, expected)
         Result(t, eff ++ funEffs)
 
       // precondition: PreTyper translates all uniform-function calls to `Call`.
       //   so the method calls here are actually methods calls on blocks as receivers.
       case c @ source.MethodCall(receiver, id, targs, vargs, bargs) =>
-        checkOverloadedMethodCall(c, receiver, id, targs map { _.resolve }, vargs, bargs, expected)
+        checkOverloadedMethodCall(c, receiver, id, targs map { _.resolveValueType }, vargs, bargs, expected)
 
       case tree @ source.Region(name, body) =>
         val reg = tree.symbol
@@ -199,7 +199,7 @@ object Typer extends Phase[NameResolved, Typechecked] {
 
         // (1) extract all handled effects and capabilities
         val providedCapabilities: List[symbols.BlockParam] = handlers map Context.withFocus { h =>
-          val effect: InterfaceType = h.effect.resolve
+          val effect: InterfaceType = h.effect.resolveBlockRef
           val capability = h.capability.map { _.symbol }.getOrElse { Context.freshCapabilityFor(effect) }
           val tpe = capability.tpe.getOrElse { INTERNAL_ERROR("Block type annotation required") }
           Context.bind(capability, tpe, CaptureSet(capability.capture))
@@ -355,7 +355,7 @@ object Typer extends Phase[NameResolved, Typechecked] {
       var handlerEffects: ConcreteEffects = Pure
 
       // Extract interface and type arguments from annotated effect
-      val tpe @ InterfaceType(constructor, targs) = sig.resolve
+      val tpe @ InterfaceType(constructor, targs) = sig.resolveBlockRef
       val interface = constructor.asInterface // can only implement concrete interfaces
 
       // (3) check all operations are covered