Skip to content
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

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Conversation

zth
Copy link
Collaborator

@zth zth commented Aug 30, 2024

This PoC explores an IR for how types are represented at runtime in JS. This could be used to:

  • Power type coercion.
  • Improve error messages for coercion.

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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to_runtime_representation

Copy link
Collaborator

@cristianoc cristianoc left a 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
Copy link
Collaborator

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.

Copy link
Collaborator Author

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.

optional: bool;
}
and runtime_js_value =
| String of {value: string value}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StringLiteral etc

Copy link
Collaborator Author

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;
Copy link
Collaborator

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?

Copy link
Collaborator

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
Copy link
Collaborator

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

Copy link
Collaborator Author

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}
Copy link
Collaborator

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?

Copy link
Collaborator Author

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.

@zth zth force-pushed the runtime-representation-ir-poc branch from 062f0d2 to 4cdf8c6 Compare September 11, 2024 18:34
Comment on lines +1524 to +1541
(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)
))
| _ -> ()
);
Copy link
Collaborator Author

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.

@zth zth force-pushed the runtime-representation-ir-poc branch from cbc0bdf to 44c3fef Compare September 15, 2024 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants