Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve KindFormula.Error reporting #1085

Merged
merged 2 commits into from
Dec 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 17 additions & 8 deletions core/src/main/scala/org/bykn/bosatsu/KindFormula.scala
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,22 @@ object KindFormula {
case object Type extends KindFormula
case class Cons(arg: Arg, result: KindFormula) extends KindFormula

sealed abstract class Error
sealed abstract class Error {
def dt: DefinedType[Option[Kind.Arg]]
}
object Error {
case class Unsatisfiable(
dte: DefinedType[Either[KnownShape, Kind.Arg]],
constraints: LongMap[NonEmptyList[Constraint]],
existing: LongMap[Variance],
unknowns: SortedSet[Long]
) extends Error
) extends Error {
def dt: DefinedType[Option[Kind.Arg]] = dte.map(_.toOption)
}

case class FromShapeError(shapeError: Shape.Error) extends Error
case class FromShapeError(shapeError: Shape.Error) extends Error {
def dt = shapeError.dt
}
}

sealed abstract class Sat
Expand Down Expand Up @@ -291,7 +298,7 @@ object KindFormula {
// we have allocated all variance variables now, we can see how many
varCount <- state.nextId
topo = Impl.combineTopo(varCount, constraints)
maybeRes = Impl.go(constraints, dirs, topo).map { vars =>
maybeRes = Impl.go(dt, constraints, dirs, topo).map { vars =>
dtFormula.map(Impl.unformula(_, vars))
}
} yield maybeRes).run.value
Expand Down Expand Up @@ -354,6 +361,7 @@ object KindFormula {
// invariant: all subgraph values must be valid keys in the result
// we can process the subgraph list in parallel. Those are all the indepentent next values
def allSolutionChunk(
dt: DefinedType[Either[Shape.KnownShape, Kind.Arg]],
cons: Cons,
existing: LongMap[Variance],
directions: LongMap[Direction],
Expand Down Expand Up @@ -414,30 +422,32 @@ object KindFormula {
NonEmptyLazyList.fromLazyList(validVariances) match {
case Some(nel) => Validated.valid(nel)
case None =>
Validated.invalidNec(Error.Unsatisfiable(cons, existing, subgraph))
Validated.invalidNec(Error.Unsatisfiable(dt, cons, existing, subgraph))
}
}

def allSolutions(
dt: DefinedType[Either[Shape.KnownShape, Kind.Arg]],
cons: Cons,
existing: LongMap[Variance],
directions: LongMap[Direction],
subgraph: NonEmptyList[SortedSet[Long]]
): EitherNec[Error, NonEmptyLazyList[LongMap[Variance]]] =
subgraph
.traverse(allSolutionChunk(cons, existing, directions, _))
.traverse(allSolutionChunk(dt, cons, existing, directions, _))
.map(KindFormula.mergeCrossProduct(_))
.toEither

def go[F[_]: Foldable](
dt: DefinedType[Either[Shape.KnownShape, Kind.Arg]],
cons: Cons,
directions: LongMap[Direction],
topo: F[NonEmptyList[SortedSet[Long]]]
): ValidatedNec[Error, LongMap[Variance]] =
topo.foldM(NonEmptyLazyList(LongMap.empty[Variance])) {
(sols, subgraph) =>
// if there is at least one good solution, we can keep going
val next = sols.map(allSolutions(cons, _, directions, subgraph))
val next = sols.map(allSolutions(dt, cons, _, directions, subgraph))
val nextPaths: LazyList[LongMap[Variance]] =
next.toLazyList.flatMap {
case Right(ll) => ll.toLazyList
Expand Down Expand Up @@ -855,7 +865,6 @@ object KindFormula {
)
} yield ()
}

}
}

Expand Down
66 changes: 20 additions & 46 deletions core/src/main/scala/org/bykn/bosatsu/PackageError.scala
Original file line number Diff line number Diff line change
Expand Up @@ -534,90 +534,64 @@ object PackageError {
case class KindInferenceError(pack: PackageName, kindError: KindFormula.Error, regions: Map[Type.Const.Defined, Region]) extends PackageError {
def message(sourceMap: Map[PackageName, (LocationMap, String)], errColor: Colorize) = {
val (lm, _) = sourceMap.getMapSrc(pack)
kindError match {
case KindFormula.Error.Unsatisfiable(_, _, _) =>
val prefix = sourceMap.headLine(pack, None)
(prefix + Doc.text(s": $kindError")).render(80)
val region = regions(kindError.dt.toTypeConst)
val ctx = lm.showRegion(region, 2, errColor)
.getOrElse(Doc.str(region)) // we should highlight the whole region
val prefix = sourceMap.headLine(pack, Some(region))
val message = kindError match {
case KindFormula.Error.Unsatisfiable(_, _, _, _) =>
// TODO: would be good to give a more precise problem, e.g. which
// type parameters are the problem.
Doc.text("could not solve for valid variances")
case KindFormula.Error.FromShapeError(se) =>
se match {
case Shape.UnificationError(dt, cons, left, right) =>
val region = regions(dt.toTypeConst)
val prefix = sourceMap.headLine(pack, Some(region))
val ctx = lm.showRegion(region, 2, errColor)
.getOrElse(Doc.str(region)) // we should highlight the whole region
(prefix + Doc.hardLine + Doc.text("shape error: expected ") + Shape.shapeDoc(left) + Doc.text(" and ") + Shape.shapeDoc(right) +
Doc.text(s" to match in the constructor ${cons.name.sourceCodeRepr}") + Doc.hardLine + Doc.hardLine +
ctx).render(80)
case Shape.ShapeMismatch(dt, cons, outer, tyApp, right) =>
case Shape.UnificationError(_, cons, left, right) =>
Doc.text("shape error: expected ") + Shape.shapeDoc(left) + Doc.text(" and ") + Shape.shapeDoc(right) +
Doc.text(s" to match in the constructor ${cons.name.sourceCodeRepr}") + Doc.hardLine
case Shape.ShapeMismatch(_, cons, outer, tyApp, right) =>
val tmap = showTypes(pack, outer :: tyApp :: Nil)
val region = regions(dt.toTypeConst)
val prefix = sourceMap.headLine(pack, Some(region))
val ctx = lm.showRegion(region, 2, errColor)
.getOrElse(Doc.str(region)) // we should highlight the whole region
val typeDoc =
if (outer != tyApp) (tmap(outer) + Doc.text(" at application ") + tmap(tyApp))
else tmap(outer)

(prefix + Doc.text(" shape error: expected ") + Shape.shapeDoc(right) + Doc.text(" -> ?") + Doc.text(" but found * ") +
Doc.text("shape error: expected ") + Shape.shapeDoc(right) + Doc.text(" -> ?") + Doc.text(" but found * ") +
Doc.text(s"in the constructor ${cons.name.sourceCodeRepr} inside type ") +
typeDoc +
Doc.hardLine + Doc.hardLine +
ctx).render(80)
Doc.hardLine
case Shape.FinishFailure(dt, left, right) =>
val region = regions(dt.toTypeConst)
val tdoc = showTypes(pack, dt.toTypeTyConst :: Nil)(dt.toTypeTyConst)
val prefix = sourceMap.headLine(pack, Some(region))
val message = Doc.text("in type ") + tdoc + Doc.text(" could not unify shapes: ") + Shape.shapeDoc(left) + Doc.text(" and ") +
Doc.text("in type ") + tdoc + Doc.text(" could not unify shapes: ") + Shape.shapeDoc(left) + Doc.text(" and ") +
Shape.shapeDoc(right)
val ctx = lm.showRegion(region, 2, errColor)
.getOrElse(Doc.str(region)) // we should highlight the whole region
(prefix + Doc.hardLine + message + Doc.hardLine + ctx).render(80)
case Shape.ShapeLoop(dt, tpe, _) =>
val region = regions(dt.toTypeConst)
val tpe2 = tpe match {
case Left(ap) => ap
case Right(v) => Type.TyVar(v)
}
val tdocs = showTypes(pack, dt.toTypeTyConst :: tpe2 :: Nil)

val prefix = sourceMap.headLine(pack, Some(region))
val message = Doc.text("in type ") + tdocs(dt.toTypeTyConst) + Doc.text(" cyclic dependency encountered in ") +
Doc.text("in type ") + tdocs(dt.toTypeTyConst) + Doc.text(" cyclic dependency encountered in ") +
tdocs(tpe2)
val ctx = lm.showRegion(region, 2, errColor)
.getOrElse(Doc.str(region)) // we should highlight the whole region
(prefix + Doc.hardLine + message + Doc.hardLine + ctx).render(80)
case Shape.UnboundVar(dt, cfn, v) =>
val region = regions(dt.toTypeConst)
val tpe2 = Type.TyVar(v)
val tdocs = showTypes(pack, dt.toTypeTyConst :: tpe2 :: Nil)

val prefix = sourceMap.headLine(pack, Some(region))
val cfnMsg = if (dt.isStruct) Doc.empty else {
Doc.text(s" in constructor ${cfn.name.sourceCodeRepr} ")
}
val message = Doc.text("in type ") + tdocs(dt.toTypeTyConst) +
Doc.text("in type ") + tdocs(dt.toTypeTyConst) +
Doc.text(" unbound type variable ") + tdocs(tpe2) + cfnMsg

val ctx = lm.showRegion(region, 2, errColor)
.getOrElse(Doc.str(region)) // we should highlight the whole region
(prefix + Doc.hardLine + message + Doc.hardLine + ctx).render(80)
case Shape.UnknownConst(dt, cfn, c) =>
val region = regions(dt.toTypeConst)
val tpe2 = Type.TyConst(c)
val tdocs = showTypes(pack, dt.toTypeTyConst :: tpe2 :: Nil)

val prefix = sourceMap.headLine(pack, Some(region))
val cfnMsg = if (dt.isStruct) Doc.empty else {
Doc.text(s" in constructor ${cfn.name.sourceCodeRepr} ")
}
val message = Doc.text("in type ") + tdocs(dt.toTypeTyConst) +
Doc.text("in type ") + tdocs(dt.toTypeTyConst) +
Doc.text(" unknown type ") + tdocs(tpe2) + cfnMsg

val ctx = lm.showRegion(region, 2, errColor)
.getOrElse(Doc.str(region)) // we should highlight the whole region
(prefix + Doc.hardLine + message + Doc.hardLine + ctx).render(80)
}
}
(prefix + Doc.hardLine + message + Doc.hardLine + ctx).render(80)
}
}
}
5 changes: 4 additions & 1 deletion core/src/main/scala/org/bykn/bosatsu/Shape.scala
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ object Shape {
Doc.text("kind(") + tdoc + Doc.char(')')
}

sealed abstract class Error
sealed abstract class Error {
def dt: DefinedType[Option[Kind.Arg]]
}

case class ShapeLoop(
dt: DefinedType[Option[Kind.Arg]],
bound: Either[rankn.Type.TyApply, rankn.Type.Var.Bound],
Expand Down
18 changes: 17 additions & 1 deletion core/src/test/scala/org/bykn/bosatsu/EvaluationTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2806,7 +2806,8 @@ package Foo
struct Foo[a: *](a: a[Int])
""")) { case [email protected](_, _, _) =>
assert(kie.message(Map.empty, Colorize.None) ==
"""in file: <unknown source>, package Foo shape error: expected * -> ? but found * in the constructor Foo inside type a[Bosatsu/Predef::Int]
"""in file: <unknown source>, package Foo
shape error: expected * -> ? but found * in the constructor Foo inside type a[Bosatsu/Predef::Int]

Region(14,41)""")
()
Expand Down Expand Up @@ -3270,4 +3271,19 @@ res = z matches (1, 2, 3, 4, 5, 6, 7, 8, 9, 10,
()
}
}

test("kind errors show the region of the type") {
val testCode = """
package ErrorCheck

struct Foo[a: -*](get: a)

"""
evalFail(List(testCode)) { case [email protected](_, _, _) =>
val message = kie.message(Map.empty, Colorize.None)
assert(message.contains("Region(21,46)"))
assert(testCode.substring(21, 46) == "struct Foo[a: -*](get: a)")
()
}
}
}
Loading