Skip to content

Commit

Permalink
Improve the error when an effect is not fully known (#678)
Browse files Browse the repository at this point in the history
Addresses #663 (even though it doesn't remove the root cause).

We improve the message by 

1. mentioning the…… unknown types and 
2. providing a hint what to do.

<img width="1479" alt="image"
src="https://github.com/user-attachments/assets/78dec94d-888b-429b-af3a-a0d6c9c86364">
  • Loading branch information
b-studios authored Nov 7, 2024
1 parent 36abeb7 commit 000a768
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 25 deletions.
8 changes: 3 additions & 5 deletions effekt/shared/src/main/scala/effekt/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -786,9 +786,7 @@ object Typer extends Phase[NameResolved, Typechecked] {
}
// we bind the function type outside of the unification scope to solve for variables.
val substituted = Context.unification(funTpe)
if (!isConcreteBlockType(substituted)) {
Context.abort(pretty"Cannot fully infer type for ${id}: ${substituted}")
}
assertConcreteFunction(id, substituted)
Context.bind(sym, substituted)

Result((), unhandledEffects)
Expand Down Expand Up @@ -1510,7 +1508,7 @@ trait TyperOps extends ContextOps { self: Context =>

private [typer] def bindCapabilities[R](binder: source.Tree, caps: List[symbols.BlockParam]): Unit =
val capabilities = caps map { cap =>
assertConcrete(cap.tpe.getOrElse { INTERNAL_ERROR("Capability type needs to be know.") }.asInterfaceType)
assertConcreteEffect(cap.tpe.getOrElse { INTERNAL_ERROR("Capability type needs to be know.") }.asInterfaceType)
positions.dupPos(binder, cap)
cap
}
Expand All @@ -1528,7 +1526,7 @@ trait TyperOps extends ContextOps { self: Context =>
* Has the potential side-effect of creating a fresh capability. Also see [[BindAll.capabilityFor()]]
*/
private [typer] def capabilityFor(tpe: InterfaceType): symbols.BlockParam =
assertConcrete(tpe)
assertConcreteEffect(tpe)
val cap = capabilityScope.capabilityFor(tpe)
annotations.update(Annotations.Captures, cap, CaptureSet(cap.capture))
cap
Expand Down
52 changes: 32 additions & 20 deletions effekt/shared/src/main/scala/effekt/typer/ConcreteEffects.scala
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ object ConcreteEffects {
* [[Typer.asConcrete]] should be used instead, since it performs substitution and dealiasing.
*/
def apply(eff: List[InterfaceType])(using Context): ConcreteEffects =
eff foreach assertConcrete
eff foreach assertConcreteEffect
fromList(eff)

def apply(effs: Effects)(using Context): ConcreteEffects = apply(effs.toList)
Expand Down Expand Up @@ -73,33 +73,45 @@ implicit def asConcrete(effs: Effects)(using C: Context): ConcreteEffects =
*
* TODO Question: should we ALWAYS require effects to be concrete, also when compared with [[TypeUnifier]]?
*/
private[typer] def assertConcrete(effs: Effects)(using C: Context): Unit =
if (!isConcreteEffects(effs)) C.abort(pretty"Effects need to be fully known: ${effs}")
private[typer] def assertConcreteEffects(effs: Effects)(using C: Context): Unit =
effs.effects.foreach(assertConcreteEffect)

private[typer] def assertConcreteEffect(eff: InterfaceType)(using C: Context): Unit =
unknowns(eff) match {
case us if us.nonEmpty =>
C.abort(pretty"Effects need to be fully known, but effect ${eff}'s type parameter(s) ${us.mkString(", ")} could not be inferred.\n\nMaybe try annotating them?")
case _ => ()
}

private[typer] def assertConcrete(eff: InterfaceType)(using C: Context): Unit =
if (!isConcreteInterfaceType(eff)) {
C.abort(pretty"Effects need to be fully known: ${eff}")
private[typer] def assertConcreteFunction(id: source.Id, tpe: BlockType)(using C: Context): Unit =
unknowns(tpe) match {
case us if us.nonEmpty =>
C.abort(pretty"Cannot fully infer type for ${id}: ${tpe}")
case _ => ()
}

private def isConcreteValueType(tpe: ValueType): Boolean = tpe match {
case ValueTypeRef(x) => isConcreteValueType(x)
case ValueTypeApp(tpe, args) => args.forall(isConcreteValueType)
case BoxedType(tpe, capture) => isConcreteBlockType(tpe) && isConcreteCaptureSet(capture)
private type Unknown = CaptUnificationVar | UnificationVar

private def unknowns(tpe: ValueType): Set[Unknown] = tpe match {
case ValueTypeRef(x) => unknowns(x)
case ValueTypeApp(tpe, args) => args.flatMap(unknowns).toSet
case BoxedType(tpe, capture) => unknowns(tpe) ++ unknowns(capture)
}

private def isConcreteValueType(tpe: TypeVar): Boolean = tpe match {
case x: UnificationVar => false
case x: TypeVar => true
private def unknowns(tpe: TypeVar): Set[Unknown] = tpe match {
case x: UnificationVar => Set(x)
case x: TypeVar => Set.empty
}

private def isConcreteBlockType(tpe: BlockType): Boolean = tpe match {
private def unknowns(tpe: BlockType): Set[Unknown] = tpe match {
case FunctionType(tparams, cparams, vparams, bparams, result, effects) =>
vparams.forall(isConcreteValueType) && bparams.forall(isConcreteBlockType) && isConcreteValueType(result) && isConcreteEffects(effects)
case InterfaceType(tpe, args) => args.forall(isConcreteValueType)
vparams.flatMap(unknowns).toSet ++ bparams.flatMap(unknowns).toSet ++ unknowns(result) ++ unknowns(effects)
case InterfaceType(tpe, args) => args.flatMap(unknowns).toSet
}
private def isConcreteCaptureSet(capt: Captures): Boolean = capt.isInstanceOf[CaptureSet]

private def isConcreteInterfaceType(eff: InterfaceType): Boolean = eff match {
case InterfaceType(tpe, args) => args.forall(isConcreteValueType)
private def unknowns(capt: Captures): Set[Unknown] = capt match {
case x @ CaptUnificationVar(role) => Set(x)
case CaptureSet(captures) => Set.empty
}
private def isConcreteEffects(effs: Effects): Boolean = effs.toList.forall(isConcreteInterfaceType)

private def unknowns(effs: Effects): Set[Unknown] = effs.toList.flatMap(unknowns).toSet

0 comments on commit 000a768

Please sign in to comment.