Allow coercing unboxed with payload to primitive when applicable #6441
Conversation
zth force-pushed the coerce-untagged-variants-with-payload-to-primitive branch from 3c8cd67 to e737fab Compare 
| Could you write in words the conditions under which this coercion is OK? I think the conditions need to imply that every value represented in the variant is string. Generally speaking this seems fine, if only a bit niche. |
| @cristianoc I updated the OP with a list of conditions. Maybe I should also add them as an explicit comment in the code. |
jscomp/ml/variant_coercion.ml Outdated
| let check_paths_same p1 p2 target_path = | ||
| Path.same p1 target_path && Path.same p2 target_path | ||
| | ||
| let can_coerce_variant ~(path : Path.t) ~unboxed |
There was a problem hiding this comment.
I think the function's name should indicate that this function is for coercing a variant to a type.
And the for_all commented or refactored so the high-level algorithm is clear:
- every case of the variant has the same runtime representation as the target type.
This allows to treat string and float at the same time as both conform to that algorithm.
There was a problem hiding this comment.
Yes, good catch! I'll clean these things up a bit.
There was a problem hiding this comment.
@cristianoc please look at 95be068 to see if the refactor is clear enough.
| @unboxed type strings = String(string) | First | Second | Third | ||
| let a: strings = String("hello") | ||
| let aa: strings = First | ||
| let b: string = (a :> string) |
There was a problem hiding this comment.
These 2 lines test the same thing. Does not hurt, but it's not helpful either.
Coercion works on types and does not depend on the specific instance.
There was a problem hiding this comment.
That said, it's kind of nice as documentation of the intentions.
There was a problem hiding this comment.
Yeah that was my intention, to illustrate that it works for both the catch all case and the regular cases. But yes, it's not actually useful as a test, just illustrating for documentation purposes.
2 participants
Allows:
Coercing from a variant to a string is OK when the variant only has cases that are represented as strings at runtime. This happens when all constructors in the variant conforms to the following:
@asattribute. The default runtime representation of a variant constructor is a string since v11@asattribute and that attribute has a string payload@unboxedand the constructor has a single payload that's astringAll of the cases above means that the variant will be represented as a string at runtime, and that means coercion to a string is safe.
Allowing going the "other way" is explored here: #6443