Repository: effekt-lang/effekt (scala)

Issue:
Generated documentation doesn't include namespaces
We run the parser and frontend before we generate the documentation. `BoxUnboxInference` flattens the namespaces, which is why we don't have `NamespaceDef`s in the `Tree` once we arrive at the documentation generator. Consequently, [effekt-api](https://effekt-lang.github.io/effekt-api/) also loses information about namespaces.

For documentation generation, we should either skip flattening the namespaces or skip the `BoxUnboxInference` phase completely.

Pull request:
Flattening namespaces is annoying for documentation generation because information gets lost and somehow needs to get reconstructed (for example with scope annotations).

Now we do not flatten namespaces anymore. One case is still missing but interestingly we also get a segfault in LLVM, no idea why.

Additionally, we also deprecate namespace definitions nested in function definitions.

Closes #1037.

Patch:
diff --git a/effekt/shared/src/main/scala/effekt/Parser.scala b/effekt/shared/src/main/scala/effekt/Parser.scala
index 0c625bbe4..bacb772c2 100644
--- a/effekt/shared/src/main/scala/effekt/Parser.scala
+++ b/effekt/shared/src/main/scala/effekt/Parser.scala
@@ -459,7 +459,7 @@ class Parser(positions: Positions, tokens: Seq[Token], source: Source) {
       }
 
   def isDefinition: Boolean = peek.kind match {
-    case `val` | `def` | `type` | `effect` | `namespace` => true
+    case `val` | `def` | `type` | `effect` => true
     case `interface` | `type` | `record` =>
       val kw = peek.kind
       fail(s"Only supported on the toplevel: ${kw.toString} declaration.")
@@ -473,7 +473,6 @@ class Parser(positions: Positions, tokens: Seq[Token], source: Source) {
         case `def`       => defDef(info)
         case `type`      => typeOrAliasDef(info)
         case `effect`    => effectDef(info)
-        case `namespace` => namespaceDef(info)
         case _ => fail("Expected definition")
       }
 
diff --git a/effekt/shared/src/main/scala/effekt/Typer.scala b/effekt/shared/src/main/scala/effekt/Typer.scala
index 9f7fea86c..42d4cf43b 100644
--- a/effekt/shared/src/main/scala/effekt/Typer.scala
+++ b/effekt/shared/src/main/scala/effekt/Typer.scala
@@ -729,6 +729,9 @@ object Typer extends Phase[NameResolved, Typechecked] {
         Context.bind(field, tpe, CaptureSet())
       }
 
+    case d @ source.NamespaceDef(name, defs, doc, span) =>
+      defs.map(precheckDef)
+
     case d: source.TypeDef => wellformed(d.symbol.tpe)
     case d: source.EffectDef => wellformed(d.symbol.effs)
 
@@ -891,6 +894,11 @@ object Typer extends Phase[NameResolved, Typechecked] {
         Result((), Pure)
       }
 
+      case d @ source.NamespaceDef(name, defs, doc, span) =>
+        defs.map(synthDef).reduceLeft {
+          case (Result(_, effs), Result(_, acc)) => Result((), effs ++ acc)
+        }
+
       // all other definitions have already been prechecked
       case d =>
         Result((), Pure)
diff --git a/effekt/shared/src/main/scala/effekt/core/Transformer.scala b/effekt/shared/src/main/scala/effekt/core/Transformer.scala
index e7d31c3d9..d396da848 100644
--- a/effekt/shared/src/main/scala/effekt/core/Transformer.scala
+++ b/effekt/shared/src/main/scala/effekt/core/Transformer.scala
@@ -125,11 +125,12 @@ object Transformer extends Phase[Typechecked, CoreTransformed] {
     case e @ source.ExternInclude(ff, path, contents, _, doc, span) =>
       List(Extern.Include(ff, contents.get))
 
+    case d @ source.NamespaceDef(name, defs, doc, span) =>
+      defs.flatMap(transformToplevel)
+
     // For now we forget about all of the following definitions in core:
     case d: source.Def.Extern => Nil
     case d: source.Def.Alias => Nil
-
-    case d: source.Def.NamespaceDef => Context.panic("Should have been removed by BoxUnboxInference")
   }
 
   /**
@@ -201,11 +202,10 @@ object Transformer extends Phase[Typechecked, CoreTransformed] {
 
       case d: source.Def.Extern => Context.panic("Only allowed on the toplevel")
       case d: source.Def.Declaration => Context.panic("Only allowed on the toplevel")
+      case d: source.Def.NamespaceDef => Context.panic("Only allowed on the toplevel")
 
       // For now we forget about all of the following definitions in core:
       case d: source.Def.Alias => transform(rest)
-
-      case d: source.Def.NamespaceDef => Context.panic("Should have been removed by BoxUnboxInference")
     }
   }
 
diff --git a/effekt/shared/src/main/scala/effekt/typer/BoxUnboxInference.scala b/effekt/shared/src/main/scala/effekt/typer/BoxUnboxInference.scala
index 7d6e4c0bd..b71e8c87d 100644
--- a/effekt/shared/src/main/scala/effekt/typer/BoxUnboxInference.scala
+++ b/effekt/shared/src/main/scala/effekt/typer/BoxUnboxInference.scala
@@ -20,7 +20,7 @@ object BoxUnboxInference extends Phase[NameResolved, NameResolved] {
 
   def rewrite(e: ModuleDecl)(using C: Context): ModuleDecl = visit(e) {
     case ModuleDecl(path, imports, defs, doc, span) =>
-      ModuleDecl(path, imports, defs.flatMap(flattenNamespaces), doc, span)
+      ModuleDecl(path, imports, defs.map(rewrite), doc, span)
   }
 
   /**
@@ -125,11 +125,6 @@ object BoxUnboxInference extends Phase[NameResolved, NameResolved] {
     }
   }
 
-  def flattenNamespaces(t: Def)(using C: Context): List[Def] = t match {
-    case Def.NamespaceDef(name, defs, doc, span) => defs.flatMap(flattenNamespaces)
-    case d => List(rewrite(d))
-  }
-
   def rewrite(t: Def)(using C: Context): Def = visit(t) {
 
     case FunDef(id, tparams, vparams, bparams, ret, body, doc, span) =>
@@ -156,6 +151,9 @@ object BoxUnboxInference extends Phase[NameResolved, NameResolved] {
       }
       DefDef(id, annot, block, doc, span)
 
+    case NamespaceDef(name, defs, doc, span) =>
+      NamespaceDef(name, defs.map(rewrite), doc, span)
+
     case d: InterfaceDef   => d
     case d: DataDef        => d
     case d: RecordDef      => d
@@ -167,13 +165,11 @@ object BoxUnboxInference extends Phase[NameResolved, NameResolved] {
     case d: ExternResource => d
     case d: ExternInterface => d
     case d: ExternInclude  => d
-
-    case d: NamespaceDef => Context.panic("Should have been removed by flattenNamespaces")
   }
 
   def rewrite(t: Stmt)(using C: Context): Stmt = visit(t) {
     case DefStmt(d, rest, span) =>
-      flattenNamespaces(d).foldRight(rewrite(rest)) { case (d, rest) => DefStmt(d, rest, span) }
+      DefStmt(rewrite(d), rewrite(rest), span)
 
     case ExprStmt(e, rest, span) =>
       ExprStmt(rewriteAsExpr(e), rewrite(rest), span)
diff --git a/examples/neg/namer/functions.check b/examples/neg/namer/functions.check
new file mode 100644
index 000000000..668f32bd9
--- /dev/null
+++ b/examples/neg/namer/functions.check
@@ -0,0 +1,3 @@
+[error] examples/neg/namer/functions.effekt:3:3: Expected statements but got keyword namespace
+  namespace nested {
+  ^^^^^^^^^
diff --git a/examples/pos/namer/functions.effekt b/examples/neg/namer/functions.effekt
similarity index 99%
rename from examples/pos/namer/functions.effekt
rename to examples/neg/namer/functions.effekt
index d8450e9ee..c265f4bfe 100644
--- a/examples/pos/namer/functions.effekt
+++ b/examples/neg/namer/functions.effekt
@@ -11,4 +11,4 @@ def somefunction() = {
 def main() = somefunction() match {
   case list::Cons(msg, _) => println(msg)
   case _ => ()
-}
\ No newline at end of file
+}
diff --git a/examples/pos/bidirectional/typeparametric.effekt b/examples/pos/bidirectional/typeparametric.effekt
index f9eaf2da9..bdafcb54d 100644
--- a/examples/pos/bidirectional/typeparametric.effekt
+++ b/examples/pos/bidirectional/typeparametric.effekt
@@ -3,7 +3,7 @@ extern interface Cap[U, V]
 extern pure def cap[U, V](): Cap[U, V] at {} =
   js   "42"
   chez "42"
-  llvm "ret %Pos undef"
+  llvm "ret %Pos zeroinitializer"
 
 interface Foo[S] {
   def op[A]() {f: Cap[S, A]}: Cap[S, A] at {f} / { Exception[S] }
@@ -21,4 +21,4 @@ def main() = {
       resume { {g: Cap[Int, B]} => do raise(42, "") }
     }
   }
-}
\ No newline at end of file
+}
diff --git a/examples/pos/namer/functions.check b/examples/pos/namer/functions.check
deleted file mode 100644
index ce0136250..000000000
--- a/examples/pos/namer/functions.check
+++ /dev/null
@@ -1,1 +0,0 @@
-hello


Test patch:
diff --git a/effekt/jvm/src/test/scala/effekt/ParserTests.scala b/effekt/jvm/src/test/scala/effekt/ParserTests.scala
index d79cf558f..acb0475d2 100644
--- a/effekt/jvm/src/test/scala/effekt/ParserTests.scala
+++ b/effekt/jvm/src/test/scala/effekt/ParserTests.scala
@@ -934,14 +934,14 @@ class ParserTests extends munit.FunSuite {
         |val y = 5
         |""".stripMargin)
 
-    val nested = parseDefinitions(
+    val nested = parseToplevel(
       """namespace list {
         |  val x = 4
         |  val y = 5
         |}
         |""".stripMargin)
 
-    val semi = parseDefinitions(
+    val semi = parseToplevel(
       """namespace list;
         |val x = 4
         |val y = 5
@@ -949,7 +949,7 @@ class ParserTests extends munit.FunSuite {
 
     assertEqualModuloSpans(nested, semi)
 
-    val nested2 = parseDefinitions(
+    val nested2 = parseProgram(
       """namespace list {
         |  namespace internal {
         |
@@ -959,32 +959,15 @@ class ParserTests extends munit.FunSuite {
         |}
         |""".stripMargin)
 
-    val semi2 = parseDefinitions(
-      """namespace list;
-        |namespace internal;
-        |
-        |val x = 4
-        |val y = 5
-        |""".stripMargin)
-
-    val semiInsertion = parseDefinitions(
+    val semi2 = parseProgram(
       """namespace list
         |namespace internal
         |
         |val x = 4
-        |val y = 5
+        |val y = 5;
         |""".stripMargin)
 
-    assertEqualModuloSpans(nested2, semi2)
-    assertEqualModuloSpans(nested2, semiInsertion)
-
-    parseDefinitions(
-      """val x = {
-        |  namespace foo;
-        |  val y = 4;
-        |  foo::y
-        |}
-        |""".stripMargin)
+    (nested2.defs zip semi2.defs).foreach(assertEqualModuloSpans)
   }
 
   test("Definitions") {
@@ -1238,24 +1221,6 @@ class ParserTests extends munit.FunSuite {
     assertEquals(varDef.span, span)
   }
 
-  test("Namespace definition parses with correct span") {
-    val (source, span) =
-      raw"""namespace Foo {
-           |↑
-           |}
-           |↑""".sourceAndSpan
-
-    val definition = parseDefinition(source.content)
-
-    val nsDef = definition match {
-      case nd@NamespaceDef(id, defs, doc, span) => nd
-      case other =>
-        throw new IllegalArgumentException(s"Expected NamespaceDef but got ${other.getClass.getSimpleName}")
-    }
-
-    assertEquals(nsDef.span, span)
-  }
-
   test("Interface definition parses with correct span") {
     val (source, span) =
       raw"""interface IBar {}