feat(const-prop): propagate type casts #10144
Conversation
|
PR checklist:
If you're unsure about any of this, please see: |
| @@ -0,0 +1,10 @@ | |||
| Constant propagation: Constants that are being type-casted will now | |||
There was a problem hiding this comment.
Who asked for it? Didn't they provide any other example?
| @@ -0,0 +1,10 @@ | |||
| Constant propagation: Constants that are being type-casted will now | |||
| propagate properly, e.g. in the following example: | |||
There was a problem hiding this comment.
With that branch name, and without "Closes SAF-NNN" or similar in the PR comment, I think Linear won't link the PR with the ticket.
| @@ -230,6 +230,7 @@ let rec eval (env : G.svalue Dataflow_var_env.t) (exp : IL.exp) : G.svalue = | |||
| match exp.e with | |||
| | Fetch lval -> eval_lval env lval | |||
| | Literal li -> G.Lit li | |||
| | Cast (_, e) -> eval env e | |||
There was a problem hiding this comment.
Out of caution, I'd prefer this is restricted to TS for now. I think TS is kind of a special case. Even in Java you would need to do something when casting e.g. (double)42. That would mean passing the language to eval, it's a bit more work, but this can be useful later too.
There was a problem hiding this comment.
If you rebase, I just merged another PR that adds a field lang : Lang.t to the env here.
What:
This PR makes it so that when we do constant propagation on a literal that is being casted, we propagate the literal.
Why:
People asked for this.
How:
I made it so evaluation of something being casted just evaluated the inner contents.
Alternatives:
This behavior may be intentional -- in a C-like language, for instance, where you can do something like
(int) false, which to my knowledge should actually be0, and not a literalfalse, this could be wrong. But, I think that this is probably a low-probability occurrence. If bothered by this, we can create a whitelist of languages in which it is safe to do this unwrapping of the cast.Test plan:
Automated tests.