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.

Sign up for GitHub

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

Jump to bottom

Kill all assert false in printer #1658

Open
chenglou opened this issue Nov 22, 2017 · 0 comments
Open

Kill all assert false in printer #1658

chenglou opened this issue Nov 22, 2017 · 0 comments
Labels
Printer things that have to do with turning an AST into Reason code

Comments

@chenglou
Copy link
Member

There are way too many of them, and they give really bad error messages to the user, e.g. #1611.

assert false should really only be used when the condition can't be achieved. Everything else should be Invalid_argument. We look super lazy with our error handling otherwise
chenglou added a commit to chenglou/reason that referenced this issue Nov 22, 2017
@chenglou
Fixes reasonml#1611
31cf16d 
Part of reasonml#1658
@chenglou chenglou mentioned this issue Nov 22, 2017
Merged
chenglou added a commit to chenglou/reason that referenced this issue Nov 22, 2017
@chenglou
Fixes reasonml#1611
b5fefea 
Part of reasonml#1658
chenglou added a commit that referenced this issue Nov 22, 2017
@chenglou
Fixes #1611 (#1659)
9c41333 
Part of #1658
@jaredly jaredly added the Printer things that have to do with turning an AST into Reason code label Jun 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Printer things that have to do with turning an AST into Reason code
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jaredly @chenglou