Skip to content

Commit

Permalink
Try to maintain invariant that there is only one symbol per definition
Browse files Browse the repository at this point in the history
  • Loading branch information
phischu committed Nov 15, 2023
1 parent 2ccc64d commit 99177f7
Show file tree
Hide file tree
Showing 12 changed files with 87 additions and 117 deletions.
14 changes: 7 additions & 7 deletions effekt/jvm/src/main/scala/effekt/Repl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ class Repl(driver: Driver) extends REPL[Tree, EffektConfig, EffektError] {
runFrontend(StringSource(""), module.make(UnitLit()), config) { cu =>
module.definitions.foreach {
case u: Def =>
outputCode(DeclPrinter(context.symbolsOf(u)), config)
outputCode(DeclPrinter(List(context.symbolOf(u.id))), config)

This comment has been minimized.

Copy link
@b-studios

b-studios Nov 16, 2023

Collaborator

Why not just call u.symbol?

}
}
}
Expand Down Expand Up @@ -221,16 +221,16 @@ class Repl(driver: Driver) extends REPL[Tree, EffektConfig, EffektError] {
module = extendedDefs

// try to find the symbol for the def to print the type
d.ids.foreach(id => (context.symbolOption(id) match {
context.symbolOption(d.id) match {
case Some(v: ValueSymbol) =>
Some(context.valueTypeOf(v))
Some(v, context.valueTypeOf(v))
case Some(b: BlockSymbol) =>
Some(context.blockTypeOf(b))
Some(b, context.blockTypeOf(b))
case t =>
None

This comment has been minimized.

Copy link
@b-studios

b-studios Nov 16, 2023

Collaborator

will this now work for valdefs? We need to check? I am most certain it does not.

}) map { tpe =>
} map { case (id, tpe) =>
outputCode(pp"${id}: ${tpe}", config)
})
}
}

case _ => ()
Expand Down Expand Up @@ -297,7 +297,7 @@ class Repl(driver: Driver) extends REPL[Tree, EffektConfig, EffektError] {
) {
def +(d: Def) = {
// drop all equally named definitions for now.
val otherDefs = definitions.filterNot { other => d.ids.map{_.name}.forall(other.ids.map{_.name}.contains) }
val otherDefs = definitions.filterNot { other => d.id == other.id }

This comment has been minimized.

Copy link
@b-studios

b-studios Nov 16, 2023

Collaborator

it needs to be .name not id since we need to compare the names itself (but please test this).

def +(d: Def) = {
// drop all equally named definitions for now.
val otherDefs = definitions.filterNot { other => other.id.name == d.id.name }
copy(definitions = otherDefs :+ d)
}

copy(definitions = otherDefs :+ d)
}
def +(i: Import) = copy(imports = imports.filterNot { _.path == i.path } :+ i)
Expand Down
2 changes: 1 addition & 1 deletion effekt/jvm/src/main/scala/effekt/Server.scala
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ trait LSPServer extends kiama.util.Server[Tree, EffektConfig, EffektError] with
* This way, we can use custom kinds like `refactor.closehole` that can be mapped to keys.
*/
def inferEffectsAction(fun: FunDef)(using C: Context): Option[TreeAction] =
val symbol = fun.symbol.head
val symbol = fun.symbol
for {
// the inferred type
(tpe, eff) <- C.inferredTypeAndEffectOption(fun)
Expand Down
2 changes: 1 addition & 1 deletion effekt/shared/src/main/scala/effekt/Intelligence.scala
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ trait Intelligence {
case d: Operation => C.definitionTreeOption(d.interface)
case a: Anon => Some(a.decl)
case s: SelfParam => s.tree match {
case d: source.Def => Some(d.ids.head) // TODO MRV 14
case d: source.Def => Some(d.id)
case _ => Some(s.tree)
}
case u => C.definitionTreeOption(u)
Expand Down
16 changes: 8 additions & 8 deletions effekt/shared/src/main/scala/effekt/Namer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ object Namer extends Phase[Parsed, NameResolved] {
*/
def preresolve(d: Def)(using Context): Unit = Context.focusing(d) {

case d @ source.ValDef(id, annot, binding) =>
case d @ source.ValDef(binders, binding) =>
()

case d @ source.VarDef(id, annot, region, binding) =>
Expand Down Expand Up @@ -225,11 +225,11 @@ object Namer extends Phase[Parsed, NameResolved] {
Context.define(id, p)
Context.bind(p.capture)

case d @ source.ValDef(ids, annots, binding) =>
val tpes = annots.map(a => a.map(resolve)) // TODO MRV: remove Option[]
case d @ source.ValDef(binders, binding) =>
val tpes = binders.map(a => a.tpe.map(resolve)) // TODO MRV: remove Option[]
resolveGeneric(binding)
(ids zip tpes) foreach { (id, tpe) =>
Context.define(id, ValBinder(Context.nameFor(id), tpe, d))
(binders zip tpes) foreach { case (source.ValueParam(id, _), tpe) =>
Context.define(id, ValBinder(Name.local(id), tpe, d))

This comment has been minimized.

Copy link
@b-studios

b-studios Nov 16, 2023

Collaborator

Why iterate over binders twice?

}

case d @ source.VarDef(id, annot, region, binding) =>
Expand All @@ -253,7 +253,7 @@ object Namer extends Phase[Parsed, NameResolved] {

// FunDef and EffDef have already been resolved as part of the module declaration
case f @ source.FunDef(id, tparams, vparams, bparams, ret, body) =>
val sym = f.symbol.head
val sym = f.symbol
Context scoped {
sym.tparams.foreach { p => Context.bind(p) }
Context.bindValues(sym.vparams)
Expand Down Expand Up @@ -292,7 +292,7 @@ object Namer extends Phase[Parsed, NameResolved] {

// The type itself has already been resolved, now resolve constructors
case d @ source.DataDef(id, tparams, ctors) =>
val data = d.symbol.head
val data = d.symbol
data.constructors = ctors map {
case source.Constructor(id, ps) =>
val name = Context.freshNameFor(id)
Expand All @@ -304,7 +304,7 @@ object Namer extends Phase[Parsed, NameResolved] {

// The record has been resolved as part of the preresolution step
case d @ source.RecordDef(id, tparams, fs) =>
val record = d.symbol.head
val record = d.symbol
val name = Context.freshNameFor(id)
val constructor = Constructor(name, record.tparams, null, record)
// we define the constructor on a copy to avoid confusion with symbols
Expand Down
9 changes: 3 additions & 6 deletions effekt/shared/src/main/scala/effekt/Parser.scala
Original file line number Diff line number Diff line change
Expand Up @@ -298,14 +298,11 @@ class EffektParsers(positions: Positions) extends EffektLexers(positions) {

lazy val valDefs: P[Def] =
`val` ~> someSep(valDef, `,`) ~ (`=` ~/> stmt) ^^ {
case defs ~ binding =>
val ids = defs.map(_._1)
val tpes = defs.map(_._2)
ValDef(ids, tpes, binding)
case defs ~ binding => ValDef(defs.asInstanceOf, binding)
}

lazy val valDef: P[(IdDef, Option[ValueType])] =
idDef ~ (`:` ~/> valueType).? ^^ { case id ~ tpe => (id, tpe)}
lazy val valDef: P[Param] =

This comment has been minimized.

Copy link
@b-studios

b-studios Nov 16, 2023

Collaborator

Maybe eventually we should rename ValueParam since this can be confusing

idDef ~ (`:` ~/> valueType).? ^^ { case id ~ tpe => ValueParam(id, tpe)}

This comment has been minimized.

Copy link
@b-studios

b-studios Nov 16, 2023

Collaborator

Please let us refactor this to be closer to the original master... I would'nt call the outer one valDefs but still valDef.

The goal in Parser is to always just call .apply on the constructor. Most of the parser follows this:

lazy val valDef: P[Def] =
`val` ~> idDef ~ (`:` ~/> valueType).? ~ (`=` ~/> stmt) ^^ ValDef.apply


lazy val varDef: P[Def] =
`var` ~/> idDef ~ (`:` ~/> valueType).? ~ (`in` ~/> idRef).? ~ (`=` ~/> stmt) ^^ {
Expand Down
62 changes: 31 additions & 31 deletions effekt/shared/src/main/scala/effekt/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ object Typer extends Phase[NameResolved, Typechecked] {
checkOverloadedMethodCall(c, receiver, id, targs map { _.resolve }, vargs, bargs, expected)

case tree @ source.Region(name, body) =>
val selfRegion = tree.symbol.head
val selfRegion = tree.symbol
Context.bind(selfRegion)

val inferredCapture = Context.freshCaptVar(CaptUnificationVar.RegionRegion(tree))
Expand All @@ -207,7 +207,7 @@ object Typer extends Phase[NameResolved, Typechecked] {
// (1) extract all handled effects and capabilities
val providedCapabilities: List[symbols.BlockParam] = handlers map Context.withFocus { h =>
val effect: InterfaceType = h.effect.resolve
val capability = h.capability.map { _.symbol.head }.getOrElse { Context.freshCapabilityFor(effect) }
val capability = h.capability.map { _.symbol }.getOrElse { Context.freshCapabilityFor(effect) }
Context.bind(capability, capability.tpe, CaptureSet(capability.capture))
capability
}
Expand Down Expand Up @@ -248,7 +248,7 @@ object Typer extends Phase[NameResolved, Typechecked] {

// Helper definitions:
// - capabilities that are bound explicitly by the user
val explicitCapabilities = handlers.flatMap { _.capability.map(_.symbol.head) }.toSet
val explicitCapabilities = handlers.flatMap { _.capability.map(_.symbol) }.toSet
// - all effects that are handled
val handled = ConcreteEffects(handlerFor.map(_._1))
// - capabilities grouped by effect
Expand Down Expand Up @@ -375,7 +375,7 @@ object Typer extends Phase[NameResolved, Typechecked] {

(params zip vps).foreach {
case (param, decl) =>
val sym = param.symbol.head
val sym = param.symbol
val annotType = sym.tpe
annotType.foreach { t => matchDeclared(t, decl, param) }
Context.bind(sym, annotType.getOrElse(decl))
Expand Down Expand Up @@ -490,7 +490,7 @@ object Typer extends Phase[NameResolved, Typechecked] {

def checkPattern(sc: ValueType, pattern: MatchPattern)(using Context, Captures): Map[Symbol, ValueType] = Context.focusing(pattern) {
case source.IgnorePattern() => Map.empty
case p @ source.AnyPattern(id) => Map(p.symbol.head -> sc)
case p @ source.AnyPattern(id) => Map(p.symbol -> sc)
case p @ source.LiteralPattern(lit) => Context.abort("Matching literals is not supported at the moment.")
// lit.checkAgainst(sc)
// Map.empty
Expand Down Expand Up @@ -564,28 +564,28 @@ object Typer extends Phase[NameResolved, Typechecked] {
// this is necessary for mutually recursive definitions
def precheckDef(d: Def)(using Context): Unit = Context.focusing(d) {
case d @ source.FunDef(id, tps, vps, bps, ret, body) =>
val fun = d.symbol.head
val fun = d.symbol

// (1) make up a fresh capture unification variable and annotate on function symbol
val cap = Context.freshCaptVar(CaptUnificationVar.FunctionRegion(d))
Context.bind(fun, cap)

// (2) Store the annotated type (important for (mutually) recursive and out-of-order definitions)
fun.annotatedType.foreach { tpe => Context.bind(d.symbol.head, tpe) }
fun.annotatedType.foreach { tpe => Context.bind(d.symbol, tpe) }

case d @ source.ExternDef(cap, id, tps, vps, bps, tpe, body) =>
val fun = d.symbol.head
val fun = d.symbol

Context.bind(fun, fun.toType, fun.capture)
if (fun.effects.canonical.nonEmpty) {
Context.abort("Unhandled control effects on extern defs not allowed")
}

case d @ source.ExternResource(id, tpe) =>
Context.bind(d.symbol.head)
Context.bind(d.symbol)

case d @ source.InterfaceDef(id, tparams, ops, isEffect) =>
d.symbol.head.operations.foreach { op =>
d.symbol.operations.foreach { op =>
if (op.effects.toList contains op.appliedInterface) {
Context.error("Bidirectional effects that mention the same effect recursively are not (yet) supported.")
}
Expand All @@ -597,7 +597,7 @@ object Typer extends Phase[NameResolved, Typechecked] {

case source.DataDef(id, tparams, ctors) =>
ctors.foreach { c =>
val constructor = c.symbol.head
val constructor = c.symbol
Context.bind(constructor, constructor.toType, CaptureSet())
constructor.fields.foreach { field =>
val tpe = field.toType
Expand All @@ -606,23 +606,23 @@ object Typer extends Phase[NameResolved, Typechecked] {
}

case d @ source.RecordDef(id, tparams, fields) =>
val constructor = d.symbol.head.constructor
val constructor = d.symbol.constructor
Context.bind(constructor, constructor.toType, CaptureSet())
constructor.fields.foreach { field =>
val tpe = field.toType
wellformed(tpe)
Context.bind(field, tpe, CaptureSet())
}

case d: source.TypeDef => wellformed(d.symbol.head.tpe)
case d: source.EffectDef => wellformed(d.symbol.head.effs)
case d: source.TypeDef => wellformed(d.symbol.tpe)
case d: source.EffectDef => wellformed(d.symbol.effs)
case _ => ()
}

def synthDef(d: Def)(using Context, Captures): Result[Unit] = Context.at(d) {
d match {
case d @ source.FunDef(id, tps, vps, bps, ret, body) =>
val sym = d.symbol.head
val sym = d.symbol
// was assigned by precheck
val functionCapture = Context.lookupCapture(sym)
val selfRegion = Context.getSelfRegion(d)
Expand Down Expand Up @@ -701,7 +701,7 @@ object Typer extends Phase[NameResolved, Typechecked] {

Result((), unhandledEffects)

case d @ source.ValDef(id, annot, binding) =>
case d @ source.ValDef(binders, binding) =>
val tpeList = d.symbol.map(_.tpe)
val Result(t, effBinding) = if (tpeList.forall(_.isDefined)) {
val Result(_, effBinding) = binding checkAgainst tpeList.flatten
Expand All @@ -716,7 +716,7 @@ object Typer extends Phase[NameResolved, Typechecked] {
Result((), effBinding)

case d @ source.VarDef(id, annot, reg, binding) =>
val sym = d.symbol.head // TODO MRV: List?
val sym = d.symbol // TODO MRV: List?

// we use the current region as an approximation for the state
val stCapt = reg map Context.symbolOf map {
Expand Down Expand Up @@ -745,13 +745,13 @@ object Typer extends Phase[NameResolved, Typechecked] {

given inferredCapture: Captures = Context.freshCaptVar(CaptUnificationVar.BlockRegion(d))

val Result(t, effBinding) = checkExprAsBlock(binding, d.symbol.head.tpe)
Context.bind(d.symbol.head, t, inferredCapture)
val Result(t, effBinding) = checkExprAsBlock(binding, d.symbol.tpe)
Context.bind(d.symbol, t, inferredCapture)
Result((), effBinding)

case d @ source.ExternDef(pure, id, tps, vps, bps, tpe, body) =>
d.symbol.head.vparams foreach Context.bind
d.symbol.head.bparams foreach Context.bind
d.symbol.vparams foreach Context.bind
d.symbol.bparams foreach Context.bind
Result((), Pure)

// all other definitions have already been prechecked
Expand Down Expand Up @@ -796,23 +796,23 @@ object Typer extends Phase[NameResolved, Typechecked] {
val valueTypes = (vparams zip vps) map {
case (param, expected) =>
val adjusted = typeSubst substitute expected
val tpe = param.symbol.head.tpe.map { got =>
val tpe = param.symbol.tpe.map { got =>
matchDeclared(got, adjusted, param);
got
} getOrElse { adjusted }
// bind types to check body
Context.bind(param.symbol.head, tpe)
Context.bind(param.symbol, tpe)
tpe
}

val blockTypes = (bparams zip bps) map {
case (param, expTpe) =>
val adjusted = typeSubst substitute expTpe
val sym = param.symbol.head
val sym = param.symbol
val got = sym.tpe
matchDeclared(got, adjusted, param)
// bind types to check body
Context.bind(param.symbol.head, got, CaptureSet(sym.capture))
Context.bind(param.symbol, got, CaptureSet(sym.capture))
got
}

Expand All @@ -821,7 +821,7 @@ object Typer extends Phase[NameResolved, Typechecked] {
val capabilities = effects.canonical.map { tpe => Context.freshCapabilityFor(tpe) }

// (5) Substitute capture params
val captParams = (bparams.map(_.symbol.head) ++ capabilities).map { p => p.capture }
val captParams = (bparams.map(_.symbol) ++ capabilities).map { p => p.capture }
val captSubst = Substitutions.captures(cps, captParams.map { p => CaptureSet(p) })

// (6) Substitute both types and captures into expected return type
Expand Down Expand Up @@ -849,15 +849,15 @@ object Typer extends Phase[NameResolved, Typechecked] {
case arg @ source.BlockLiteral(tparams, vparams, bparams, body) => Context in {
val tps = tparams.map { p => p.symbol.asTypeParam }
val vps = vparams.map { p =>
val param = p.symbol.head
val tpe = p.symbol.head.tpe.getOrElse {
val param = p.symbol
val tpe = p.symbol.tpe.getOrElse {
Context.abort("Expected type needs to be known for function arguments at the moment.")
}
Context.bind(param, tpe)
tpe
}
val bps = bparams.map { p =>
val param = p.symbol.head
val param = p.symbol
val tpe = param.tpe
Context.bind(param, tpe)
tpe
Expand All @@ -877,14 +877,14 @@ object Typer extends Phase[NameResolved, Typechecked] {
val capabilities = effs.canonical.map { caps.apply }
Context.bindCapabilities(arg, capabilities)

val cps = (bparams.map(_.symbol.head) ++ capabilities).map(_.capture)
val cps = (bparams.map(_.symbol) ++ capabilities).map(_.capture)

val funType = FunctionType(tps, cps, vps, bps, tpe, effs.toEffects)


// Like with functions, bound parameters and capabilities are not closed over
usingCaptureWithout(inferredCapture) {
(bparams.map(_.symbol.head) ++ capabilities).map(_.capture) ++ List(selfRegion)
(bparams.map(_.symbol) ++ capabilities).map(_.capture) ++ List(selfRegion)
}

Result(funType, Pure)
Expand Down
9 changes: 0 additions & 9 deletions effekt/shared/src/main/scala/effekt/context/Annotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -504,15 +504,6 @@ trait AnnotationsDB { self: Context =>
sym
}

/**
* Searching the symbol for a definition
*
* These lookups should not fail (except there is a bug in the compiler)
*/

def symbolsOf(tree: source.Definition): List[Symbol] =
tree.ids.map(symbolOf)

This comment has been minimized.

Copy link
@b-studios

b-studios Nov 16, 2023

Collaborator

We had this in the original master

* Searching the symbol for a definition
*
* These lookups should not fail (except there is a bug in the compiler)
*/
def symbolOf(tree: source.Definition): Symbol =
symbolOf(tree.id)

Maybe check where it was used to see whether we forgot something.

/**
* Searching the definition for a symbol
*/
Expand Down
Loading

0 comments on commit 99177f7

Please sign in to comment.