More boolean simplifications in generated code.#7149
Conversation
| let res = | ||
| match (e1.expression_desc, e2.expression_desc) with | ||
| | Bool false, _ -> Some false_ | ||
| | _, Bool false -> Some false_ |
There was a problem hiding this comment.
Does this need a check that e1 doesn't have side effects?
There was a problem hiding this comment.
Thanks! It does.
There was a problem hiding this comment.
@yallop nice find! See any more issues of that kind? 👀
There was a problem hiding this comment.
Yes, there seem to be a few. To pick some at random:
- in
eq_null_undefined_booleanthematch (a.expression_desc, b.expression_desc) with | ( (Null | Undefined _), (Bool _ | Number _ | Typeof _ | Fun _ | Array _ | Caml_block _) ) ->
TypeoforArrayorCaml_blockcould have side effects, so can't always be safely dropped - in
int_compif| Ceq, Optional_block _, Undefined _ | Ceq, Undefined _, Optional_block _ -> false_
Optional_blockcan have side effects then it shouldn't be dropped - in
simplify_orthe left operand shouldn't be dropped if it has side effects.| _, Bool true -> Some true_
This is just a selection; there are some similar cases in those functions and others. I don't have all the development tools set up, so can't easily send a PR, unfortunately.
There was a problem hiding this comment.
Yes, there seem to be a few. To pick some at random:
the
TypeoforArrayorCaml_blockcould have side effects, so can't always be safely droppedin
int_comp| Ceq, Optional_block _, Undefined _ | Ceq, Undefined _, Optional_block _ -> false_if
Optional_blockcan have side effects then it shouldn't be droppedin
simplify_or| _, Bool true -> Some true_the left operand shouldn't be dropped if it has side effects.
This is just a selection; there are some similar cases in those functions and others. I don't have all the development tools set up, so can't easily send a PR, unfortunately.
3 participants
This handles all the boolean simplifications in #7142.
What's left from that issue is conditionals
b ? x : y, that can be done separately.