-
Notifications
You must be signed in to change notification settings - Fork 118
feat: Invariance limitations improvements #5464
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: master
Are you sure you want to change the base?
Conversation
(* Restrict to [isolated] types only, at least for now *) | ||
| t, Any when isolated t -> | ||
(* When the solution is between [t] and [Any], choose [t] when there are no other choices except [Any] *) | ||
| t, Any when has_no_supertypes 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.
Can you do the symmetric thing for Non, t when has_no_subtypes 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.
We could, yes. I just did not see enough examples in practice that would benefit from that. So I wanted to limit this heuristic for now
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.
But I guess it makes sense to make it symmetric. the has_no_subtypes
is already ignoring the bottom type Non
so it should be consistent.
I'll add some examples covering that. I guess something returning a function that takes a mutable data structure 😄
src/mo_frontend/bi_match.ml
Outdated
(Cons.name c) | ||
display_constraint (lb, c, ub) | ||
display_rel (lb,"=/=",ub))) | ||
if !error_msg = "" then |
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.
Why only report the first error? To avoid information overload?
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.
To retain the original behavior. We could gather all of them and report all. I just needed to finish the building of env
so that I have the substitution.
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.
OK, I've refactored and now we gather all 'under-constrained' and merge into a single error message, see the test output here
@crusso @christoph-dfinity Hm, perhaps now with dot-notation we might actually prefer 'type instantiations' over 'return type annotations' because it is easier to write them with a dot-chain of functions like list
.map<X,Y>(func x = ...)
.filter(func y = ...)
... Though we could probably easily allow partial type instantiations like list
.map<_,Y>(func x = ...) Because only the 2nd type parameter of map on a mutable list is essential to provide |
T
andAny
, pickT
when it has no proper supertypes (exceptAny
).Previously we needed the
T
to be isolated (no proper subtypes nor supertypes).This change allows
Int
and?T
as well! (AssumingT
has no supertypes).See the tests for more examples.
Likewise, when choosing between
Non
andT
, pickT
when it has no proper subtypes (exceptNon
).lambdas-core.mo
test cases e.g. herebecause implicit instantiation of type parameter R is under-constrained with
is a bit cryptic, maybe we can find a better alternative that explains that there is no best solution and the compiler won't make an arbitrary choice...For example (error message improvement)
Before
After
In the future we could integrate it into the vscode extension to provide a 'quick fix' that would insert this type annotation.
Limitations: The return type might be too long, we would not use type aliases, maybe we should avoid the suggestion when the type is longer than X characters.