-
Notifications
You must be signed in to change notification settings - Fork 8
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
Unify the two DSLs for RISE into a single one #76
Conversation
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.
How does one decide between using ::
or !:
, and what issues arise when using the wrong one?
I think that we need to discuss equality more so that we have one or if necessary multiple clear definitions. This can be resolved in another PR.
def untypedTopLevel[T <: Expr](d: ToBeTyped[T]): ToBeTyped[TopLevel] = | ||
toBeTyped(topLevel(toExpr(d))) | ||
|
||
def eraseTypeFromExpr[T <: Expr](e: T): T = |
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.
maybe this should be private, or contain the word unsafe
?
val constraints = mutable.ArrayBuffer[Constraint]() | ||
val (typed_e, ftvSubs) = constrainTypes(e, constraints, mutable.Map()) | ||
val solution = unfreeze(ftvSubs, Constraint.solve(constraints.toSeq, Seq())(explDep)) | ||
traversal.DepthFirstLocalResult(typed_e, Visitor(solution)) |
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.
will this DSL inference perform something like printTypeAndTypeHoles
?
printTypesAndTypeHoles(r) |
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.
does this DSL enforce unique names for type inference and DPIA?
val unique_e = uniqueNames.enforce(e) |
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 do not know the relationship to printTypeAndTypeHoles
(neither do I understand what is done there).
We are not enforcing unique names in the type inference process.
Substitution is no longer relying on unique names.
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.
printTypeAndTypeHoles
will handle debug primitives:
- print the types of the
printType
primitives - print the types of the
typeHole
primitives, throwing an error if a type hole is found (because a type hole makes the expression incomplete)
I think that we should add this to the new DSL type inference (while avoiding excessive prints), or alternatively move this code to fromRise
on the transition to DPIA.
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.
About unique names, I think that we still require unique names on the DPIA side, so we might want to move the enforce to fromRise
if the type inference does not require it anymore.
val bnfExpr = normalize(ev).apply(betaReduction)(expr).get | ||
// TODO: consider if we can remove this going forward | ||
val exprWithUniqueNames = uniqueNames.enforce(expr) | ||
val bnfExpr = normalize(ev).apply(betaReduction)(exprWithUniqueNames).get |
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.
beta reduction can break unique names ((fun x. x x) (fun y. y) --> (fun y. y) (fun y. y)
), maybe we should enforce unique names after normalization.
This should be revisited in a separate PR.
…aints for one long running test.
I have delayed the implementation of the enforcement of unique names and suggest we accept this PR without it and I will look at this in a separate PR. I hope that's fine with you @Bastacyclop. |
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.
That's reasonable, please open an issue for the unique names question and the printing.
This PR removes the DSL that helped to construct untyped expressions for which the type has to be inferred explicitly with a call to
infer
.The only remaining DSL uses the
ToBeTyped
class to construct expressions for which the types are inferred automatically on conversion to theExpr
class.This PR also introduces an import distinction in our interpretation of type annotations:
TypeAnnotation
s (writtene :: t
) add a constraint that has to be resolved in type inferenceTypeAssertaion
s (writtene !: t
) insist that the type inferred fore
must be made compatible witht
, that is, the constraint solving only goes one way, type variables int
will not be instantiated during constraint solving, only the once ine.t
.The
TypeAsseration
s are used during rewriting, when we know already what the expected type of the replacement must be and we assert that the inferred type of the expression must be made compatible with the asserted type.By switching the DSLs this PR also rolls out syntax changes introduced in #70.