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

Fix crash when a record union contains any() #447

Merged
merged 3 commits into from
Aug 31, 2022

Conversation

erszcz
Copy link
Collaborator

@erszcz erszcz commented Aug 30, 2022

Fixes #445.

Copy link
Collaborator

@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this will solve the issue. I have some suggestion but you can choose which solution you prefer.

There is some redundant code here. What if expect_record_type could return fields_tys in more cases, e.g. in the case of no union (a single record type) and in the case of any as well, could that get rid of some duplication? (optional)

src/typechecker.erl Outdated Show resolved Hide resolved
@erszcz
Copy link
Collaborator Author

erszcz commented Aug 31, 2022

There is some redundant code here. What if expect_record_type could return fields_tys in more cases, e.g. in the case of no union (a single record type) and in the case of any as well, could that get rid of some duplication? (optional)

Good point, we can simplify it all a bit. I'll make this change before we merge this.

@erszcz
Copy link
Collaborator Author

erszcz commented Aug 31, 2022

There is some redundant code here. What if expect_record_type could return fields_tys in more cases [...]

Ok, I did give it a try, but it's no use. Whatever we gain in do_type_check_expr_in we lose in add_type_pat, which is another call site of expect_record_type, but unions are handled completely differently there so reusing their code path doesn't make sense.

Why not add a clause in type_check_record_union_in?

But I did that, so there's a tiny bit less repetition :)

@erszcz erszcz merged commit eb8fc74 into josefs:master Aug 31, 2022
@erszcz erszcz deleted the record-union-with-any branch August 31, 2022 14:23
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

Successfully merging this pull request may close these issues.

function_clause crash in typechecker:get_unassigned_fields/2
2 participants