-
Notifications
You must be signed in to change notification settings - Fork 19
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
Add comparable typevar constraints #390
base: master
Are you sure you want to change the base?
Conversation
@@ -2016,7 +2048,8 @@ next_count() -> | |||
|
|||
ets_tables() -> | |||
[options, type_vars, constraints, freshen_tvars, type_errors, | |||
defined_contracts, warnings, function_calls, all_functions]. | |||
defined_contracts, warnings, function_calls, all_functions, | |||
ord_constraint_types]. |
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.
Where are eq_constraint_types?
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.
There is no such table as I didn't find a use for it, if all id
s are comparable by equality, what should go in that table?
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 don't get why there is a need for a table for ord
but not for eq
. Would you explain why one is needed and the other is not?
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.
Since all types of the form {id, _, Id}
are compared by eq
, there is no need to store all of them in a table, but not all types of the above form are compared by ord
so I only need to store that.
So checking if {id, _, "int"}
is comparable by ord
will only succeed because "int"
is in the table ord_constraint_types
, but checking if any other type of the form {id, _, Id}
is comparable by eq
will always succeed.
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.
Check this commit that made address
comparable by ord
fc28759.
@@ -0,0 +1,169 @@ | |||
contract A = entrypoint init() = () | |||
|
|||
main contract C = |
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.
Please write tests for type inference:
function f(x) = x == x // PASS
function g() = f(g) // FAIL
@@ -29,9 +29,11 @@ namespace List = | |||
[] => abort("drop_last_unsafe: list empty") | |||
|
|||
|
|||
function contains(e : 'a, l : list('a)) = switch(l) |
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.
Please add versions of insert_by
and sort
(and possibly others) that instead of taking a comparison lambda, rely on the constraints
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.
Remember that you actually can reuse the existing definitions by just applying them with a lambda
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.
Update fold in aeso_syntax_utils
Fixes: #229