-
Notifications
You must be signed in to change notification settings - Fork 35
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
Skip functions using maybe expr, don't crash on maybe ... else #571
Conversation
Since too complex guards lead to skipping a function altogether now, this condition is redundant. Dropping it allows exhaustiveness checking to benefit from the simple refinements from guards that are supported.
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.
Hey Radek! I agree that not crashing on maybe
/maybe … else
is a good idea. 🙂
I also agree that it is a bit better to skip these function silently (as in this PR) rather than with a warning (as is the current behaviour), as one may strive for zero Gradualizer warnings (for example in a CI) and the maybe warning would complicate that. However, maybe even a bit better solution would be to skip just the maybe
expressions (instead of skipping whole functions) and assign the expression the type any()
. I am not sure how much work would that be (if only a small change in type_check_expr(_in)
would be enough or whether other places would need to be updated as well) though…
src/typechecker.erl
Outdated
@@ -2232,8 +2232,10 @@ do_type_check_expr(Env, {'try', _, Block, CaseCs, CatchCs, AfterBlock}) -> | |||
|
|||
%% Maybe - value-based error handling expression | |||
%% See https://www.erlang.org/eeps/eep-0049 | |||
do_type_check_expr(_Env, {'maybe', Anno, [{maybe_match, _, _LHS, _RHS}]} = MaybeExpr) -> | |||
erlang:throw({unsupported_expression, Anno, MaybeExpr}). | |||
do_type_check_expr(_Env, {'maybe', _, _}) -> |
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 think that equivalent code should be also added to do_type_check_expr_in
.
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.
Good point, added. In the case of do_type_check_expr_in
, though, it's not possible to assume any()
, since the operation is checking (as in checking vs inference in bidirectional typing), so it would certainly lead to false positives. I think it's better to skip such functions altogether for now.
This handles
maybe ... else
instead of crashing. It also skips processing functions which usemaybe
similar to how #570 skips functions that use too complex guards.