Skip to content
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

Union type as ADT argument #37

Closed
danabr opened this issue Sep 2, 2016 · 4 comments
Closed

Union type as ADT argument #37

danabr opened this issue Sep 2, 2016 · 4 comments

Comments

@danabr
Copy link
Contributor

danabr commented Sep 2, 2016

The following test case fails:

union_type_as_adt_arg_test() ->
    Code = "module adt\n\n"
           "type union = int | atom\n\n"
           "type t = Union union\n\n"
           "make () = Union 1",
    ?assertMatch({ok, _}, module_typ_and_parse(Code)).

Error:

{error, {cannot_unify,adt,7,  {adt,"union",[],[]}, t_int}}

It seems like the knowledge about what members are in the union got lost along the way.

This is the line reporting the error: https://github.com/j14159/mlfe/blob/master/src/mlfe_typer.erl#L557

@j14159
Copy link
Collaborator

j14159 commented Sep 2, 2016

I wonder if this is similar to #34 - wouldn't surprise me if we could simplify the handling of ADTs and their unification while solving both, it's a bit heavy-handed at the moment. I'm going to try to find some time to sort out #25 over the next week (I suspect it's just a case of furthering generalization) and then tackle this (unless it's already solved by then ;) )

@danabr
Copy link
Contributor Author

danabr commented Sep 2, 2016

I have a prototypical fix here: master...danabr:fix/complex-types

I say "prototypical", because the code needs cleanup, and I'm not sure I should do type lookup like this. It also happens to fix the example in #34, but there are more complex ways to use aliases than that example which I'm sure will break even with this simple fix.

@danabr
Copy link
Contributor Author

danabr commented Sep 9, 2016

I've updated the complex-types branch with something that works even for type aliases and type unions.

However, there is something weird going on. unify_adt expects two cells as its first arguments, but there are occasions when it gets something that is not a cell. You can see this by removing the "is_pid(C2) test and running the tests.

@j14159
Copy link
Collaborator

j14159 commented Nov 10, 2016

Closing due to PR #49

@j14159 j14159 closed this as completed Nov 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants