-
Notifications
You must be signed in to change notification settings - Fork 40
Instrumentation into Block IR #355
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
base: hkmc2
Are you sure you want to change the base?
Conversation
add Block.mls
merge upstream hkmc2
…ation during diff testing
…ring Add Block.mls into DiffMaker and prepare the function for lowering Use older Import syntax for now
after shape information is removed, the extra typechecking is no longer helpful
use no ident after match, like in Lowering.scala
the definitions should utilize CachedHash before getting merged
since the shape is not inferred, each branch can use the same x to compute its branch
renaming mlsript/staging to mlscript/block-staging avoids DiffTestRunner excluding the test files using the filter intended to exlude ucs/staging
this removes the check in L218 because evaluation of Scope is now deferred:wq
| def init(using State): Ctx = Ctx.empty.copy(env = Map( | ||
| "globalThis" -> globalThisSymbol, | ||
| "Term" -> termSymbol, | ||
| "Block" -> blockSymbol, |
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.
Again, I don't think this should be here and neither should "Term". This is the context meant for the user. For internal uses, one should use an internal symbol (like runtimeSymbol), not a plain name. Cc @NeilKleistGao.
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.
I think the block and the shape module would not be used explicitly by users, but users do need to write code like Term.codegen. if we remove it from the context, then users should import the term module explicitly.
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.
If some functions like this should be accessible by users, they should be in the Predef. The Predef could either re-export Term (but it's a bit weird, since most things in it are not meant to be used directly by users) or define some submodule, such as module meta with { fun codegen... }.
Call implementation will be after the algorithms for instrumenting functions are completed.
allocating
optionNmeThis changed some diff tests (
OpenWildcard.mls,Imports.mls,ImportMLs.mls) that imported Option themselves.If I allocate
optionNmeafter checking ifstageCodeis enabled, it fails to link to the file.Is there some way to avoid changing the diff tests, or is this fine?
implementing I-Inst
The current shape for Class
C(n:s)includes the field names (n), but the field names are not available inInstantiation.The field names are used in
sel. If the class is staged using I-Cls, then we can use the stagedClsLikeDefnto retrieve the symbol names, but when do we do when it isn't staged?