-
Notifications
You must be signed in to change notification settings - Fork 14
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
Drop intrinsic lists in core #194
Conversation
def addToScope(definition: Definition, body: Stmt): Stmt = body match { | ||
case Scope(definitions, body) => Scope(definition :: definitions, body) | ||
case other => Scope(List(definition), other) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use this helper function (for now) to not introduce too many Scope
s in the translation.
|
||
case Definition.Let(id, tpe, binding) => | ||
js.Const(nameDef(id), toJS(binding)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The refactoring bares first fruit.
Def(id, tpe, transform(block)(using env, Context), rest) | ||
case (core.Definition.Let(id, tpe, binding), rest) => | ||
Let(id, tpe, transform(binding)(using env, Context), rest) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we go back to the horrible intrinsic lists, to refactor one bit at a time. :)
@phischu @marzipankaiser there is probably a better way to implement the translation to Machine. For now all LLVM tests go through. Maybe you could revise it? I propose the following:
WDYT? |
Please do not merge.
Using the intrinsic list encoding on core is really annoying on the module level, since a special return value usally unit has to be made up to terminate the list. This is a technical debt that we carry around for quite some time now,
As a step forward, here we
core.Definition
core.Scope
to locally usecore.Definition
Scope(Scope...))
core.Defintion.Mutual
to separate out mutually recursive block definitionsList[Definition]
instead ofStmt
lifted
.Completely unrelated to this PR, 7ff2ce2 also desugars while into a recursive function to further simplify core and lifted.