Skip to content

Commit

Permalink
Add diagnostic information (#234)
Browse files Browse the repository at this point in the history
Closes #232 by catching common errors and presenting them to the user in
a more accessible way.
  • Loading branch information
j-mie6 authored Apr 9, 2024
2 parents 0ef18a3 + 985e706 commit d6f5db0
Show file tree
Hide file tree
Showing 9 changed files with 98 additions and 18 deletions.
10 changes: 9 additions & 1 deletion parsley/shared/src/main/scala/parsley/Parsley.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import parsley.combinator.option
import parsley.errors.ErrorBuilder
import parsley.expr.{chain, infix}

import parsley.internal.diagnostics.UserException
import parsley.internal.deepembedding.{frontend, singletons}
import parsley.internal.machine.Context

Expand Down Expand Up @@ -128,7 +129,14 @@ final class Parsley[+A] private [parsley] (private [parsley] val internal: front
* @since 3.0.0
* @group run
*/
def parse[Err: ErrorBuilder](input: String): Result[Err, A] = new Context(internal.instrs, input, internal.numRegs, None).run()
def parse[Err: ErrorBuilder](input: String): Result[Err, A] = {
try new Context(internal.instrs, input, internal.numRegs, None).run()
catch {
// $COVERAGE-OFF$
case UserException(err) => throw err // scalastyle:ignore throw
// $COVERAGE-ON$
}
}

// RESULT CHANGING COMBINATORS
/** This combinator allows the result of this parser to be changed using a given function.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* Copyright 2020 Parsley Contributors <https://github.com/j-mie6/Parsley/graphs/contributors>
*
* SPDX-License-Identifier: BSD-3-Clause
*/
package parsley.exceptions

// $COVERAGE-OFF$
private [parsley] class CorruptedReferenceException
extends ParsleyException("A reference has been used across two different parsers in separate calls to parse, causing it to be misallocated")
// $COVERAGE-ON$
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@
package parsley.exceptions

// $COVERAGE-OFF$
private [parsley] class UnfilledRegisterException
extends ParsleyException("A parser uses a register that has not been initialised by a `put`")
private [parsley] class UnfilledReferenceException
extends ParsleyException("A parser uses a reference that has not been initialised by a `set`")
// $COVERAGE-ON$
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ import scala.annotation.nowarn
import scala.collection.mutable

import parsley.XAssert._
import parsley.exceptions.BadLazinessException
import parsley.state.Ref

import parsley.internal.deepembedding.{Cont, ContOps, Id}, ContOps.{perform, result, ContAdapter}
import parsley.internal.deepembedding.backend, backend.StrictParsley
import parsley.internal.diagnostics.NullParserException
import parsley.internal.machine.instructions, instructions.Instr

/** This is the root type of the parsley "frontend": it represents a combinator tree
Expand Down Expand Up @@ -135,7 +135,6 @@ private [parsley] abstract class LazyParsley[+A] private [deepembedding] {
* @param seen the set of all nodes that have previously been seen by the let-finding
* @param state stores all the information of the let-finding process
*/
@throws[BadLazinessException]("if this parser references another parser before it has been initialised")
final protected [frontend] def findLets[M[_, +_]: ContOps, R](seen: Set[LazyParsley[_]])(implicit state: LetFinderState): M[R, Unit] = {
state.addPred(this)
if (seen.contains(this)) result(state.addRec(this))
Expand All @@ -148,7 +147,7 @@ private [parsley] abstract class LazyParsley[+A] private [deepembedding] {
try findLetsAux(seen + this)
catch {
// $COVERAGE-OFF$
case _: NullPointerException => throw new BadLazinessException // scalastyle:ignore throw
case NullParserException(err) => throw err // scalastyle:ignore throw
// $COVERAGE-ON$
}
}
Expand Down
51 changes: 51 additions & 0 deletions parsley/shared/src/main/scala/parsley/internal/diagnostics.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright 2020 Parsley Contributors <https://github.com/j-mie6/Parsley/graphs/contributors>
*
* SPDX-License-Identifier: BSD-3-Clause
*/
package parsley.internal.diagnostics

import parsley.exceptions.{BadLazinessException, CorruptedReferenceException, ParsleyException}

private [parsley] object UserException {
def unapply(e: Throwable): Option[Throwable] = e match {
case _: ParsleyException => None
case e if userStackTrace(e.getStackTrace) =>
e.setStackTrace(pruneParsley(e.getStackTrace))
Some(e)
case _ => None
}

def userStackTrace(e: Array[StackTraceElement]) = e.view.takeWhile(!_.getClassName.startsWith("parsley.internal")).exists { ste =>
!ste.getClassName.startsWith("scala") || !ste.getClassName.startsWith("java")
}
def pruneParsley(e: Array[StackTraceElement]): Array[StackTraceElement] = {
val (userBits, parsleyTrace) = e.span(!_.getClassName.startsWith("parsley.internal"))
userBits ++ parsleyTrace.dropWhile(_.getClassName.startsWith("parsley.internal"))
}
}

private [parsley] object RegisterOutOfBoundsException {
def unapply(e: Throwable): Option[Throwable] = e match {
case e: ArrayIndexOutOfBoundsException => e.getStackTrace.headOption.collect {
// this exception was thrown plainly during the execution of an instruction
// only register arrays are accessed raw like this: therefore it must be an
// out of bounds register.
case ste if ste.getMethodName == "apply"
&& ste.getClassName.startsWith("parsley.internal.machine.instructions") =>
val err = new CorruptedReferenceException
err.addSuppressed(e)
err
}
case _ => None
}
}

private [parsley] object NullParserException {
def unapply(e: Throwable): Option[Throwable] = e match {
// this should only be true when the null was tripped from within the parsley namespace,
// not the user one
case e: NullPointerException if !UserException.userStackTrace(e.getStackTrace) => Some(new BadLazinessException)
case _ => None
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import parsley.Success
import parsley.XAssert._
import parsley.errors.ErrorBuilder

import parsley.internal.diagnostics.RegisterOutOfBoundsException
import parsley.internal.errors.{CaretWidth, ExpectItem, LineBuilder, UnexpectDesc}
import parsley.internal.machine.errors.{ClassicFancyError, DefuncError, DefuncHints, EmptyHints,
ErrorItemBuilder, ExpectedError, ExpectedErrorWithReason, UnexpectedError}
Expand Down Expand Up @@ -124,11 +125,20 @@ private [parsley] final class Context(private [machine] var instrs: Array[Instr]
}
// $COVERAGE-ON$

@tailrec private [parsley] def run[Err: ErrorBuilder, A](): Result[Err, A] = {
private [parsley] def run[Err: ErrorBuilder, A](): Result[Err, A] = {
try go[Err, A]()
catch {
// additional diagnostic checks
// $COVERAGE-OFF$
case RegisterOutOfBoundsException(err) => throw err // scalastyle:ignore throw
// $COVERAGE-ON$
}
}
@tailrec private def go[Err: ErrorBuilder, A](): Result[Err, A] = {
//println(pretty)
if (running) { // this is the likeliest branch, so should be executed with fewest comparisons
instrs(pc)(this)
run[Err, A]()
go[Err, A]()
}
else if (good) {
assert(stack.size == 1, s"stack must end a parse with exactly one item, it has ${stack.size}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ import parsley.internal.errors.ExpectDesc
import parsley.internal.machine.Context
import parsley.internal.machine.XAssert._

import org.typelevel.scalaccompat.annotation.nowarn3

private [internal] final class Satisfies(f: Char => Boolean, expected: Iterable[ExpectDesc]) extends Instr {
def this(f: Char => Boolean, expected: LabelConfig) = this(f, expected.asExpectDescs)
override def apply(ctx: Context): Unit = {
Expand Down Expand Up @@ -157,7 +155,7 @@ private [internal] object Span extends Instr {
}

// This instruction holds mutate state, but it is safe to do so, because it's always the first instruction of a DynCall.
private [parsley] final class CalleeSave(var label: Int, localRegs: Set[Ref[_]] @nowarn3, reqSize: Int, slots: List[(Int, Int)], saveArray: Array[AnyRef])
private [parsley] final class CalleeSave(var label: Int, localRegs: Set[Ref[_]], reqSize: Int, slots: List[(Int, Int)], saveArray: Array[AnyRef])
extends InstrWithLabel {
private def this(label: Int, localRegs: Set[Ref[_]], reqSize: Int, slots: List[Int]) =
this(label, localRegs, reqSize, slots.zipWithIndex, new Array[AnyRef](slots.length))
Expand Down Expand Up @@ -190,7 +188,8 @@ private [parsley] final class CalleeSave(var label: Int, localRegs: Set[Ref[_]]
saveArray(idx) = null
}
// This is the only way to get them reallocated on the next invocation
localRegs.foreach(_.deallocate()): @nowarn3
// FIXME: I think this isn't thread-safe, because two flatMaps can simulataneously reallocate?
localRegs.foreach(_.deallocate())
}

private def continue(ctx: Context): Unit = {
Expand Down
10 changes: 6 additions & 4 deletions parsley/shared/src/main/scala/parsley/state.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import scala.collection.Factory
import parsley.XAssert._
import parsley.combinator.{whenS, whileS}
import parsley.syntax.zipped.Zipped2
import parsley.exceptions.UnfilledRegisterException
import parsley.exceptions.UnfilledReferenceException

import parsley.internal.deepembedding.{frontend, singletons}

Expand Down Expand Up @@ -277,16 +277,18 @@ object state {

private [this] var _v: Int = -1
private [parsley] def addr: Int = {
if (!allocated) throw new UnfilledRegisterException // scalastyle:ignore throw
if (!allocated) throw new UnfilledReferenceException // scalastyle:ignore throw
_v
}
private [parsley] def allocated: Boolean = _v != -1
private [parsley] def allocate(v: Int): Unit = {
assert(!allocated)
this._v = v
}
// This must ONLY be used by CalleeSave in flatMap
private [parsley] def deallocate(): Unit = _v = -1
private [parsley] def deallocate(): Unit = {
assert((new Throwable).getStackTrace.exists(_.getClassName == "parsley.internal.machine.instructions.CalleeSave"))
_v = -1
}
//override def toString: String = s"Reg(${if (allocated) addr else "unallocated"})"
}

Expand Down
4 changes: 2 additions & 2 deletions parsley/shared/src/test/scala/parsley/token/SpaceTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import parsley.Parsley.atomic

import descriptions.{SpaceDesc, LexicalDesc}
import parsley.character.{string, char}
import parsley.exceptions.UnfilledRegisterException
import parsley.exceptions.UnfilledReferenceException
import parsley.VanillaError
import parsley.Failure
import parsley.TestError
Expand Down Expand Up @@ -231,7 +231,7 @@ class SpaceTests extends ParsleyTest {
val basicDependent = basicMixed.copy(whitespaceIsContextDependent = true)

"context-dependent whitespace" must "be initialised" in {
a [UnfilledRegisterException] must be thrownBy {
a [UnfilledReferenceException] must be thrownBy {
makeSpace(basicDependent).whiteSpace.parse(" ")
}
}
Expand Down

0 comments on commit d6f5db0

Please sign in to comment.