Multiple Return Values #272
Conversation
Fixes the compilation errors introduced by cherry-picked aa8b5cb.
This makes optimizations easier and automatically whole-program.
| @@ -567,7 +588,7 @@ object Named { | |||
| } | |||
|
|
|||
| extension [T <: Definitions](t: T & Definition) { | |||
| def symbol(using C: Context): ResolvedDefinitions[T] = C.symbolOf(t).asInstanceOf | |||
| def symbol(using C: Context): List[ResolvedDefinitions[T]] = C.symbolsOf(t).map(asInstanceOf) // TODO MRV 26: ok? | |||
There was a problem hiding this comment.
@denhie Here this should be C.symbolsOf(t).asInstanceOf without the map.
| @@ -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.symbolOf(u)), config) | |||
| outputCode(DeclPrinter(context.symbolsOf(u)), config) | |||
There was a problem hiding this comment.
Why is this change necessary? Can you please help me understand? Is the reason that a definition now introduces multiple symbols?
There was a problem hiding this comment.
My intuition tells me that there are invariants which are not represented here. This will lead to a lot of code changes that might not be necessary.
There was a problem hiding this comment.
Yes, a definition now has multiple symbols.
| } | ||
| output.emitln(s"\nImported Functions\n==================") | ||
| cu.terms.toList.sortBy { case (n, _) => n }.foreach { | ||
| case (name, syms) => | ||
| syms.collect { | ||
| case sym if !sym.isSynthetic => | ||
| outputCode(DeclPrinter(sym), config) | ||
| outputCode(DeclPrinter(List(sym)), config) |
There was a problem hiding this comment.
I don't like that DeclPrinter now takes a list... This feels wrong.
| @@ -215,21 +215,22 @@ class Repl(driver: Driver) extends REPL[Tree, EffektConfig, EffektError] { | |||
| case d: Def => | |||
| val extendedDefs = module + d | |||
| val decl = extendedDefs.make(UnitLit()) | |||
| val output = config.output() | |||
|
|
||
| runFrontend(source, decl, config) { cu => | ||
| module = extendedDefs | ||
|
|
||
| // try to find the symbol for the def to print the type | ||
| (context.symbolOption(d.id) match { | ||
| d.ids.foreach(id => (context.symbolOption(id) match { |
There was a problem hiding this comment.
I believe this is not the correct implementation. It should print
x:Int, y:String, z:Bool
not
x: Int
y: String
z: Bool
or what do you think?
There was a problem hiding this comment.
No, this is the correct behavior in this case. It can still be refactored, though.
| import effekt.source.{ FunDef, Hole, ModuleDecl, Tree } | ||
| import effekt.util.{ PlainMessaging, getOrElseAborting } | ||
| import effekt.source.{FunDef, Hole, ModuleDecl, Tree} | ||
| import effekt.util.{PlainMessaging, getOrElseAborting} |
There was a problem hiding this comment.
Please try to keep this changes out of the PR or change them upstream, so that the diff is smaller :)
| @@ -231,10 +234,11 @@ trait LSPServer extends kiama.util.Server[Tree, EffektConfig, EffektError] with | |||
| } | |||
| } yield res | |||
|
|
|||
| def needsUpdate(annotated: (ValueType, Effects), inferred: (ValueType, Effects))(using Context): Boolean = { | |||
| def needsUpdate(annotated: (List[ValueType], Effects), inferred: (List[ValueType], Effects))(using Context): Boolean = { | |||
| val (tpe1, effs1) = annotated | |||
There was a problem hiding this comment.
This should be called tpes1 since they are now multiple.
| val (tpe1, effs1) = annotated | ||
| val (tpe2, effs2) = inferred | ||
| tpe1 != tpe2 || effs1 != effs2 | ||
|
|
||
| tpe1.size != tpe2.size || tpe1.zip(tpe2).forall { case (t1, t2) => t1 != t2 } || effs1 != effs2 |
There was a problem hiding this comment.
please don't write huge boolean expressions like this.
| tpe1.size != tpe2.size || tpe1.zip(tpe2).forall { case (t1, t2) => t1 != t2 } || effs1 != effs2 | |
| val differentArity = tpes1.size != tpes2.size | |
| val differentEffects = effs1 != effs2 | |
| def differentTypes = (tpes1 zip tpes2).forall { case (tpe1, tpe2) => tpe1 != tpe2 } | |
| differentArity || differentEffects || differentTypes |
…es-invariants Feature/multiple return values invariants
|
|
||
| //</editor-fold> | ||
|
|
||
| //<editor-fold desc="statements and definitions"> | ||
|
|
||
| def checkStmt(stmt: Stmt, expected: Option[ValueType])(using Context, Captures): Result[ValueType] = | ||
| def checkStmt(stmt: Stmt, expected: Option[List[ValueType]])(using Context, Captures): Result[List[ValueType]] = |
There was a problem hiding this comment.
I think this should be a list of option since each individual component might be known or not.
jiribenes
force-pushed
the
master
branch
3 times, most recently
from
ee9d209
to
58c8510
Compare
No description provided.