-
Notifications
You must be signed in to change notification settings - Fork 453
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
PoC: IR for runtime representation #6993
base: master
Are you sure you want to change the base?
Conversation
jscomp/ml/runtime_representation.ml
Outdated
value = Known (type_expr_to_possible_values label.ld_type env); | ||
}) | ||
|
||
let rec type_expr_to_possible_values (type_expr : Types.type_expr) (env : Env.t) |
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.
to_runtime_representation
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.
This is great. Even just documenting what the possible runtime representations produced by the compiler are is super valuable.
Not sure if you want to cover all cases at this stage.
If not, it's perfectly fine to have a partial function that returns an optional runtime repr. So it returns None when the case is not covered yet.
@@ -23,6 +23,7 @@ | |||
* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *) | |||
|
|||
let () = Ast_untagged_variants.extract_concrete_typedecl := Ctype.extract_concrete_typedecl | |||
let () = Runtime_representation.extract_concrete_typedecl := Ctype.extract_concrete_typedecl |
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.
Any idea why this file is called "polyfill"?
The only reason I can think about for its existence is: hack to avoid recursion amongst modules. If that's not the role, then perhaps this file should not exist.
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.
No idea. IIRC this is a "random" file I put the assignment in just to get it there.
jscomp/ml/runtime_representation.ml
Outdated
optional: bool; | ||
} | ||
and runtime_js_value = | ||
| String of {value: string value} |
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.
StringLiteral etc
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 opted to let value
control whether it's a literal or not, but it's probably clearer to make actual literals. I'll revise.
| Array of {element_type: runtime_js_value value} | ||
| Object of { | ||
properties: object_property list; | ||
can_have_unknown_properties: bool; |
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.
not sure what this boolean does
the runtime representation is never exhaustive, you can always have more stuff
no code depends on the absence of properties right?
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.
open and closed objects have the same runtime representation right?
} | ||
| Dict of {value_type: runtime_js_value list} | ||
| Promise of {resolved_type: runtime_js_value value} | ||
| Any |
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'd remove this and use a partial function instead
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.
Not sure I follow, could you expand?
properties: object_property list; | ||
can_have_unknown_properties: bool; | ||
} | ||
| Dict of {value_type: runtime_js_value list} |
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.
why is the runtime representation of a dict different from the one of an object?
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.
Because a dict has no known properties but a fixed field value type, whereas an object has (at least partially) known properties with potential different values. So it's a matter of trying to make it easy to not accidentally coerce between the two.
062f0d2
to
4cdf8c6
Compare
(match tr1 with | ||
| [(t1, _); (_, t2)] -> | ||
let a_runtime_representation = Runtime_representation.to_runtime_representation t2 env in | ||
let b_runtime_representation = Runtime_representation.to_runtime_representation t1 env in | ||
a_runtime_representation |> List.iter( | ||
fun a_value -> | ||
b_runtime_representation |> List.iter( | ||
fun b_value -> | ||
if Runtime_representation.runtime_values_match a_value b_value then ( | ||
() | ||
) | ||
else Runtime_representation.explain_why_not_matching a_value b_value | ||
|> List.iter(fun s -> fprintf ppf "@ %s" s) | ||
)) | ||
| _ -> () | ||
); |
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.
@cristianoc the code in here is very difficult to understand, so this isn't correct, but I think it might be along the lines at least of what we'd want if we want to enhance the subtyping messages with information about the runtime representations.
A missing piece is additional information and hints about how something is configured (@ as, @ tag, etc). Haven't figured out yet how to tackle that.
cbc0bdf
to
44c3fef
Compare
This PoC explores an IR for how types are represented at runtime in JS. This could be used to: