Skip to content

Commit

Permalink
Add %ffi as variation of %raw with arity checking. (#6251)
Browse files Browse the repository at this point in the history
* Begin exposing %raw arity

Only print to stderr for now?

See #6213

* Use %ffi attribute and handle arity zero

Move to using `%ffi` extension to avoid breaking changes, and begin checking arity zero.

For a JS function of arity 0, check that the ReScript type is `unit => _`.

* Check all arities and make error message gpt3.5-proof.

Extend arity check to all arities.

Iterate on the error message so gpt3.5 can figure out a correct fix when given the error message with no context.

* One more example.

* cleanup

* format

* comment

* Update CHANGELOG.md
  • Loading branch information
cristianoc authored May 23, 2023
1 parent 86c905a commit c72928e
Show file tree
Hide file tree
Showing 9 changed files with 87 additions and 11 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
# 11.0.0-beta.2 (Unreleased)

#### :rocket: New Feature

- Introduced a new `%ffi` extension that provides a more robust mechanism for JavaScript function interoperation by considering function arity in type constraints. This enhancement improves safety when dealing with JavaScript functions by enforcing type constraints based on the arity of the function. [PR #6251](https://github.com/rescript-lang/rescript-compiler/pull/6251)

# 11.0.0-beta.1

#### :rocket: Main New Feature
Expand Down
1 change: 1 addition & 0 deletions jscomp/frontend/ast_exp_extension.ml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ open Ast_helper
let handle_extension e (self : Bs_ast_mapper.mapper)
(({txt; loc}, payload) : Parsetree.extension) =
match txt with
| "ffi" -> Ast_exp_handle_external.handle_ffi ~loc ~payload
| "bs.raw" | "raw" ->
Ast_exp_handle_external.handle_raw ~kind:Raw_exp loc payload
| "bs.re" | "re" ->
Expand Down
54 changes: 50 additions & 4 deletions jscomp/frontend/ast_exp_handle_external.ml
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ let handle_debugger loc (payload : Ast_payload.t) =
Location.raise_errorf ~loc "%%debugger extension doesn't accept arguments"

let handle_raw ~kind loc payload =
let is_function = ref false in
let is_function = ref None in
match Ast_payload.raw_as_string_exp_exn ~kind ~is_function payload with
| None -> (
match kind with
Expand All @@ -93,11 +93,57 @@ let handle_raw ~kind loc payload =
~pval_type:(Typ.arrow Nolabel (Typ.any ()) (Typ.any ()))
[exp];
pexp_attributes =
(if !is_function then
Ast_attributes.internal_expansive :: exp.pexp_attributes
else exp.pexp_attributes);
(match !is_function with
| None -> exp.pexp_attributes
| Some _ -> Ast_attributes.internal_expansive :: exp.pexp_attributes);
}

let handle_ffi ~loc ~payload =
let is_function = ref None in
let err () =
Location.raise_errorf ~loc
"%%ffi extension can only be applied to a string containing a JavaScript \
function such as \"(x) => ...\""
in
match
Ast_payload.raw_as_string_exp_exn ~kind:Raw_exp ~is_function payload
with
| None -> err ()
| Some exp ->
(* Wrap a type constraint based on arity.
E.g. for arity 2 constrain to type (_, _) => _ *)
let wrapTypeConstraint (e : Parsetree.expression) =
let loc = e.pexp_loc in
let any = Ast_helper.Typ.any ~loc:e.pexp_loc () in
let unit = Ast_literal.type_unit ~loc () in
let rec arrow ~arity =
if arity = 0 then Ast_helper.Typ.arrow ~loc Nolabel unit any
else if arity = 1 then Ast_helper.Typ.arrow ~loc Nolabel any any
else Ast_helper.Typ.arrow ~loc Nolabel any (arrow ~arity:(arity - 1))
in
match !is_function with
| Some arity ->
let type_ =
Ast_uncurried.uncurriedType ~loc
~arity:(if arity = 0 then 1 else arity)
(arrow ~arity)
in
Ast_helper.Exp.constraint_ ~loc e type_
| _ -> err ()
in
wrapTypeConstraint
{
exp with
pexp_desc =
Ast_external_mk.local_external_apply loc ~pval_prim:["#raw_expr"]
~pval_type:(Typ.arrow Nolabel (Typ.any ()) (Typ.any ()))
[exp];
pexp_attributes =
(match !is_function with
| None -> exp.pexp_attributes
| Some _ -> Ast_attributes.internal_expansive :: exp.pexp_attributes);
}

let handle_raw_structure loc payload =
match Ast_payload.raw_as_string_exp_exn ~kind:Raw_program payload with
| Some exp ->
Expand Down
2 changes: 2 additions & 0 deletions jscomp/frontend/ast_exp_handle_external.mli
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ val handle_external : Location.t -> string -> Parsetree.expression

val handle_debugger : Location.t -> Ast_payload.t -> Parsetree.expression_desc

val handle_ffi : loc:Location.t -> payload:Ast_payload.t -> Parsetree.expression

val handle_raw :
kind:Js_raw_info.raw_kind ->
Location.t ->
Expand Down
10 changes: 5 additions & 5 deletions jscomp/ml/ast_payload.ml
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,11 @@ let raw_as_string_exp_exn ~(kind : Js_raw_info.raw_kind) ?is_function (x : t) :
Location.raise_errorf ~loc
"Syntax error: a valid JS regex literal expected");
(match is_function with
| Some is_function -> (
match Classify_function.classify_exp prog with
| Js_function {arity = _; _} -> is_function := true
| _ -> ())
| None -> ());
| Some is_function -> (
match Classify_function.classify_exp prog with
| Js_function {arity; _} -> is_function := Some arity
| _ -> ())
| None -> ());
errors
| Raw_program -> snd (Parser_flow.parse_program false None str));
Some {e with pexp_desc = Pexp_constant (Pconst_string (str, None))}
Expand Down
2 changes: 1 addition & 1 deletion jscomp/ml/ast_payload.mli
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ val is_single_ident : t -> Longident.t option

val raw_as_string_exp_exn :
kind:Js_raw_info.raw_kind ->
?is_function:bool ref ->
?is_function:int option ref ->
t ->
Parsetree.expression option
(** Convert %raw into expression *)
Expand Down
13 changes: 13 additions & 0 deletions jscomp/test/FFI.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions jscomp/test/FFI.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
@@uncurried

let canUseCanvas: unit => bool = %ffi(`
function canUseCanvas() {
return !!document.createElement('canvas').getContext;
}
`)

let add: (int, int) => int = %ffi(`(x,y)=>x+y`)
3 changes: 2 additions & 1 deletion jscomp/test/build.ninja

Large diffs are not rendered by default.

0 comments on commit c72928e

Please sign in to comment.