Improve whitespace handling in Lexer & Parser, warn on bad whitespace around binops

effekt/jvm/src/test/scala/effekt/LexerTests.scala

@@ -11,6 +11,11 @@ import munit.Location
 class LexerTests extends munit.FunSuite {
 
   def assertTokensEq(prog: String, expected: TokenKind*)(using Location): Unit = {
+    val tokens = Lexer.lex(StringSource(prog, ""))
+    assertEquals(tokens.map { t => t.kind }.filterNot { k => k == Space }, expected.toVector)
+  }
+
+  def assertTokensEqWithWhitespace(prog: String, expected: TokenKind*)(using Location): Unit = {
     val tokens = Lexer.lex(StringSource(prog, ""))
     assertEquals(tokens.map { t => t.kind }, expected.toVector)
   }
@@ -37,6 +42,15 @@ class LexerTests extends munit.FunSuite {
       `return`, `box`, `{`, `(`, `)`, `=>`, `(`, Ident("x"), `,`, Ident("y"), `)`, `}`, Newline,
       EOF
     )
+    assertTokensEqWithWhitespace(
+      prog,
+      `def`, Space, Ident("f"), `[`, Ident("A"), `]`,
+      `(`, Ident("x"), `:`, Space, Ident("A"), `,`, Space, Ident("y"), `:`, Space, Ident("A"), `)`,
+        `:`, Space, `(`, `)`, Space, `=>`, Space, `(`, Ident("A"), `,`, Space, Ident("A"), `)`, Space,
+        `at`, Space, `{`, `}`, Space, `=`, Newline,
+      Space, `return`, Space, `box`, Space, `{`, Space, `(`, `)`, Space, `=>`, Space, `(`, Ident("x"), `,`, Space, Ident("y"), `)`, Space, `}`, Newline,
+      EOF
+    )
   }
 
   test("braces") {
@@ -343,7 +357,7 @@ class LexerTests extends munit.FunSuite {
     assertTokensEq(prog, EOF)
   }
 
-  test("ignore whitespace") {
+  test("extra whitespace") {
     val prog =
       """// interface definition
         |  interface
@@ -366,6 +380,29 @@ class LexerTests extends munit.FunSuite {
       `}`, Newline,
       EOF
     )
+    assertTokensEqWithWhitespace(
+      prog,
+      Comment(" interface definition"), Newline,
+      Space, `interface`, Newline, Newline, Space, Ident("Eff"), `[`, Ident("A"), `,`, Newline, Space, Ident("B"), `]`, Space, `{`,
+      Space, `def`, Space, Ident("operation"), Newline, Space, `[`, Ident("C"), `]`, Newline, Space, `(`, Ident("x"), `:`, Space, Ident("C"), `)`, Space, `:`,
+      Newline, Newline, Space, `(`, Ident("A"), `,`, Space, Ident("B"), `)`, Newline,
+      Newline,
+      Space,
+      `}`, Newline,
+      EOF
+    )
+  }
+
+  test("just whitespace") {
+    val prog = "\n\n\n\r\n   \t val\t\t\t\t   x =\t\r\r\n42\nx"
+    assertTokensEqWithWhitespace(
+      prog,
+      Newline, Newline, Newline, Newline, Space,
+      `val`, Space, Ident("x"), Space, `=`, Space, Newline,
+      Integer(n = 42), Newline,
+      Ident("x"),
+      EOF
+    )
   }
 
   test("resilience") {

effekt/shared/src/main/scala/effekt/Lexer.scala

@@ -288,9 +288,6 @@ class Lexer(source: Source) extends Iterator[Token] {
   override def hasNext: Boolean = !eof
 
   override def next(): Token =
-    if !resumeStringNext || delimiters.isEmpty then
-      skipWhitespace()
-
     tokenStartPosition = position
     val kind = nextToken()
 
@@ -300,17 +297,6 @@ class Lexer(source: Source) extends Iterator[Token] {
     else
       Token(tokenStartPosition.offset, position.offset - 1, kind)
 
-  private def skipWhitespace(): Unit =
-    while !atEndOfInput do
-      currentChar match {
-        case ' ' | '\t' => advance()
-        case '\n' => return // Stop here, let newline be handled as a token
-        case '\r' =>
-          if nextChar == '\n' then return // Stop here for \r\n
-          else advance() // Treat standalone \r as whitespace
-        case _ => return
-      }
-
   private def atEndOfInput: Boolean =
     currentChar == '\u0000'
 
@@ -351,6 +337,15 @@ class Lexer(source: Source) extends Iterator[Token] {
       advance()
     }
 
+  private def advanceSpaces(): TokenKind = {
+    advanceWhile {
+      case ('\r', '\n') => false
+      case ('\n', _)    => false
+      case (curr, _)    => curr.isWhitespace
+    }
+    TokenKind.Space
+  }
+
   private def peekAhead(offset: Int): Char =
     val targetIndex = position.offset + offset
     if targetIndex < source.content.length then
@@ -380,8 +375,10 @@ class Lexer(source: Source) extends Iterator[Token] {
     }
 
     (currentChar, nextChar) match {
+      // Whitespace: first try matching newlines, then whitespace-like
       case ('\n',    _) => advanceWith(TokenKind.Newline)
       case ('\r', '\n') => advance2With(TokenKind.Newline)
+      case (c, _) if c.isWhitespace => advanceSpaces()
 
       // Numbers
       case (c, _) if c.isDigit => number()

effekt/shared/src/main/scala/effekt/Parser.scala

@@ -7,6 +7,7 @@ import effekt.source.*
 import effekt.source.Origin.Synthesized
 import effekt.util.VirtualSource
 import kiama.parsing.{Input, ParseResult}
+import kiama.util.Severities.Severity
 import kiama.util.{Position, Range, Source}
 
 import scala.annotation.{tailrec, targetName}
@@ -52,23 +53,24 @@ object Fail {
   def expectedButGot(expected: String, got: String, position: Int): Fail =
     Fail(s"Expected ${expected} but got ${got}", position)
 }
-case class SoftFail(message: String, positionStart: Int, positionEnd: Int)
+
+case class RecoverableDiagnostic(kind: Severity, message: String, positionStart: Int, positionEnd: Int)
 
 class Parser(tokens: Seq[Token], source: Source) {
 
-  var softFails: ListBuffer[SoftFail] = ListBuffer[SoftFail]()
+  var recoverableDiagnostics: ListBuffer[RecoverableDiagnostic] = ListBuffer[RecoverableDiagnostic]()
 
-  private def report(msg: String, fromPosition: Int, toPosition: Int, source: Source = source)(using C: Context) = {
+  private def report(msg: String, fromPosition: Int, toPosition: Int, severity: Severity = kiama.util.Severities.Error, source: Source = source)(using C: Context) = {
     val from = source.offsetToPosition(tokens(fromPosition).start)
     val to = source.offsetToPosition(tokens(toPosition).end + 1)
     val range = Range(from, to)
-    C.report(effekt.util.messages.ParseError(msg, Some(range)))
+    C.report(effekt.util.messages.ParseError(msg, Some(range), severity))
   }
 
   def parse(input: Input)(using C: Context): Option[ModuleDecl] = {
-    def reportSoftFails()(using C: Context): Unit =
-      softFails.foreach {
-        case SoftFail(msg, from, to) => report(msg, from, to, source = input.source)
+    def reportRecoverableDiagnostics()(using C: Context): Unit =
+      recoverableDiagnostics.foreach { d =>
+        report(d.message, d.positionStart, d.positionEnd, d.kind)
       }
 
     try {
@@ -78,13 +80,16 @@ class Parser(tokens: Seq[Token], source: Source) {
       //val after = System.currentTimeMillis()
       //println(s"${input.source.name}: ${after - before}ms")
 
-      // Report soft fails
-      reportSoftFails()
-      if softFails.isEmpty then res else None
+      // Report recoverable diagnostics
+      reportRecoverableDiagnostics()
+      // Don't fret unless we had a recoverable error
+      if recoverableDiagnostics.forall { d =>
+        d.kind != kiama.util.Severities.Error
+      } then res else None
     } catch {
       case Fail(msg, pos) =>
         // Don't forget soft fails!
-        reportSoftFails()
+        reportRecoverableDiagnostics()
 
         report(msg, pos, pos, source = input.source)
         None
@@ -114,6 +119,7 @@ class Parser(tokens: Seq[Token], source: Source) {
 
   // always points to the latest non-space position
   var position: Int = 0
+  spaces() // HACK: eat spaces before we begin!
 
   def recover(tokenKind: TokenKind, tokenPosition: Int): TokenKind = tokenKind match {
     case TokenKind.Error(err) =>
@@ -141,11 +147,19 @@ class Parser(tokens: Seq[Token], source: Source) {
 
   def peek: Token = tokens(position).failOnErrorToken(position)
 
-  /**
-   * Negative lookahead
-   */
-  def lookbehind(offset: Int): Token =
-    tokens(position - offset)
+  def sawNewlineLast: Boolean = {
+    @tailrec
+    def go(position: Int): Boolean =
+      if position < 0 then fail("Unexpected start of file")
+
+      tokens(position).failOnErrorToken(position) match {
+        case token if isSpace(token.kind) && token.kind != Newline => go(position - 1)
+        case token if token.kind == Newline => true
+        case _ => false
+      }
+
+    go(position - 1)
+  }
 
   /**
    * Peeks n tokens ahead of the current one.
@@ -317,7 +331,7 @@ class Parser(tokens: Seq[Token], source: Source) {
 
     // \n   while
     //      ^
-    case _ => lookbehind(1).kind == Newline
+    case _ => sawNewlineLast
   }
   def semi(): Unit = peek.kind match {
     // \n   ; while
@@ -329,7 +343,7 @@ class Parser(tokens: Seq[Token], source: Source) {
 
     // \n   while
     //      ^
-    case _ if lookbehind(1).kind == Newline => ()
+    case _ if sawNewlineLast => ()
 
     case _ => fail("Expected terminator: `;` or a newline")
   }
@@ -582,7 +596,7 @@ class Parser(tokens: Seq[Token], source: Source) {
     // If we can't parse `effectDef` or `operationDef`, we should try parsing an interface with the wrong keyword
     // and report an error to the user if the malformed interface would be valid.
     def interfaceDefUsingEffect(): Maybe[InterfaceDef] =
-      backtrack(restoreSoftFails = false):
+      backtrack(restoreRecoverable = false):
         softFailWith("Unexpected 'effect', did you mean to declare an interface of multiple operations using the 'interface' keyword?"):
           interfaceDef(info, `effect`)
 
@@ -1001,6 +1015,7 @@ class Parser(tokens: Seq[Token], source: Source) {
     nonterminal:
       var left = nonTerminal()
       while (ops.contains(peek.kind)) {
+         checkBinaryOpWhitespace()
          val op = next()
          val right = nonTerminal()
          left = binaryOp(left, op, right)
@@ -1053,6 +1068,16 @@ class Parser(tokens: Seq[Token], source: Source) {
   def TypeTuple(tps: Many[Type]): Type =
     TypeRef(IdRef(List("effekt"), s"Tuple${tps.size}", tps.span.synthesized), tps, tps.span.synthesized)
 
+  // Check that the current token is surrounded by whitespace. If not, soft fail.
+  private def checkBinaryOpWhitespace(): Unit = {
+    val wsBefore = position     > 0             && isSpace(tokens(position - 1).kind)
+    val wsAfter  = position + 1 < tokens.length && isSpace(tokens(position + 1).kind)
+
+    if (!wsBefore || !wsAfter) {
+      warn(s"Missing whitespace around binary operator", position, position)
+    }
+  }
+
   /**
    * This is a compound production for
    *  - member selection <EXPR>.<NAME>
@@ -1101,7 +1126,7 @@ class Parser(tokens: Seq[Token], source: Source) {
   // argument lists cannot follow a linebreak:
   //   foo      ==    foo;
   //   ()             ()
-  def isArguments: Boolean = lookbehind(1).kind != Newline && (peek(`(`) || peek(`[`) || peek(`{`))
+  def isArguments: Boolean = !sawNewlineLast && (peek(`(`) || peek(`[`) || peek(`{`))
   def arguments(): (List[ValueType], List[ValueArg], List[Term]) =
     if (!isArguments) fail("at least one argument section (types, values, or blocks)", peek.kind)
     (maybeTypeArgs().unspan, maybeValueArgs(), maybeBlockArgs())
@@ -1595,9 +1620,11 @@ class Parser(tokens: Seq[Token], source: Source) {
   def fail(msg: String): Nothing =
     throw Fail(msg, position)
 
-  def softFail(message: String, start: Int, end: Int): Unit = {
-    softFails += SoftFail(message, start, end)
-  }
+  def softFail(message: String, start: Int, end: Int): Unit =
+    recoverableDiagnostics += RecoverableDiagnostic(kiama.util.Severities.Error, message, start, end)
+
+  def warn(message: String, start: Int, end: Int): Unit =
+    recoverableDiagnostics += RecoverableDiagnostic(kiama.util.Severities.Warning, message, start, end)
 
   inline def softFailWith[T](inline message: String)(inline p: => T): T = {
     val startPosition = position
@@ -1618,21 +1645,21 @@ class Parser(tokens: Seq[Token], source: Source) {
   inline def when[T](t: TokenKind)(inline thn: => T)(inline els: => T): T =
     if peek(t) then { consume(t); thn } else els
 
-  inline def backtrack[T](inline restoreSoftFails: Boolean = true)(inline p: => T): Maybe[T] =
+  inline def backtrack[T](inline restoreRecoverable: Boolean = true)(inline p: => T): Maybe[T] =
     val before = position
     val beforePrevious = previous
     val labelBefore = currentLabel
-    val softFailsBefore = softFails.clone()
+    val recoverableBefore = recoverableDiagnostics.clone()
     try { Maybe.Some(p, span(tokens(before).end)) } catch {
       case Fail(_, _) => {
         position = before
         previous = beforePrevious
         currentLabel = labelBefore
-        if restoreSoftFails then softFails = softFailsBefore
+        if restoreRecoverable then recoverableDiagnostics = recoverableBefore
         Maybe.None(Span(source, pos(), pos(), Synthesized))
       }
     }
-  inline def backtrack[T](inline p: => T): Maybe[T] = backtrack(restoreSoftFails = true)(p)
+  inline def backtrack[T](inline p: => T): Maybe[T] = backtrack(restoreRecoverable = true)(p)
 
   def interleave[A](xs: List[A], ys: List[A]): List[A] = (xs, ys) match {
     case (x :: xs, y :: ys) => x :: y :: interleave(xs, ys)

effekt/shared/src/main/scala/effekt/core/Parser.scala

@@ -6,7 +6,7 @@ import effekt.source.{FeatureFlag, Span}
 import effekt.symbols.builtins
 import effekt.util.messages.{ErrorReporter, ParseError}
 import kiama.parsing.{NoSuccess, ParseResult, Parsers, Success}
-import kiama.util.{Position, Range, Source, StringSource}
+import kiama.util.{Position, Range, Severities, Source, StringSource}
 
 class Names(private var knownNames: Map[String, Id]) {
   private val Suffix = """^(.*)\$(\d+)$""".r
@@ -249,7 +249,7 @@ class CoreParsers(names: Names) extends EffektLexers {
       case res: NoSuccess =>
         val input = res.next
         val range = Range(input.position, input.nextPosition)
-        C.report(ParseError(res.message, Some(range)))
+        C.report(ParseError(res.message, Some(range), Severities.Error))
         None
     }
 

effekt/shared/src/main/scala/effekt/util/ColoredMessaging.scala

@@ -55,7 +55,7 @@ trait ColoredMessaging extends EffektMessaging {
     messages.sorted.map(formatMessage).distinct.mkString("")
 
   override def formatContent(err: EffektError): String = err match {
-    case ParseError(msg, range)               => msg
+    case ParseError(msg, range, severity)     => msg
     case PlainTextError(msg, range, severity) => msg
     case StructuredError(StructuredMessage(sc, args), _, _) => sc.s(args.map {
       case id: source.IdDef    => highlight(TypePrinter.show(id))

effekt/shared/src/main/scala/effekt/util/Messages.scala

@@ -10,7 +10,7 @@ object messages {
   type EffektMessages = Vector[EffektError]
 
   sealed trait EffektError extends Message
-  case class ParseError(message: String, range: Option[Range]) extends EffektError { val severity = Error }
+  case class ParseError(message: String, range: Option[Range], severity: Severity) extends EffektError
   case class AmbiguousOverloadError(matches: List[(symbols.BlockSymbol, symbols.FunctionType)], range: Option[Range]) extends EffektError { val severity = Error }
   case class FailedOverloadError(failedAttempts: List[(symbols.BlockSymbol, symbols.FunctionType, EffektMessages)], range: Option[Range]) extends EffektError { val severity = Error }
   case class PlainTextError(content: String, range: Option[Range], severity: Severity) extends EffektError

examples/benchmarks/are_we_fast_yet/nbody.effekt

@@ -92,9 +92,9 @@ def run(n: Int) = {
     def offsetMomentum(body: Body, px: Double, py: Double, pz: Double) =
       Body(
         body.x, body.y, body.z,
-        0.0 -(px / SOLAR_MASS),
-        0.0 -(py / SOLAR_MASS),
-        0.0 -(pz / SOLAR_MASS),
+        0.0 - (px / SOLAR_MASS),
+        0.0 - (py / SOLAR_MASS),
+        0.0 - (pz / SOLAR_MASS),
         body.mass)
 
     bodies.unsafeSet(0, sun.offsetMomentum(px, py, pz))

examples/llvm/gids.effekt

@@ -22,6 +22,6 @@ def main() = region r {
         println(g())
         println(h())
     } with GID {
-        def gid() = { n = n+1; resume(n) }
+        def gid() = { n = n + 1; resume(n) }
     }
 }

examples/llvm/localfunctionasargument.effekt

@@ -3,10 +3,10 @@ def hof(x: Int){ f: Int => Int }: Int = {
 }
 
 def foo(a: Int) = {
-    def bar(b: Int): Int = b*a
+    def bar(b: Int): Int = b * a
     hof(5){bar}
 }
 
-def main() = { 
+def main() = {
     println(foo(2))
 }

examples/llvm/polymorphic_failtooption.effekt

@@ -22,7 +22,7 @@ def safeDiv(x: Int, y: Int): Int / Fail = {
     if (y == 0){
         do Fail(); 0
     } else {
-        x/y
+        x / y
     }
 }