Skip to content

Fix string escape in @as("foo") on argument of externals. #5446

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Jun 20, 2022
Merged

Fix string escape in @as("foo") on argument of externals. #5446

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
bea0b32
emit str
cristianoc Jun 19, 2022
f8c33db
Hack timestamps to get around tree-sitter failing on M1.
cristianoc Jun 19, 2022
9ba7146
Do not escape strings in @bs.as on externals.
cristianoc Jun 19, 2022
a43cf9d
Hack the timestamp by doing a dummy modification.
cristianoc Jun 19, 2022
1445dc3
Comments in code.
cristianoc Jun 19, 2022
eea6b5a
An alternative solution that requires fewer changes.
cristianoc Jun 19, 2022
273d2b4
Remove changes to Str.
cristianoc Jun 20, 2022
c3c8d74
Restore original files.
cristianoc Jun 20, 2022
f9d1223
More restore files.
cristianoc Jun 20, 2022
94a51b9
Comment.
cristianoc Jun 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions jscomp/core/j.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,11 @@ and expression_desc =
The last pararemter [true] return unit
*)
| Str of bool * string
(* A string is UTF-8 encoded, the string may contain
(* A string is UTF-8 encoded, and may contain
escape sequences.
The first argument is used to mark it is non-pure, please
don't optimize it, since it does have side effec,
examples like "use asm;" and our compiler may generate "error;..."
which is better to leave it alone
The last argument is passed from as `j` from `{j||j}`
First argument: used to mark it as non-pure.
Please treat it carefully as it affects optimization.
Second argument: the string "j" in `{j||j}`.
*)
| Unicode of string
(* It is escaped string, print delimited by '"'*)
Expand Down
5 changes: 4 additions & 1 deletion jscomp/core/lam_compile_const.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,5 +101,8 @@ and translate (x : Lam_constant.t) : J.expression =
let translate_arg_cst (cst : External_arg_spec.cst) =
match cst with
| Arg_int_lit i -> E.int (Int32.of_int i)
| Arg_string_lit i -> E.str i
| Arg_string_lit i ->
(* Avoid escaping *)
let s = "\"" ^ i ^ "\"" in
E.raw_js_code (Exp (Js_literal { comment = None })) s
| Arg_js_literal s -> E.raw_js_code (Exp (Js_literal { comment = None })) s
20 changes: 12 additions & 8 deletions jscomp/frontend/ast_external_process.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ let refine_arg_type ~(nolabel : bool) (ptyp : Ast_core_type.t) :
External_arg_spec.attr =
if ptyp.ptyp_desc = Ptyp_any then
let ptyp_attrs = ptyp.ptyp_attributes in
let result = Ast_attributes.iter_process_bs_string_or_int_as ptyp_attrs in
match result with
let payload = Ast_attributes.iter_process_bs_string_or_int_as ptyp_attrs in
match payload with
| None -> spec_of_ptyp nolabel ptyp
| Some cst -> (
(* (_[@as ])*)
Expand All @@ -111,20 +111,24 @@ let refine_obj_arg_type ~(nolabel : bool) (ptyp : Ast_core_type.t) :
External_arg_spec.attr =
if ptyp.ptyp_desc = Ptyp_any then (
let ptyp_attrs = ptyp.ptyp_attributes in
let result = Ast_attributes.iter_process_bs_string_or_int_as ptyp_attrs in
let payload = Ast_attributes.iter_process_bs_string_or_int_as ptyp_attrs in
(* when ppx start dropping attributes
we should warn, there is a trade off whether
we should warn dropped non bs attribute or not
*)
Bs_ast_invariant.warn_discarded_unused_attributes ptyp_attrs;
match result with
match payload with
| None -> Bs_syntaxerr.err ptyp.ptyp_loc Invalid_underscore_type_in_external
| Some (Int i) ->
(* (_[@as ])*)
(* This type is used in obj only to construct obj type*)
(* @as(24) *)
(* This type is used in obj only to construct obj type *)
Arg_cst (External_arg_spec.cst_int i)
| Some (Str i) -> Arg_cst (External_arg_spec.cst_string i)
| Some (Js_literal_str s) -> Arg_cst (External_arg_spec.cst_obj_literal s))
| Some (Str i) ->
(* @as("foo") *)
Arg_cst (External_arg_spec.cst_string i)
| Some (Js_literal_str s) ->
(* @as(json`true`) *)
Arg_cst (External_arg_spec.cst_obj_literal s))
else (* ([`a|`b] [@string]) *)
spec_of_ptyp nolabel ptyp

Expand Down
2 changes: 1 addition & 1 deletion jscomp/frontend/external_arg_spec.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
* along with this program; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *)

(** type definitions for external argument *)
(** type definitions for arguments to a function declared external *)

type cst =
| Arg_int_lit of int
Expand Down
3 changes: 2 additions & 1 deletion jscomp/test/build.ninja
View file
Open in desktop

Large diffs are not rendered by default.

12 changes: 12 additions & 0 deletions jscomp/test/external_ppx2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'use strict';


f("\h\e\l\lo", 42);

var x = "\h\e\l\lo";

var y;

exports.x = x;
exports.y = y;
/* Not a pure module */
4 changes: 4 additions & 0 deletions jscomp/test/external_ppx2.res
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
external f: (@as("\h\e\l\lo") _, int) => unit = "f"

let x = "\h\e\l\lo"
let y = f(42)
37 changes: 21 additions & 16 deletions lib/4.06.1/unstable/js_compiler.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65324,13 +65324,11 @@ and expression_desc =
The last pararemter [true] return unit
*)
| Str of bool * string
(* A string is UTF-8 encoded, the string may contain
(* A string is UTF-8 encoded, and may contain
escape sequences.
The first argument is used to mark it is non-pure, please
don't optimize it, since it does have side effec,
examples like "use asm;" and our compiler may generate "error;..."
which is better to leave it alone
The last argument is passed from as `j` from `{j||j}`
First argument: used to mark it as non-pure.
Please treat it carefully as it affects optimization.
Second argument: the string "j" in `{j||j}`.
*)
| Unicode of string
(* It is escaped string, print delimited by '"'*)
Expand Down Expand Up @@ -73411,7 +73409,7 @@ end = struct
* along with this program; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *)

(** type definitions for external argument *)
(** type definitions for arguments to a function declared external *)

type cst =
| Arg_int_lit of int
Expand Down Expand Up @@ -85219,7 +85217,10 @@ and translate (x : Lam_constant.t) : J.expression =
let translate_arg_cst (cst : External_arg_spec.cst) =
match cst with
| Arg_int_lit i -> E.int (Int32.of_int i)
| Arg_string_lit i -> E.str i
| Arg_string_lit i ->
(* Avoid escaping *)
let s = "\"" ^ i ^ "\"" in
E.raw_js_code (Exp (Js_literal { comment = None })) s
| Arg_js_literal s -> E.raw_js_code (Exp (Js_literal { comment = None })) s

end
Expand Down Expand Up @@ -258219,8 +258220,8 @@ let refine_arg_type ~(nolabel : bool) (ptyp : Ast_core_type.t) :
External_arg_spec.attr =
if ptyp.ptyp_desc = Ptyp_any then
let ptyp_attrs = ptyp.ptyp_attributes in
let result = Ast_attributes.iter_process_bs_string_or_int_as ptyp_attrs in
match result with
let payload = Ast_attributes.iter_process_bs_string_or_int_as ptyp_attrs in
match payload with
| None -> spec_of_ptyp nolabel ptyp
| Some cst -> (
(* (_[@as ])*)
Expand All @@ -258242,20 +258243,24 @@ let refine_obj_arg_type ~(nolabel : bool) (ptyp : Ast_core_type.t) :
External_arg_spec.attr =
if ptyp.ptyp_desc = Ptyp_any then (
let ptyp_attrs = ptyp.ptyp_attributes in
let result = Ast_attributes.iter_process_bs_string_or_int_as ptyp_attrs in
let payload = Ast_attributes.iter_process_bs_string_or_int_as ptyp_attrs in
(* when ppx start dropping attributes
we should warn, there is a trade off whether
we should warn dropped non bs attribute or not
*)
Bs_ast_invariant.warn_discarded_unused_attributes ptyp_attrs;
match result with
match payload with
| None -> Bs_syntaxerr.err ptyp.ptyp_loc Invalid_underscore_type_in_external
| Some (Int i) ->
(* (_[@as ])*)
(* This type is used in obj only to construct obj type*)
(* @as(24) *)
(* This type is used in obj only to construct obj type *)
Arg_cst (External_arg_spec.cst_int i)
| Some (Str i) -> Arg_cst (External_arg_spec.cst_string i)
| Some (Js_literal_str s) -> Arg_cst (External_arg_spec.cst_obj_literal s))
| Some (Str i) ->
(* @as("foo") *)
Arg_cst (External_arg_spec.cst_string i)
| Some (Js_literal_str s) ->
(* @as(json`true`) *)
Arg_cst (External_arg_spec.cst_obj_literal s))
else (* ([`a|`b] [@string]) *)
spec_of_ptyp nolabel ptyp

Expand Down
37 changes: 21 additions & 16 deletions lib/4.06.1/unstable/js_playground_compiler.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65324,13 +65324,11 @@ and expression_desc =
The last pararemter [true] return unit
*)
| Str of bool * string
(* A string is UTF-8 encoded, the string may contain
(* A string is UTF-8 encoded, and may contain
escape sequences.
The first argument is used to mark it is non-pure, please
don't optimize it, since it does have side effec,
examples like "use asm;" and our compiler may generate "error;..."
which is better to leave it alone
The last argument is passed from as `j` from `{j||j}`
First argument: used to mark it as non-pure.
Please treat it carefully as it affects optimization.
Second argument: the string "j" in `{j||j}`.
*)
| Unicode of string
(* It is escaped string, print delimited by '"'*)
Expand Down Expand Up @@ -73411,7 +73409,7 @@ end = struct
* along with this program; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *)

(** type definitions for external argument *)
(** type definitions for arguments to a function declared external *)

type cst =
| Arg_int_lit of int
Expand Down Expand Up @@ -85219,7 +85217,10 @@ and translate (x : Lam_constant.t) : J.expression =
let translate_arg_cst (cst : External_arg_spec.cst) =
match cst with
| Arg_int_lit i -> E.int (Int32.of_int i)
| Arg_string_lit i -> E.str i
| Arg_string_lit i ->
(* Avoid escaping *)
let s = "\"" ^ i ^ "\"" in
E.raw_js_code (Exp (Js_literal { comment = None })) s
| Arg_js_literal s -> E.raw_js_code (Exp (Js_literal { comment = None })) s

end
Expand Down Expand Up @@ -259682,8 +259683,8 @@ let refine_arg_type ~(nolabel : bool) (ptyp : Ast_core_type.t) :
External_arg_spec.attr =
if ptyp.ptyp_desc = Ptyp_any then
let ptyp_attrs = ptyp.ptyp_attributes in
let result = Ast_attributes.iter_process_bs_string_or_int_as ptyp_attrs in
match result with
let payload = Ast_attributes.iter_process_bs_string_or_int_as ptyp_attrs in
match payload with
| None -> spec_of_ptyp nolabel ptyp
| Some cst -> (
(* (_[@as ])*)
Expand All @@ -259705,20 +259706,24 @@ let refine_obj_arg_type ~(nolabel : bool) (ptyp : Ast_core_type.t) :
External_arg_spec.attr =
if ptyp.ptyp_desc = Ptyp_any then (
let ptyp_attrs = ptyp.ptyp_attributes in
let result = Ast_attributes.iter_process_bs_string_or_int_as ptyp_attrs in
let payload = Ast_attributes.iter_process_bs_string_or_int_as ptyp_attrs in
(* when ppx start dropping attributes
we should warn, there is a trade off whether
we should warn dropped non bs attribute or not
*)
Bs_ast_invariant.warn_discarded_unused_attributes ptyp_attrs;
match result with
match payload with
| None -> Bs_syntaxerr.err ptyp.ptyp_loc Invalid_underscore_type_in_external
| Some (Int i) ->
(* (_[@as ])*)
(* This type is used in obj only to construct obj type*)
(* @as(24) *)
(* This type is used in obj only to construct obj type *)
Arg_cst (External_arg_spec.cst_int i)
| Some (Str i) -> Arg_cst (External_arg_spec.cst_string i)
| Some (Js_literal_str s) -> Arg_cst (External_arg_spec.cst_obj_literal s))
| Some (Str i) ->
(* @as("foo") *)
Arg_cst (External_arg_spec.cst_string i)
| Some (Js_literal_str s) ->
(* @as(json`true`) *)
Arg_cst (External_arg_spec.cst_obj_literal s))
else (* ([`a|`b] [@string]) *)
spec_of_ptyp nolabel ptyp

Expand Down
37 changes: 21 additions & 16 deletions lib/4.06.1/whole_compiler.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226378,7 +226378,7 @@ end = struct
* along with this program; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *)

(** type definitions for external argument *)
(** type definitions for arguments to a function declared external *)

type cst =
| Arg_int_lit of int
Expand Down Expand Up @@ -228282,13 +228282,11 @@ and expression_desc =
The last pararemter [true] return unit
*)
| Str of bool * string
(* A string is UTF-8 encoded, the string may contain
(* A string is UTF-8 encoded, and may contain
escape sequences.
The first argument is used to mark it is non-pure, please
don't optimize it, since it does have side effec,
examples like "use asm;" and our compiler may generate "error;..."
which is better to leave it alone
The last argument is passed from as `j` from `{j||j}`
First argument: used to mark it as non-pure.
Please treat it carefully as it affects optimization.
Second argument: the string "j" in `{j||j}`.
*)
| Unicode of string
(* It is escaped string, print delimited by '"'*)
Expand Down Expand Up @@ -246788,7 +246786,10 @@ and translate (x : Lam_constant.t) : J.expression =
let translate_arg_cst (cst : External_arg_spec.cst) =
match cst with
| Arg_int_lit i -> E.int (Int32.of_int i)
| Arg_string_lit i -> E.str i
| Arg_string_lit i ->
(* Avoid escaping *)
let s = "\"" ^ i ^ "\"" in
E.raw_js_code (Exp (Js_literal { comment = None })) s
| Arg_js_literal s -> E.raw_js_code (Exp (Js_literal { comment = None })) s

end
Expand Down Expand Up @@ -262270,8 +262271,8 @@ let refine_arg_type ~(nolabel : bool) (ptyp : Ast_core_type.t) :
External_arg_spec.attr =
if ptyp.ptyp_desc = Ptyp_any then
let ptyp_attrs = ptyp.ptyp_attributes in
let result = Ast_attributes.iter_process_bs_string_or_int_as ptyp_attrs in
match result with
let payload = Ast_attributes.iter_process_bs_string_or_int_as ptyp_attrs in
match payload with
| None -> spec_of_ptyp nolabel ptyp
| Some cst -> (
(* (_[@as ])*)
Expand All @@ -262293,20 +262294,24 @@ let refine_obj_arg_type ~(nolabel : bool) (ptyp : Ast_core_type.t) :
External_arg_spec.attr =
if ptyp.ptyp_desc = Ptyp_any then (
let ptyp_attrs = ptyp.ptyp_attributes in
let result = Ast_attributes.iter_process_bs_string_or_int_as ptyp_attrs in
let payload = Ast_attributes.iter_process_bs_string_or_int_as ptyp_attrs in
(* when ppx start dropping attributes
we should warn, there is a trade off whether
we should warn dropped non bs attribute or not
*)
Bs_ast_invariant.warn_discarded_unused_attributes ptyp_attrs;
match result with
match payload with
| None -> Bs_syntaxerr.err ptyp.ptyp_loc Invalid_underscore_type_in_external
| Some (Int i) ->
(* (_[@as ])*)
(* This type is used in obj only to construct obj type*)
(* @as(24) *)
(* This type is used in obj only to construct obj type *)
Arg_cst (External_arg_spec.cst_int i)
| Some (Str i) -> Arg_cst (External_arg_spec.cst_string i)
| Some (Js_literal_str s) -> Arg_cst (External_arg_spec.cst_obj_literal s))
| Some (Str i) ->
(* @as("foo") *)
Arg_cst (External_arg_spec.cst_string i)
| Some (Js_literal_str s) ->
(* @as(json`true`) *)
Arg_cst (External_arg_spec.cst_obj_literal s))
else (* ([`a|`b] [@string]) *)
spec_of_ptyp nolabel ptyp

Expand Down