-
Notifications
You must be signed in to change notification settings - Fork 54
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
Type-safe and static higher-order functions #452
base: main
Are you sure you want to change the base?
Conversation
CodSpeed Performance ReportMerging #452 will not alter performanceComparing Summary
|
No one asked for it but here's something you can do (type-safely) 😆
|
Thanks for these changes! I was wondering if you would be down to add some more tests to show whats now possible or at least what errors are caught at type checking time now. I had an idea looking at this that it might be possible to remove the additional |
@saulshanabrook @Alex-Fischman Right, this PR actually does two separate but related things:
As for if This is not to say we don't have any hacks in this PR. Notably, because of the way we implement this, to get the type of |
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.
@yihozhang and I talked on a call about this PR. We discussed that it would be possible to avoid the addition of the new #
sort and constructor if we know when calling the get
method of TypeConstraint
if we are in the checking or the resolution phase. In the checking phase, we can be exact in our type checking of functions, because we know the name of the function based on the term. In the resolution, we can't be exact, because the term is no longer a literal, but that's fine.
We also discussed possible ways the function interface could evolve to make it accessible to users, i.e. what kind of public interface to have. I was hesitant to get closer to something user friendly without just going all the way and allowing functions to be passed as symbols to other functions.
If you still think this it he best approach for the typechecker and for consts long term @yihozhang I defer to you and would support this PR.
I think a few more tests would be helpful to nail down the expected behavior as well, as I said above, so we can be precise about what this change brings.
The current implementation of higher-order functions (#348) does only a best-effort type checking: It works well for functions, but does little for primitives. This PR fixes this by statically check the type even for primitives. Moreover, we no longer rely on
get_sort_from_value
to get the dynamic type of a value forunstable-fn
, which will unblock #448.Moreover, the previous type checking for higher order functions is bad because the type of a primitive is derived by looking at the argument. This PR changes that by introducing a concept similar to
constexpr
:(# "a")
is a value of typeConst<"a">
and is the only inhabitant of this type. This way, the type of(unstable-fn (# "*"))
can be determined by pure type-level inspection. This allows us to guaranteeResolvedCall::from_resolution
can infer which primitive to take with only the types of the arguments. Imagine otherwise,from_resolution
cannot determine the type of(unstable-fn "+" 1)
if all it sees it that the arguments have type(String, i64)
(instead of(Const<"+">, 1)
)More specifically, this PR does the following:
SpecializedPrimitive
instead of aPrimitive
so we can pass down the type information when applying a primitive (this gets rid of the need forget_sort_from_value
ConstSort
and its constructor#
.Constraint
because we need to ensureTypeInfo
is still available during the delayed constraint generation.