From 5f1a2331b420cbe126cdfa3aeb37cc72b61a16f3 Mon Sep 17 00:00:00 2001 From: Oscar Boykin Date: Mon, 18 Sep 2023 17:09:47 -1000 Subject: [PATCH] Fix long bug in error regions --- .../main/scala/org/bykn/bosatsu/Expr.scala | 204 ++++---- .../scala/org/bykn/bosatsu/ListUtil.scala | 41 ++ .../scala/org/bykn/bosatsu/PackageError.scala | 436 +++++++++--------- .../org/bykn/bosatsu/SourceConverter.scala | 19 +- .../scala/org/bykn/bosatsu/TypedExpr.scala | 20 +- .../scala/org/bykn/bosatsu/rankn/Infer.scala | 160 +++++-- .../org/bykn/bosatsu/EvaluationTest.scala | 48 +- .../scala/org/bykn/bosatsu/ListUtilTest.scala | 82 ++++ 8 files changed, 628 insertions(+), 382 deletions(-) create mode 100644 core/src/main/scala/org/bykn/bosatsu/ListUtil.scala create mode 100644 core/src/test/scala/org/bykn/bosatsu/ListUtilTest.scala diff --git a/core/src/main/scala/org/bykn/bosatsu/Expr.scala b/core/src/main/scala/org/bykn/bosatsu/Expr.scala index 8a2ac6dd6..60365504c 100644 --- a/core/src/main/scala/org/bykn/bosatsu/Expr.scala +++ b/core/src/main/scala/org/bykn/bosatsu/Expr.scala @@ -7,7 +7,7 @@ package org.bykn.bosatsu import cats.implicits._ import cats.data.{Chain, Writer, NonEmptyList} -import cats.{Applicative, Eval, Traverse} +import cats.Applicative import scala.collection.immutable.SortedSet import org.bykn.bosatsu.rankn.Type @@ -15,6 +15,108 @@ import Identifier.{Bindable, Constructor} sealed abstract class Expr[T] { def tag: T + + /** + * All the free variables in this expression in order + * encountered and with duplicates (to see how often + * they appear) + */ + lazy val freeVarsDup: List[Bindable] = { + import Expr._ + // nearly identical code to TypedExpr.freeVarsDup, bugs should be fixed in both places + this match { + case Generic(_, expr) => + expr.freeVarsDup + case Annotation(t, _, _) => + t.freeVarsDup + case Local(ident, _) => + ident :: Nil + case Global(_, _, _) => + Nil + case Lambda(args, res, _) => + val nameSet = args.toList.iterator.map(_._1).toSet + ListUtil.filterNot(res.freeVarsDup)(nameSet) + case App(fn, args, _) => + fn.freeVarsDup ::: args.reduceMap(_.freeVarsDup) + case Let(arg, argE, in, rec, _) => + val argFree0 = argE.freeVarsDup + val argFree = + if (rec.isRecursive) { + ListUtil.filterNot(argFree0)(_ === arg) + } + else argFree0 + + argFree ::: (ListUtil.filterNot(in.freeVarsDup)(_ === arg)) + case Literal(_, _) => + Nil + case Match(arg, branches, _) => + val argFree = arg.freeVarsDup + + val branchFrees = branches.toList.map { case (p, b) => + // these are not free variables in this branch + val newBinds = p.names.toSet + val bfree = b.freeVarsDup + if (newBinds.isEmpty) bfree + else ListUtil.filterNot(bfree)(newBinds) + } + // we can only take one branch, so count the max on each branch: + val branchFreeMax = branchFrees + .zipWithIndex + .flatMap { case (names, br) => names.map((_, br)) } + // these groupBys are okay because we sort at the end + .groupBy(identity) // group-by-name x branch + .map { case ((name, branch), names) => (names.length, branch, name) } + .groupBy(_._3) // group by just the name now + .toList + .flatMap { case (_, vs) => + val (cnt, branch, name) = vs.maxBy(_._1) + List.fill(cnt)((branch, name)) + } + .sorted + .map(_._2) + + argFree ::: branchFreeMax + } + } + + lazy val globals: Set[Expr.Global[T]] = { + import Expr._ + // nearly identical code to TypedExpr.freeVarsDup, bugs should be fixed in both places + this match { + case Generic(_, expr) => + expr.globals + case Annotation(t, _, _) => + t.globals + case Local(_, _) => Set.empty + case g @ Global(_, _, _) => Set.empty + g + case Lambda(_, res, _) => res.globals + case App(fn, args, _) => + fn.globals | args.reduceMap(_.globals) + case Let(_, argE, in, _, _) => + argE.globals | in.globals + case Literal(_, _) => Set.empty + case Match(arg, branches, _) => + arg.globals | branches.reduceMap { case (_, b) => b.globals } + } + } + + def replaceTag(t: T): Expr[T] = { + import Expr._ + this match { + case Generic(_, _) => this + case a@Annotation(_, _, _) => a.copy(tag = t) + case l@Local(_, _) => l.copy(tag = t) + case g @ Global(_, _, _) => g.copy(tag = t) + case l@Lambda(_, _, _) => l.copy(tag = t) + case a@App(_, _, _) => a.copy(tag = t) + case l@Let(_, _, _, _, _) => l.copy(tag = t) + case l@Literal(_, _) => l.copy(tag = t) + case m@Match(_, _, _) => m.copy(tag = t) + } + } + + def notFree(b: Bindable): Boolean = + !freeVarsDup.contains(b) } object Expr { @@ -188,106 +290,6 @@ object Expr { } } - /* - * We have seen some intermitten CI failures if this isn't lazy - * presumably due to initialiazation order - */ - implicit lazy val exprTraverse: Traverse[Expr] = - new Traverse[Expr] { - - // Traverse on NonEmptyList[(Pattern[_], Expr[?])] - private lazy val tne = { - type Tup[T] = (Pattern[(PackageName, Constructor), Type], T) - type TupExpr[T] = (Pattern[(PackageName, Constructor), Type], Expr[T]) - val tup: Traverse[TupExpr] = Traverse[Tup].compose(exprTraverse) - Traverse[NonEmptyList].compose(tup) - } - - def traverse[G[_]: Applicative, A, B](fa: Expr[A])(f: A => G[B]): G[Expr[B]] = - fa match { - case Annotation(e, tpe, a) => - (e.traverse(f), f(a)).mapN(Annotation(_, tpe, _)) - case Local(s, t) => - f(t).map(Local(s, _)) - case Global(p, s, t) => - f(t).map(Global(p, s, _)) - case Generic(bs, e) => - traverse(e)(f).map(Generic(bs, _)) - case App(fn, args, t) => - (fn.traverse(f), args.traverse(_.traverse(f)), f(t)).mapN { (fn1, a1, b) => - App(fn1, a1, b) - } - case Lambda(args, expr, t) => - (expr.traverse(f), f(t)).mapN { (e1, t1) => - Lambda(args, e1, t1) - } - case Let(arg, exp, in, rec, tag) => - (exp.traverse(f), in.traverse(f), f(tag)).mapN { (e1, i1, t1) => - Let(arg, e1, i1, rec, t1) - } - case Literal(lit, tag) => - f(tag).map(Literal(lit, _)) - case Match(arg, branches, tag) => - val argB = arg.traverse(f) - val branchB = tne.traverse(branches)(f) - (argB, branchB, f(tag)).mapN { (a, bs, t) => - Match(a, bs, t) - } - } - - def foldLeft[A, B](fa: Expr[A], b: B)(f: (B, A) => B): B = - fa match { - case Annotation(e, _, tag) => - val b1 = foldLeft(e, b)(f) - f(b1, tag) - case n: Name[A] => f(b, n.tag) - case App(fn, args, tag) => - val b1 = foldLeft(fn, b)(f) - val b2 = args.foldLeft(b1) { (b1, x) => foldLeft(x, b1)(f) } - f(b2, tag) - case Generic(_, in) => foldLeft(in, b)(f) - case Lambda(_, expr, tag) => - val b1 = foldLeft(expr, b)(f) - f(b1, tag) - case Let(_, exp, in, _, tag) => - val b1 = foldLeft(exp, b)(f) - val b2 = foldLeft(in, b1)(f) - f(b2, tag) - case Literal(_, tag) => - f(b, tag) - case Match(arg, branches, tag) => - val b1 = foldLeft(arg, b)(f) - val b2 = tne.foldLeft(branches, b1)(f) - f(b2, tag) - } - - def foldRight[A, B](fa: Expr[A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] = - fa match { - case Annotation(e, _, tag) => - val lb1 = foldRight(e, lb)(f) - f(tag, lb1) - case n: Name[A] => f(n.tag, lb) - case App(fn, args, tag) => - val b1 = f(tag, lb) - val b2 = args.foldRight(b1)((a, b1) => foldRight(a, b1)(f)) - foldRight(fn, b2)(f) - case Generic(_, in) => foldRight(in, lb)(f) - case Lambda(_, expr, tag) => - val b1 = f(tag, lb) - foldRight(expr, b1)(f) - case Let(_, exp, in, _, tag) => - val b1 = f(tag, lb) - val b2 = foldRight(in, b1)(f) - foldRight(exp, b2)(f) - case Literal(_, tag) => - f(tag, lb) - case Match(arg, branches, tag) => - val b1 = f(tag, lb) - val b2 = tne.foldRight(branches, b1)(f) - foldRight(arg, b2)(f) - } - } - def buildPatternLambda[A]( args: NonEmptyList[Pattern[(PackageName, Constructor), Type]], body: Expr[A], diff --git a/core/src/main/scala/org/bykn/bosatsu/ListUtil.scala b/core/src/main/scala/org/bykn/bosatsu/ListUtil.scala new file mode 100644 index 000000000..22af4a10f --- /dev/null +++ b/core/src/main/scala/org/bykn/bosatsu/ListUtil.scala @@ -0,0 +1,41 @@ +package org.bykn.bosatsu + +import cats.data.NonEmptyList + +private[bosatsu] object ListUtil { + // filter b from a pretty short lst but try to conserve lst if possible + def filterNot[A](lst: List[A])(b: A => Boolean): List[A] = + lst match { + case Nil => lst + case h :: tail => + val t1 = filterNot(tail)(b) + if (b(h)) t1 + else if (t1 eq tail) lst + else (h :: t1) // we only allocate here + } + + def greedyGroup[A, G](list: NonEmptyList[A])(one: A => G)(combine: (G, A) => Option[G]): NonEmptyList[G] = { + def loop(g: G, tail: List[A]): NonEmptyList[G] = + tail match { + case Nil => NonEmptyList.one(g) + case tailh :: tailt => + combine(g, tailh) match { + case None => + // can't combine into the head group, start a new group + g :: loop(one(tailh), tailt) + case Some(g1) => + // we can combine into a new group + loop(g1, tailt) + } + } + + loop(one(list.head), list.tail) + } + + def greedyGroup[A, G](list: List[A])(one: A => G)(combine: (G, A) => Option[G]): List[G] = + NonEmptyList.fromList(list) match { + case None => Nil + case Some(nel) => greedyGroup(nel)(one)(combine).toList + } + +} \ No newline at end of file diff --git a/core/src/main/scala/org/bykn/bosatsu/PackageError.scala b/core/src/main/scala/org/bykn/bosatsu/PackageError.scala index 06abc9661..c1c1aa460 100644 --- a/core/src/main/scala/org/bykn/bosatsu/PackageError.scala +++ b/core/src/main/scala/org/bykn/bosatsu/PackageError.scala @@ -188,233 +188,243 @@ object PackageError { case class TypeErrorIn(tpeErr: Infer.Error, pack: PackageName) extends PackageError { def message(sourceMap: Map[PackageName, (LocationMap, String)], errColor: Colorize) = { val (lm, _) = sourceMap.getMapSrc(pack) - val (teMessage, region) = tpeErr match { - case Infer.Error.NotUnifiable(t0, t1, r0, r1) => - val context0 = - if (r0 == r1) Doc.space // sometimes the region of the error is the same on right and left - else { - val m = lm.showRegion(r0, 2, errColor).getOrElse(Doc.str(r0)) // we should highlight the whole region - Doc.hardLine + m + Doc.hardLine - } - val context1 = - lm.showRegion(r1, 2, errColor).getOrElse(Doc.str(r1)) // we should highlight the whole region - - val fnHint = - (t0, t1) match { - case (Type.RootConst(Type.FnType(_, leftSize)), - Type.RootConst(Type.FnType(_, rightSize))) => - // both are functions - def args(n: Int) = if (n == 1) "one argument" else s"$n arguments" - Doc.text(s"hint: the first type is a function with ${args(leftSize)} and the second is a function with ${args(rightSize)}.") + Doc.hardLine - case (Type.Fun(_, _), _) | (_, Type.Fun(_, _)) => - Doc.text("hint: this often happens when you apply the wrong number of arguments to a function.") + Doc.hardLine - case _ => - Doc.empty - } + def singleToDoc(tpeErr: Infer.Error.Single): Doc = { + val (teMessage, region) = tpeErr match { + case Infer.Error.NotUnifiable(t0, t1, r0, r1) => + val context0 = + if (r0 == r1) Doc.space // sometimes the region of the error is the same on right and left + else { + val m = lm.showRegion(r0, 2, errColor).getOrElse(Doc.str(r0)) // we should highlight the whole region + Doc.hardLine + m + Doc.hardLine + } + val context1 = + lm.showRegion(r1, 2, errColor).getOrElse(Doc.str(r1)) // we should highlight the whole region + + val fnHint = + (t0, t1) match { + case (Type.RootConst(Type.FnType(_, leftSize)), + Type.RootConst(Type.FnType(_, rightSize))) => + // both are functions + def args(n: Int) = if (n == 1) "one argument" else s"$n arguments" + Doc.text(s"hint: the first type is a function with ${args(leftSize)} and the second is a function with ${args(rightSize)}.") + Doc.hardLine + case (Type.Fun(_, _), _) | (_, Type.Fun(_, _)) => + Doc.text("hint: this often happens when you apply the wrong number of arguments to a function.") + Doc.hardLine + case _ => + Doc.empty + } - val tmap = showTypes(pack, List(t0, t1)) - val doc = Doc.text("type error: expected type ") + tmap(t0) + - context0 + Doc.text("to be the same as type ") + tmap(t1) + - Doc.hardLine + fnHint + context1 + val tmap = showTypes(pack, List(t0, t1)) + val doc = Doc.text("type error: expected type ") + tmap(t0) + + context0 + Doc.text("to be the same as type ") + tmap(t1) + + Doc.hardLine + fnHint + context1 + + (doc, Some(r0)) + case Infer.Error.VarNotInScope((_, name), scope, region) => + val ctx = lm.showRegion(region, 2, errColor).getOrElse(Doc.str(region)) + val candidates: List[String] = + nearest(name, scope.map { case ((_, n), _) => (n, ()) }, 3) + .map { case (n, _) => n.asString } + + val cmessage = + if (candidates.nonEmpty) candidates.mkString("\nClosest: ", ", ", ".\n") + else "" + val qname = "\"" + name.sourceCodeRepr + "\"" + (Doc.text("name ") + Doc.text(qname) + Doc.text(" unknown.") + Doc.text(cmessage) + Doc.hardLine + + ctx, Some(region)) + case Infer.Error.SubsumptionCheckFailure(t0, t1, r0, r1, _) => + val context0 = + if (r0 == r1) Doc.space // sometimes the region of the error is the same on right and left + else { + val m = lm.showRegion(r0, 2, errColor).getOrElse(Doc.str(r0)) // we should highlight the whole region + Doc.hardLine + m + Doc.hardLine + } + val context1 = + lm.showRegion(r1, 2, errColor).getOrElse(Doc.str(r1)) // we should highlight the whole region + + val tmap = showTypes(pack, List(t0, t1)) + val doc = Doc.text("type ") + tmap(t0) + context0 + + Doc.text("does not subsume type ") + tmap(t1) + Doc.hardLine + + context1 - (doc, Some(r0)) - case Infer.Error.VarNotInScope((_, name), scope, region) => - val ctx = lm.showRegion(region, 2, errColor).getOrElse(Doc.str(region)) - val candidates: List[String] = - nearest(name, scope.map { case ((_, n), _) => (n, ()) }, 3) + (doc, Some(r0)) + case uc@Infer.Error.UnknownConstructor((_, n), region, _) => + val near = nearest(n, uc.knownConstructors.map { case (_, n) => (n, ()) }.toMap, 3) .map { case (n, _) => n.asString } - val cmessage = - if (candidates.nonEmpty) candidates.mkString("\nClosest: ", ", ", ".\n") - else "" - val qname = "\"" + name.sourceCodeRepr + "\"" - (Doc.text("name ") + Doc.text(qname) + Doc.text(" unknown.") + Doc.text(cmessage) + Doc.hardLine + - ctx, Some(region)) - case Infer.Error.SubsumptionCheckFailure(t0, t1, r0, r1, _) => - val context0 = - if (r0 == r1) Doc.space // sometimes the region of the error is the same on right and left - else { - val m = lm.showRegion(r0, 2, errColor).getOrElse(Doc.str(r0)) // we should highlight the whole region - Doc.hardLine + m + Doc.hardLine - } - val context1 = - lm.showRegion(r1, 2, errColor).getOrElse(Doc.str(r1)) // we should highlight the whole region - - val tmap = showTypes(pack, List(t0, t1)) - val doc = Doc.text("type ") + tmap(t0) + context0 + - Doc.text("does not subsume type ") + tmap(t1) + Doc.hardLine + - context1 - - (doc, Some(r0)) - case uc@Infer.Error.UnknownConstructor((_, n), region, _) => - val near = nearest(n, uc.knownConstructors.map { case (_, n) => (n, ()) }.toMap, 3) - .map { case (n, _) => n.asString } - - val nearStr = - if (near.isEmpty) "" - else near.mkString(", nearest: ", ", ", "") - - val context = - lm.showRegion(region, 2, errColor).getOrElse(Doc.str(region)) // we should highlight the whole region - - val doc = Doc.text("unknown constructor ") + Doc.text(n.asString) + - Doc.text(nearStr) + Doc.hardLine + context - (doc, Some(region)) - case Infer.Error.KindCannotTyApply(applied, region) => - val tmap = showTypes(pack, applied :: Nil) - val context = - lm.showRegion(region, 2, errColor).getOrElse(Doc.str(region)) // we should highlight the whole region - val doc = Doc.text("kind error: for kind of the left of ") + - tmap(applied) + Doc.text(" is *. Cannot apply to kind *.") + Doc.hardLine + - context - - (doc, Some(region)) - case Infer.Error.KindInvalidApply(applied, leftK, rightK, region) => - val leftT = applied.on - val rightT = applied.arg - val tmap = showTypes(pack, applied :: leftT :: rightT :: Nil) - val context = - lm.showRegion(region, 2, errColor).getOrElse(Doc.str(region)) - val doc = Doc.text("kind error: ") + Doc.text("the type: ") + tmap(applied) + - Doc.text(" is invalid because the left ") + tmap(leftT) + Doc.text(" has kind ") + Kind.toDoc(leftK) + - Doc.text(" and the right ") + tmap(rightT) + Doc.text(" has kind ") + Kind.toDoc(rightK) + - Doc.text(s" but left cannot accept the kind of the right:") + - Doc.hardLine + - context - - (doc, Some(region)) - case Infer.Error.KindMetaMismatch(meta, rightT, rightK, metaR, rightR) => - val tmap = showTypes(pack, meta :: rightT :: Nil) - val context0 = - lm.showRegion(metaR, 2, errColor).getOrElse(Doc.str(metaR)) // we should highlight the whole region - val context1 = { - if (metaR != rightR) { - Doc.text(" at: ") + Doc.hardLine + - lm.showRegion(rightR, 2, errColor).getOrElse(Doc.str(rightR)) + // we should highlight the whole region - Doc.hardLine - } - else { - Doc.empty + val nearStr = + if (near.isEmpty) "" + else near.mkString(", nearest: ", ", ", "") + + val context = + lm.showRegion(region, 2, errColor).getOrElse(Doc.str(region)) // we should highlight the whole region + + val doc = Doc.text("unknown constructor ") + Doc.text(n.asString) + + Doc.text(nearStr) + Doc.hardLine + context + (doc, Some(region)) + case Infer.Error.KindCannotTyApply(applied, region) => + val tmap = showTypes(pack, applied :: Nil) + val context = + lm.showRegion(region, 2, errColor).getOrElse(Doc.str(region)) // we should highlight the whole region + val doc = Doc.text("kind error: for kind of the left of ") + + tmap(applied) + Doc.text(" is *. Cannot apply to kind *.") + Doc.hardLine + + context + + (doc, Some(region)) + case Infer.Error.KindInvalidApply(applied, leftK, rightK, region) => + val leftT = applied.on + val rightT = applied.arg + val tmap = showTypes(pack, applied :: leftT :: rightT :: Nil) + val context = + lm.showRegion(region, 2, errColor).getOrElse(Doc.str(region)) + val doc = Doc.text("kind error: ") + Doc.text("the type: ") + tmap(applied) + + Doc.text(" is invalid because the left ") + tmap(leftT) + Doc.text(" has kind ") + Kind.toDoc(leftK) + + Doc.text(" and the right ") + tmap(rightT) + Doc.text(" has kind ") + Kind.toDoc(rightK) + + Doc.text(s" but left cannot accept the kind of the right:") + + Doc.hardLine + + context + + (doc, Some(region)) + case Infer.Error.KindMetaMismatch(meta, rightT, rightK, metaR, rightR) => + val tmap = showTypes(pack, meta :: rightT :: Nil) + val context0 = + lm.showRegion(metaR, 2, errColor).getOrElse(Doc.str(metaR)) // we should highlight the whole region + val context1 = { + if (metaR != rightR) { + Doc.text(" at: ") + Doc.hardLine + + lm.showRegion(rightR, 2, errColor).getOrElse(Doc.str(rightR)) + // we should highlight the whole region + Doc.hardLine + } + else { + Doc.empty + } } - } - val doc = Doc.text("kind error: ") + Doc.text("the type: ") + tmap(meta) + - Doc.text(" of kind: ") + Kind.toDoc(meta.toMeta.kind) + Doc.text(" at: ") + Doc.hardLine + - context0 + Doc.hardLine + Doc.hardLine + - Doc.text("cannot be unified with the type ") + tmap(rightT) + - Doc.text(" of kind: ") + Kind.toDoc(rightK) + context1 + - Doc.hardLine + - Doc.text("because the first kind does not subsume the second.") - - (doc, Some(metaR)) - case Infer.Error.UnexpectedMeta(meta, in, metaR, rightR) => - val tymeta = Type.TyMeta(meta) - val tmap = showTypes(pack, tymeta :: in :: Nil) - val context0 = - lm.showRegion(metaR, 2, errColor).getOrElse(Doc.str(metaR)) // we should highlight the whole region - val context1 = { - if (metaR != rightR) { - Doc.text(" at: ") + Doc.hardLine + - lm.showRegion(rightR, 2, errColor).getOrElse(Doc.str(rightR)) + // we should highlight the whole region - Doc.hardLine - } - else { - Doc.empty + val doc = Doc.text("kind error: ") + Doc.text("the type: ") + tmap(meta) + + Doc.text(" of kind: ") + Kind.toDoc(meta.toMeta.kind) + Doc.text(" at: ") + Doc.hardLine + + context0 + Doc.hardLine + Doc.hardLine + + Doc.text("cannot be unified with the type ") + tmap(rightT) + + Doc.text(" of kind: ") + Kind.toDoc(rightK) + context1 + + Doc.hardLine + + Doc.text("because the first kind does not subsume the second.") + + (doc, Some(metaR)) + case Infer.Error.UnexpectedMeta(meta, in, metaR, rightR) => + val tymeta = Type.TyMeta(meta) + val tmap = showTypes(pack, tymeta :: in :: Nil) + val context0 = + lm.showRegion(metaR, 2, errColor).getOrElse(Doc.str(metaR)) // we should highlight the whole region + val context1 = { + if (metaR != rightR) { + Doc.text(" at: ") + Doc.hardLine + + lm.showRegion(rightR, 2, errColor).getOrElse(Doc.str(rightR)) + // we should highlight the whole region + Doc.hardLine + } + else { + Doc.empty + } } - } - val doc = Doc.text("Unexpected unknown: the type: ") + tmap(tymeta) + - Doc.text(" of kind: ") + Kind.toDoc(meta.kind) + Doc.text(" at: ") + Doc.hardLine + - context0 + Doc.hardLine + Doc.hardLine + - Doc.text("inside the type ") + tmap(in) + context1 + - Doc.hardLine + - Doc.text("this sometimes happens when a function arg has been omitted, or an illegal recursive type or function.") - - (doc, Some(metaR)) - case Infer.Error.KindNotUnifiable(leftK, leftT, rightK, rightT, leftR, rightR) => - val tStr = showTypes(pack, leftT :: rightT :: Nil) - - val context0 = - lm.showRegion(leftR, 2, errColor).getOrElse(Doc.str(leftR)) - val context1 = { - if (leftR != rightR) { - Doc.text(" at: ") + Doc.hardLine + - lm.showRegion(rightR, 2, errColor).getOrElse(Doc.str(rightR)) - } - else { - Doc.empty + val doc = Doc.text("Unexpected unknown: the type: ") + tmap(tymeta) + + Doc.text(" of kind: ") + Kind.toDoc(meta.kind) + Doc.text(" at: ") + Doc.hardLine + + context0 + Doc.hardLine + Doc.hardLine + + Doc.text("inside the type ") + tmap(in) + context1 + + Doc.hardLine + + Doc.text("this sometimes happens when a function arg has been omitted, or an illegal recursive type or function.") + + (doc, Some(metaR)) + case Infer.Error.KindNotUnifiable(leftK, leftT, rightK, rightT, leftR, rightR) => + val tStr = showTypes(pack, leftT :: rightT :: Nil) + + val context0 = + lm.showRegion(leftR, 2, errColor).getOrElse(Doc.str(leftR)) + val context1 = { + if (leftR != rightR) { + Doc.text(" at: ") + Doc.hardLine + + lm.showRegion(rightR, 2, errColor).getOrElse(Doc.str(rightR)) + } + else { + Doc.empty + } } - } - val doc = Doc.text("kind mismatch error: ") + - tStr(leftT) + Doc.text(": ") + Kind.toDoc(leftK) + Doc.text(" at:") + Doc.hardLine + context0 + - Doc.text(" cannot be unified with kind: ") + - tStr(rightT) + Doc.text(": ") + Kind.toDoc(rightK) + context1 - - (doc, Some(leftR)) - case Infer.Error.NotPolymorphicEnough(tpe, _, _, region) => - val tmap = showTypes(pack, tpe :: Nil) - val context = - lm.showRegion(region, 2, errColor).getOrElse(Doc.str(region)) - - (Doc.text("the type ") + tmap(tpe) + Doc.text(" is not polymorphic enough") + Doc.hardLine + context, Some(region)) - case Infer.Error.ArityMismatch(leftA, leftR, rightA, rightR) => - val context0 = - lm.showRegion(leftR, 2, errColor).getOrElse(Doc.str(leftR)) - val context1 = { - if (leftR != rightR) { - Doc.text(" at: ") + Doc.hardLine + - lm.showRegion(rightR, 2, errColor).getOrElse(Doc.str(rightR)) - } - else { - Doc.empty + val doc = Doc.text("kind mismatch error: ") + + tStr(leftT) + Doc.text(": ") + Kind.toDoc(leftK) + Doc.text(" at:") + Doc.hardLine + context0 + + Doc.text(" cannot be unified with kind: ") + + tStr(rightT) + Doc.text(": ") + Kind.toDoc(rightK) + context1 + + (doc, Some(leftR)) + case Infer.Error.NotPolymorphicEnough(tpe, _, _, region) => + val tmap = showTypes(pack, tpe :: Nil) + val context = + lm.showRegion(region, 2, errColor).getOrElse(Doc.str(region)) + + (Doc.text("the type ") + tmap(tpe) + Doc.text(" is not polymorphic enough") + Doc.hardLine + context, Some(region)) + case Infer.Error.ArityMismatch(leftA, leftR, rightA, rightR) => + val context0 = + lm.showRegion(leftR, 2, errColor).getOrElse(Doc.str(leftR)) + val context1 = { + if (leftR != rightR) { + Doc.text(" at: ") + Doc.hardLine + + lm.showRegion(rightR, 2, errColor).getOrElse(Doc.str(rightR)) + } + else { + Doc.empty + } } - } - def args(n: Int) = - if (n == 1) "one argument" else s"$n arguments" - - (Doc.text(s"function with ${args(leftA)} at:") + Doc.hardLine + context0 + - Doc.text(s" does not match function with ${args(rightA)}") + context1, Some(leftR)) - case Infer.Error.ArityTooLarge(found, max, region) => - val context = - lm.showRegion(region, 2, errColor).getOrElse(Doc.str(region)) - - (Doc.text(s"function with $found arguments is too large. Maximum function argument count is $max.") + Doc.hardLine + context, - Some(region)) - case Infer.Error.UnexpectedBound(bound, _, reg, _) => - val tyvar = Type.TyVar(bound) - val tmap = showTypes(pack, tyvar :: Nil) - val context = - lm.showRegion(reg, 2, errColor).getOrElse(Doc.str(reg)) - - (Doc.text("unexpected bound: ") + tmap(tyvar) + Doc.hardLine + context, Some(reg)) - case Infer.Error.UnionPatternBindMismatch(_, names, region) => - val context = - lm.showRegion(region, 2, errColor).getOrElse(Doc.str(region)) - - val uniqueSets = graph.Tree.distinctBy(names)(_.toSet) - val uniqs = Doc.intercalate(Doc.char(',') + Doc.line, - uniqueSets.toList.map { names => - Doc.text(names.iterator.map(_.sourceCodeRepr).mkString("[", ", ", "]")) - } - ) - (Doc.text("not all union elements bind the same names: ") + - (Doc.line + uniqs + context).nested(4).grouped, - Some(region)) - case Infer.Error.UnknownDefined(const, reg) => - val tpe = Type.TyConst(const) - val tmap = showTypes(pack, tpe :: Nil) - val context = - lm.showRegion(reg, 2, errColor).getOrElse(Doc.str(reg)) - - (Doc.text("unknown type: ") + tmap(tpe) + Doc.hardLine + context, Some(reg)) - case ie: Infer.Error.InternalError => - (Doc.text(ie.message), Some(ie.region)) + def args(n: Int) = + if (n == 1) "one argument" else s"$n arguments" + + (Doc.text(s"function with ${args(leftA)} at:") + Doc.hardLine + context0 + + Doc.text(s" does not match function with ${args(rightA)}") + context1, Some(leftR)) + case Infer.Error.ArityTooLarge(found, max, region) => + val context = + lm.showRegion(region, 2, errColor).getOrElse(Doc.str(region)) + + (Doc.text(s"function with $found arguments is too large. Maximum function argument count is $max.") + Doc.hardLine + context, + Some(region)) + case Infer.Error.UnexpectedBound(bound, _, reg, _) => + val tyvar = Type.TyVar(bound) + val tmap = showTypes(pack, tyvar :: Nil) + val context = + lm.showRegion(reg, 2, errColor).getOrElse(Doc.str(reg)) + + (Doc.text("unexpected bound: ") + tmap(tyvar) + Doc.hardLine + context, Some(reg)) + case Infer.Error.UnionPatternBindMismatch(_, names, region) => + val context = + lm.showRegion(region, 2, errColor).getOrElse(Doc.str(region)) + + val uniqueSets = graph.Tree.distinctBy(names)(_.toSet) + val uniqs = Doc.intercalate(Doc.char(',') + Doc.line, + uniqueSets.toList.map { names => + Doc.text(names.iterator.map(_.sourceCodeRepr).mkString("[", ", ", "]")) + } + ) + (Doc.text("not all union elements bind the same names: ") + + (Doc.line + uniqs + context).nested(4).grouped, + Some(region)) + case Infer.Error.UnknownDefined(const, reg) => + val tpe = Type.TyConst(const) + val tmap = showTypes(pack, tpe :: Nil) + val context = + lm.showRegion(reg, 2, errColor).getOrElse(Doc.str(reg)) + + (Doc.text("unknown type: ") + tmap(tpe) + Doc.hardLine + context, Some(reg)) + case ie: Infer.Error.InternalError => + (Doc.text(ie.message), Some(ie.region)) + } + val h = sourceMap.headLine(pack, region) + (h + Doc.hardLine + teMessage) + } + + val finalDoc = tpeErr match { + case s: Infer.Error.Single => singleToDoc(s) + case c@Infer.Error.Combine(_, _) => + val twoLines = Doc.hardLine + Doc.hardLine + c.flatten.iterator.map(singleToDoc).reduce((a, b) => a + (twoLines + b)) } - val h = sourceMap.headLine(pack, region) - (h + Doc.hardLine + teMessage).render(80) + finalDoc.render(80) } } diff --git a/core/src/main/scala/org/bykn/bosatsu/SourceConverter.scala b/core/src/main/scala/org/bykn/bosatsu/SourceConverter.scala index c6afb9a29..5cb247c7f 100644 --- a/core/src/main/scala/org/bykn/bosatsu/SourceConverter.scala +++ b/core/src/main/scala/org/bykn/bosatsu/SourceConverter.scala @@ -191,6 +191,7 @@ final class SourceConverter( case Binding(BindingStatement(pat, value, Padding(_, rest))) => val erest = withBound(rest, pat.names) + val assignRegion = decl.region - value.region def solvePat(pat: Pattern.Parsed, rrhs: Result[Expr[Declaration]]): Result[Expr[Declaration]] = pat match { case Pattern.Var(arg) => @@ -198,9 +199,11 @@ final class SourceConverter( Expr.Let(arg, rhs, e, RecursionKind.NonRecursive, decl) } case Pattern.Annotation(pat, tpe) => - toType(tpe, decl.region).flatMap { realTpe => + toType(tpe, assignRegion).flatMap { realTpe => // move the annotation to the right - val newRhs = rrhs.map(Expr.Annotation(_, realTpe, decl)) + // it's not ideal to use the Declaration of rhs, but it's better + // than the entire let + val newRhs = rrhs.map { r => Expr.Annotation(r, realTpe, r.tag) } solvePat(pat, newRhs) } case Pattern.Named(nm, p) => @@ -210,7 +213,7 @@ final class SourceConverter( } case pat => // TODO: we need the region on the pattern... - (convertPattern(pat, decl.region - value.region), erest, rrhs).parMapN { (newPattern, e, rhs) => + (convertPattern(pat, assignRegion), erest, rrhs).parMapN { (newPattern, e, rhs) => val expBranches = NonEmptyList.of((newPattern, e)) Expr.Match(rhs, expBranches, decl) } @@ -218,9 +221,9 @@ final class SourceConverter( solvePat(pat, loop(value)) case Comment(CommentStatement(_, Padding(_, decl))) => - loop(decl).map(_.as(decl)) + loop(decl).map(_.replaceTag(decl)) case CommentNB(CommentStatement(_, Padding(_, decl))) => - loop(decl).map(_.as(decl)) + loop(decl).map(_.replaceTag(decl)) case DefFn(defstmt@DefStatement(_, _, _, _, _)) => val inExpr = defstmt.result match { case (_, Padding(_, in)) => withBound(in, defstmt.name :: Nil) @@ -260,11 +263,11 @@ final class SourceConverter( Expr.buildPatternLambda(args, body, decl) } case la@LeftApply(_, _, _, _) => - loop(la.rewrite).map(_.as(decl)) + loop(la.rewrite).map(_.replaceTag(decl)) case Literal(lit) => success(Expr.Literal(lit, decl)) case Parens(p) => - loop(p).map(_.as(decl)) + loop(p).map(_.replaceTag(decl)) case Var(ident) => success(resolveToVar(ident, decl, bound, topBound)) case Match(_, arg, branches) => @@ -325,7 +328,7 @@ final class SourceConverter( val fnName: Expr[Declaration] = Expr.Global(PackageName.PredefName, Identifier.Name("concat_String"), s) - Expr.buildApp(fnName, listExpr.as(s: Declaration) :: Nil, s) + Expr.buildApp(fnName, listExpr.replaceTag(s: Declaration) :: Nil, s) } } case l@ListDecl(list) => diff --git a/core/src/main/scala/org/bykn/bosatsu/TypedExpr.scala b/core/src/main/scala/org/bykn/bosatsu/TypedExpr.scala index 97913ece0..bdb33df7d 100644 --- a/core/src/main/scala/org/bykn/bosatsu/TypedExpr.scala +++ b/core/src/main/scala/org/bykn/bosatsu/TypedExpr.scala @@ -103,6 +103,7 @@ sealed abstract class TypedExpr[+T] { self: Product => * they appear) */ lazy val freeVarsDup: List[Bindable] = + // nearly identical code to Expr.freeVarsDup, bugs should be fixed in both places this match { case Generic(_, expr) => expr.freeVarsDup @@ -114,18 +115,18 @@ sealed abstract class TypedExpr[+T] { self: Product => Nil case AnnotatedLambda(args, res, _) => val nameSet = args.toList.iterator.map(_._1).toSet - TypedExpr.filterNot(res.freeVarsDup)(nameSet) + ListUtil.filterNot(res.freeVarsDup)(nameSet) case App(fn, args, _, _) => fn.freeVarsDup ::: args.reduceMap(_.freeVarsDup) case Let(arg, argE, in, rec, _) => val argFree0 = argE.freeVarsDup val argFree = if (rec.isRecursive) { - TypedExpr.filterNot(argFree0)(_ === arg) + ListUtil.filterNot(argFree0)(_ === arg) } else argFree0 - argFree ::: (TypedExpr.filterNot(in.freeVarsDup)(_ === arg)) + argFree ::: (ListUtil.filterNot(in.freeVarsDup)(_ === arg)) case Literal(_, _, _) => Nil case Match(arg, branches, _) => @@ -136,7 +137,7 @@ sealed abstract class TypedExpr[+T] { self: Product => val newBinds = p.names.toSet val bfree = b.freeVarsDup if (newBinds.isEmpty) bfree - else TypedExpr.filterNot(bfree)(newBinds) + else ListUtil.filterNot(bfree)(newBinds) } // we can only take one branch, so count the max on each branch: val branchFreeMax = branchFrees @@ -163,17 +164,6 @@ sealed abstract class TypedExpr[+T] { self: Product => object TypedExpr { - // filter b from a pretty short lst but try to conserve lst if possible - private def filterNot[A](lst: List[A])(b: A => Boolean): List[A] = - lst match { - case Nil => lst - case h :: tail => - val t1 = filterNot(tail)(b) - if (b(h)) t1 - else if (t1 eq tail) lst - else (h :: t1) // we only allocate here - } - type Rho[A] = TypedExpr[A] // an expression with a Rho type (no top level forall) sealed abstract class Name[A] extends TypedExpr[A] with Product diff --git a/core/src/main/scala/org/bykn/bosatsu/rankn/Infer.scala b/core/src/main/scala/org/bykn/bosatsu/rankn/Infer.scala index e3c55b2e5..82c582a16 100644 --- a/core/src/main/scala/org/bykn/bosatsu/rankn/Infer.scala +++ b/core/src/main/scala/org/bykn/bosatsu/rankn/Infer.scala @@ -2,16 +2,18 @@ package org.bykn.bosatsu.rankn import cats.Monad import cats.arrow.FunctionK -import cats.data.NonEmptyList +import cats.data.{Chain, NonEmptyChain, NonEmptyList} -import cats.implicits._ +import cats.syntax.all._ import org.bykn.bosatsu.{ Expr, HasRegion, Identifier, Kind, + ListUtil, PackageName, + ParallelViaProduct, Pattern => GenPattern, Region, RecursionKind, @@ -68,6 +70,12 @@ object Infer { TailRecM(a, fn) } + implicit val inferParallel: cats.Parallel[Infer] = + new ParallelViaProduct[Infer] { + def monad = inferMonad + def parallelProduct[A, B](fa: Infer[A], fb: Infer[B]): Infer[(A, B)] = ParallelProduct(fa, fb) + } + /** * The first element of the tuple are the the bound type @@ -179,12 +187,12 @@ object Infer { sealed abstract class Error object Error { - + sealed abstract class Single extends Error /** * These are errors in the ability to type the code * Generally these cannot be caught by other phases */ - sealed abstract class TypeError extends Error + sealed abstract class TypeError extends Single case class NotUnifiable(left: Type, right: Type, leftRegion: Region, rightRegion: Region) extends TypeError case class KindNotUnifiable(leftK: Kind, leftT: Type, rightK: Kind, rightT: Type, leftRegion: Region, rightRegion: Region) extends TypeError @@ -203,7 +211,7 @@ object Infer { * These are errors that prevent typing due to unknown names, * They could be caught in a phase that collects all the naming errors */ - sealed abstract class NameError extends Error + sealed abstract class NameError extends Single // This could be a user error if we don't check scoping before typing case class VarNotInScope(varName: Name, vars: Map[Name, Type], region: Region) extends NameError @@ -217,7 +225,7 @@ object Infer { /** * These can only happen if the compiler has bugs at some point */ - sealed abstract class InternalError extends Error { + sealed abstract class InternalError extends Single { def message: String def region: Region } @@ -238,6 +246,22 @@ object Infer { def message = s"unknown var in $tpe: $mess at $region" // $COVERAGE-ON$ we don't test these messages, maybe they should be removed } + + // here is when we have more than one error + case class Combine(left: Error, right: Error) extends Error { + private def flatten(errs: NonEmptyList[Error], acc: Chain[Single]): NonEmptyChain[Single] = + errs match { + case NonEmptyList(s: Single, tail) => + tail match { + case Nil => NonEmptyChain.fromChainAppend(acc, s) + case h :: t => flatten(NonEmptyList(h, t), acc :+ s) + } + case NonEmptyList(Combine(a, b), tail) => + flatten(NonEmptyList(a, b :: tail), acc) + } + + lazy val flatten: NonEmptyChain[Single] = flatten(NonEmptyList(left, right :: Nil), Chain.empty) + } } @@ -263,6 +287,22 @@ object Infer { case Right(a) => fn(a).run(env) } } + + case class ParallelProduct[A, B](fa: Infer[A], fb: Infer[B]) extends Infer[(A, B)] { + def run(env: Env) = + fa.run(env).flatMap { + case Left(errA) => + fb.run(env).map { + case Right(_) => Left(errA) + case Left(errB) => Left(Error.Combine(errA, errB)) + } + case Right(a) => fb.run(env).map { + case Left(err) => Left(err) + case Right(b) => Right((a, b)) + } + } + } + case class Peek[A](fa: Infer[A]) extends Infer[Either[Error, A]] { def run(env: Env) = fa.run(env).resetOnLeft(Left[Either[Error, A], Nothing](_)).map { @@ -475,7 +515,7 @@ object Infer { // note due to contravariance in input, we reverse the order there for { // we know that they have the same length because we have already called unifyFn - coarg <- a2s.zip(a1s).traverse { case (a2, a1) => subsCheck(a2, a1, left, right) } + coarg <- a2s.zip(a1s).parTraverse { case (a2, a1) => subsCheck(a2, a1, left, right) } // r2 is already in weak-prenex form cores <- subsCheckRho(r1, r2, left, right) ks <- checkedKinds @@ -781,7 +821,7 @@ object Infer { argsRegion = args.reduceMap(region[Expr[A]](_)) argRes <- unifyFn(args.length, fnTRho, region(fn), argsRegion) (argT, resT) = argRes - typedArg <- args.zip(argT).traverse { case (arg, argT) => checkSigma(arg, argT) } + typedArg <- args.zip(argT).parTraverse { case (arg, argT) => checkSigma(arg, argT) } coerce <- instSigma(resT, expect, region(term)) } yield coerce(TypedExpr.App(typedFn, typedArg, resT, tag)) case Generic(tpes, in) => @@ -812,7 +852,7 @@ object Infer { // this comes from page 54 of the paper, but I can't seem to find examples // where this will fail if we reverse (as we had for a long time), which // indicates the testing coverage is incomplete - zipped.traverse_ { + zipped.parTraverse_ { case ((_, Some(tpe)), varT) => subsCheck(varT, tpe, region(term), rr) case ((_, None), _) => unit @@ -822,7 +862,7 @@ object Infer { } yield TypedExpr.AnnotatedLambda(namesVarsT, typedBody, tag) case infer@Expected.Inf(_) => for { - nameVarsT <- args.traverse { + nameVarsT <- args.parTraverse { case (n, Some(tpe)) => // TODO do we need to narrow or instantiate tpe? pure((n, tpe)) @@ -847,9 +887,9 @@ object Infer { // compilers/evaluation can possibly optimize non-recursive // cases differently val rhsBody = rhs match { - case Annotation(expr, tpe, tag) => + case Annotation(expr, tpe, _) => extendEnv(name, tpe) { - checkSigma(expr, tpe).product(typeCheckRho(body, expect)) + checkSigma(expr, tpe).parProduct(typeCheckRho(body, expect)) } case _ => newMetaType(Kind.Type) // the kind of a let value is a Type @@ -880,17 +920,30 @@ object Infer { // In this branch, we typecheck the rhs *without* name in the environment // so any recursion in this case won't typecheck, and shadowing rules are // in place - for { - typedRhs <- inferSigma(rhs) - varT = typedRhs.getType - typedBody <- extendEnv(name, varT)(typeCheckRho(body, expect)) - } yield TypedExpr.Let(name, typedRhs, typedBody, isRecursive, tag) + val rhsBody = rhs match { + case Annotation(expr, tpe, _) => + // check in parallel so we collect more errors + checkSigma(expr, tpe) + .parProduct( + extendEnv(name, tpe) { typeCheckRho(body, expect) } + ) + case _ => + // we don't know the type of rhs, so we have to infer then check the body + for { + typedRhs <- inferSigma(rhs) + typedBody <- extendEnv(name, typedRhs.getType)(typeCheckRho(body, expect)) + } yield (typedRhs, typedBody) + } + + rhsBody.map { case (rhs, body) => + TypedExpr.Let(name, rhs, body, isRecursive, tag) + } } case Annotation(term, tpe, tag) => - for { - typedTerm <- checkSigma(term, tpe) - coerce <- instSigma(tpe, expect, region(tag)) - } yield coerce(typedTerm) + (checkSigma(term, tpe), instSigma(tpe, expect, region(tag))) + .parMapN { (typedTerm, coerce) => + coerce(typedTerm) + } case Match(term, branches, tag) => // all of the branches must return the same type: @@ -912,14 +965,14 @@ object Infer { expect match { case Expected.Check((resT, _)) => for { - tbranches <- branches.traverse { case (p, r) => + tbranches <- branches.parTraverse { case (p, r) => // note, resT is in weak-prenex form, so this call is permitted checkBranch(p, check, r, resT) } } yield TypedExpr.Match(tsigma, tbranches, tag) case infer@Expected.Inf(_) => for { - tbranches <- branches.traverse { case (p, r) => + tbranches <- branches.parTraverse { case (p, r) => inferBranch(p, check, r) } (rho, regRho, resBranches) <- narrowBranches(tbranches) @@ -975,7 +1028,7 @@ object Infer { for { (minRes, (minPat, resTRho, minIdx)) <- minBy(withIdx.head, withIdx.tail)((a, b) => ltEq(a, b)) resRegion = region(minRes) - resBranches <- withIdx.traverse { case (te, (p, tpe, idx)) => + resBranches <- withIdx.parTraverse { case (te, (p, tpe, idx)) => if (idx != minIdx) { // unfortunately we have to check each branch again to get the correct coerce subsCheckRho2(resTRho, tpe, resRegion, region(te)) @@ -1094,7 +1147,7 @@ object Infer { for { tpeA <- tpeOfList listA = Type.TyApply(Type.ListType, tpeA) - inners <- items.traverse(checkItem(tpeA, listA, _)) + inners <- items.parTraverse(checkItem(tpeA, listA, _)) innerPat = inners.map(_._1) innerBinds = inners.flatMap(_._2) } yield (GenPattern.Annotation(GenPattern.ListPat(innerPat), listA), innerBinds) @@ -1115,13 +1168,13 @@ object Infer { // if the pattern arity does not match the arity of the constructor // but we don't want to error type-checking since we want to show // the maximimum number of errors to the user - envs <- args.zip(params).traverse { case (p, t) => checkPat(p, t, reg) } + envs <- args.zip(params).parTraverse { case (p, t) => checkPat(p, t, reg) } pats = envs.map(_._1) bindings = envs.map(_._2) } yield (GenPattern.PositionalStruct(nm, pats), bindings.flatten) case u@GenPattern.Union(h, t) => - (typeCheckPattern(h, sigma, reg), t.traverse(typeCheckPattern(_, sigma, reg))) - .mapN { case ((h, binds), neList) => + (typeCheckPattern(h, sigma, reg), t.parTraverse(typeCheckPattern(_, sigma, reg))) + .parMapN { case ((h, binds), neList) => val pat = GenPattern.Union(h, neList.map(_._1)) val allBinds = NonEmptyList(binds, (neList.map(_._2).toList)) identicalBinds(u, allBinds, reg).as((pat, binds)) @@ -1137,11 +1190,11 @@ object Infer { val rest = t.map(_.toSet) if (rest.forall(_ == bs)) { val bm = binds.map(_.toMap) - bs.toList.traverse_ { v => + bs.toList.parTraverse_ { v => val bmh = bm.head val bmt = bm.tail val tpe = bmh(v) - bmt.traverse_ { m2 => + bmt.parTraverse_ { m2 => val tpe2 = m2(v) unifyType(tpe, tpe2, reg, reg) } @@ -1397,22 +1450,47 @@ object Infer { def extendEnvList[A](bindings: List[(Bindable, Type)])(of: Infer[A]): Infer[A] = Infer.Impl.ExtendEnvs(bindings.map { case (n, t) => ((None, n), t) }, of) - private def extendEnvPack[A](pack: PackageName, name: Bindable, tpe: Type)(of: Infer[A]): Infer[A] = - Infer.Impl.ExtendEnvs(((Some(pack), name), tpe) :: Nil, of) + private def extendEnvListPack[A](pack: PackageName, nameTpe: List[(Bindable, Type)])(of: Infer[A]): Infer[A] = + Infer.Impl.ExtendEnvs(nameTpe.map { case (name, tpe) => ((Some(pack), name), tpe) }, of) /** * Packages are generally just lists of lets, this allows you to infer * the scheme for each in the context of the list */ - def typeCheckLets[A: HasRegion](pack: PackageName, ls: List[(Bindable, RecursionKind, Expr[A])]): Infer[List[(Bindable, RecursionKind, TypedExpr[A])]] = - ls match { - case Nil => Infer.pure(Nil) - case (name, rec, expr) :: tail => - for { - te <- if (rec.isRecursive) recursiveTypeCheck(name, expr) else typeCheck(expr) - rest <- extendEnvPack(pack, name, te.getType)(typeCheckLets(pack, tail)) - } yield (name, rec, te) :: rest - } + def typeCheckLets[A: HasRegion](pack: PackageName, ls: List[(Bindable, RecursionKind, Expr[A])]): Infer[List[(Bindable, RecursionKind, TypedExpr[A])]] = { + // Group together lets that don't include each other to get more type errors + // if we can + type G = NonEmptyChain[(Bindable, RecursionKind, Expr[A])] + + def run(groups: List[G]): Infer[List[(Bindable, RecursionKind, TypedExpr[A])]] = + groups match { + case Nil => Infer.pure(Nil) + case group :: tail => + group.parTraverse { case (name, rec, expr) => + (if (rec.isRecursive) recursiveTypeCheck(name, expr) else typeCheck(expr)) + .map { te => (name, rec, te) } + } + .flatMap { groupChain => + val glist = groupChain.toList + extendEnvListPack(pack, glist.map { case (b, _, te) => (b, te.getType) }) { + run(tail) + } + .map(glist ::: _) + } + } + + val groups: List[G] = + ListUtil.greedyGroup(ls)({ item => NonEmptyChain.one(item) }) { case (bs, item @ (_, _, expr)) => + val dependsOnGroup = + expr.globals.iterator.exists { + case Expr.Global(p, n1, _) => (p == pack) && bs.exists(_._1 == n1) + } + if (dependsOnGroup) None // we can't run in parallel + else Some(bs :+ item) + } + + run(groups) + } /** * This is useful to testing purposes. diff --git a/core/src/test/scala/org/bykn/bosatsu/EvaluationTest.scala b/core/src/test/scala/org/bykn/bosatsu/EvaluationTest.scala index a08e9156c..1eadac35c 100644 --- a/core/src/test/scala/org/bykn/bosatsu/EvaluationTest.scala +++ b/core/src/test/scala/org/bykn/bosatsu/EvaluationTest.scala @@ -2881,11 +2881,12 @@ def quick_sort0(cmp, left, right): bigs = quick_sort0(cmp, tail) [*smalls, *bigs] """)) { case kie@PackageError.TypeErrorIn(_, _) => - assert(kie.message(Map.empty, Colorize.None) == - """in file: , package QS -type error: expected type Bosatsu/Predef::Fn3[(?43, ?41) -> Bosatsu/Predef::Comparison] to be the same as type Bosatsu/Predef::Fn2 + assert(kie.message(Map.empty, Colorize.None) == """in file: , package QS +type error: expected type Bosatsu/Predef::Fn3[(?43, ?41) -> Bosatsu/Predef::Comparison] +Region(403,414) +to be the same as type Bosatsu/Predef::Fn2 hint: the first type is a function with 3 arguments and the second is a function with 2 arguments. -Region(396,450)""") +Region(415,424)""") () } @@ -3083,4 +3084,43 @@ v = loop(b) main = v """), "A", VInt(1)) } + + test("we get error messages from multiple type errors top level") { + val testCode = """ +package ErrorCheck + +x: Int = "1" +y: String = 1 + +""" + evalFail(List(testCode)) { case kie@PackageError.TypeErrorIn(_, _) => + val message = kie.message(Map.empty, Colorize.None) + assert(message.contains("Region(30,33)")) + assert(testCode.substring(30, 33) == "\"1\"") + assert(message.contains("Region(46,47)")) + assert(testCode.substring(46, 47) == "1") + () + } + } + + test("we get error messages from multiple type errors top nested") { + val testCode = """ +package ErrorCheck + +z = ( + x: Int = "1" + y: String = 1 + (x, y) +) + +""" + evalFail(List(testCode)) { case kie@PackageError.TypeErrorIn(_, _) => + val message = kie.message(Map.empty, Colorize.None) + assert(message.contains("Region(38,41)")) + assert(testCode.substring(38, 41) == "\"1\"") + assert(message.contains("Region(56,57)")) + assert(testCode.substring(56, 57) == "1") + () + } + } } diff --git a/core/src/test/scala/org/bykn/bosatsu/ListUtilTest.scala b/core/src/test/scala/org/bykn/bosatsu/ListUtilTest.scala new file mode 100644 index 000000000..d9091f177 --- /dev/null +++ b/core/src/test/scala/org/bykn/bosatsu/ListUtilTest.scala @@ -0,0 +1,82 @@ +package org.bykn.bosatsu + +import cats.data.NonEmptyList +import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks.{ forAll, PropertyCheckConfiguration } +import org.scalatest.funsuite.AnyFunSuite +import org.scalacheck.{Arbitrary, Gen} + +class ListUtilTest extends AnyFunSuite { + + implicit val generatorDrivenConfig: PropertyCheckConfiguration = + PropertyCheckConfiguration(minSuccessful = 5000) + + def genNEL[A](ga: Gen[A]): Gen[NonEmptyList[A]] = + Gen.sized { sz => + if (sz <= 1) ga.map(NonEmptyList.one) + else Gen.zip(ga, Gen.listOfN(sz - 1, ga)).map { case (h, t) => NonEmptyList(h, t) } + } + + implicit def arbNEL[A: Arbitrary]: Arbitrary[NonEmptyList[A]] = + Arbitrary(genNEL(Arbitrary.arbitrary[A])) + + test("unit group has 1 item") { + forAll { (nel: NonEmptyList[Int]) => + val unit = ListUtil.greedyGroup(nel)(_ => ())((_, _) => Some(())) + assert(unit == NonEmptyList.one(())) + } + } + + test("groups satisfy edge property") { + forAll { (nel: NonEmptyList[Int], accept: (Int, Int) => Boolean) => + val groups = ListUtil.greedyGroup(nel)(a => Set(a))((s, i) => if (s.forall(accept(_, i))) Some(s + i) else None) + groups.toList.foreach { g => + val items = g.toList.zipWithIndex + for { + (i1, idx1) <- items + (i2, idx2) <- items + } assert((idx1 == idx2) || accept(i1, i2) || accept(i2, i1)) + } + } + } + + test("there are as most as many groups as inputs") { + forAll { (nel: NonEmptyList[Int], one: Int => Int, accept: (Int, Int) => Option[Int]) => + val groups = ListUtil.greedyGroup(nel)(one)(accept) + assert(groups.length <= nel.length) + } + } + + test("if we always accept there is one group") { + forAll { (nel: NonEmptyList[Int], one: Int => Int) => + val groups = ListUtil.greedyGroup(nel)(one)((x, y) => Some(x + y)) + assert(groups.length == 1) + } + } + + test("if we never accept there are as many groups as came in") { + forAll { (nel: NonEmptyList[Int], one: Int => Int) => + val groups = ListUtil.greedyGroup(nel)(one)((_, _) => None) + assert(groups.length == nel.length) + } + } + + test("groups direct property") { + forAll { (nel: NonEmptyList[Int], accept: (List[Int], Int) => Boolean) => + val groups = ListUtil.greedyGroup(nel)(a => a :: Nil)((s, i) => if (accept(s, i)) Some(i :: s) else None) + groups.toList.foreach { g => + def check(g: List[Int]): Unit = + g match { + case Nil => fail("expected at least one item") + case _ :: Nil => + // this can always happen + () + case head :: tail => + assert(accept(tail, head)) + check(tail) + } + + check(g) + } + } + } +} \ No newline at end of file