Skip to content

allow keyword in properties #1430

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

Merged
merged 2 commits into from
Mar 23, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
43 changes: 32 additions & 11 deletions jscomp/bin/whole_compiler.ml
Original file line number Diff line number Diff line change
Expand Up @@ -83367,12 +83367,37 @@ let pp_string f ?(quote='"') (* ?(utf=false)*) s =
P.string f quote_s
;;

(** used in printing keys
{[
{"x" : x};;
{x : x }
]}
*)
let property_string f s =
if Ext_ident.property_no_need_convert s then
P.string f s
else
pp_string f ~quote:(best_string_quote s) s

(** used in property access
{[
f.x ;;
f["x"];;
]}
*)
let property_access f s =
if Ext_ident.property_no_need_convert s then
begin
P.string f L.dot;
P.string f s;
end
else
begin
P.bracket_group f 1 @@ fun _ ->
pp_string f ~quote:( best_string_quote s) s
end


(* TODO: check utf's correct semantics *)
let pp_quote_string f s =
pp_string f ~quote:(best_string_quote s ) s
Expand Down Expand Up @@ -83705,11 +83730,16 @@ and vident cxt f (v : J.vident) =
begin match v with
| Id v | Qualified(v, _, None) ->
ident cxt f v
| Qualified (id,_, Some name) ->
| Qualified (id, (Ml | Runtime), Some name) ->
let cxt = ident cxt f id in
P.string f L.dot;
P.string f (Ext_ident.convert true name);
cxt
| Qualified (id, External _, Some name) ->
let cxt = ident cxt f id in
property_access f name ;
cxt

end

and expression l cxt f (exp : J.expression) : Ext_pp_scope.t =
Expand Down Expand Up @@ -84197,16 +84227,7 @@ and
| Dot (e, s,normal) ->
let action () =
let cxt = expression 15 cxt f e in
if Ext_ident.property_no_need_convert s then
begin
P.string f L.dot;
P.string f s;
end
else
begin
P.bracket_group f 1 @@ fun _ ->
pp_string f ~quote:( best_string_quote s) s
end;
property_access f s ;
(* See [Js_program_loader.obj_of_exports]
maybe in the ast level we should have
refer and export
Expand Down
43 changes: 43 additions & 0 deletions jscomp/core/design.md
Original file line number Diff line number Diff line change
Expand Up @@ -228,3 +228,46 @@ We introduced `#` so that we can do some optimizations.
http://www.2ality.com/2012/03/converting-to-string.html
Note that `""+ Symbol()` does not work any more, we should favor `String` instead


# name mangling

## let bound identifier mangling

### keyword
Note there are two issues, if it is keyword, the output may not be parsable if we don't do name mangling


```js
> var case = 3
< SyntaxError: Cannot use the keyword 'case' as a variable name.
```
### global variable
If it is global variable, it is parsable, it may trigger even subtle errors:

```js
(function(){ 'use strict'; var document = 3; console.log(document)})()
VM1146:1 3
3
```
This could be problematic for bindings
```ocaml
let process = 3
Process.env##OCAML
```
In general global variables would be problematic for bindings

## property name mangling

Nowadays, JS engine support keywords as property name very well

```js
var f = { true : true, false : false }
```

But it has problems when it is too simple for parsing
```js
var f = { true, false} // parsign rules ambiguity
```

If we don't do ES6, we should not go with name mangling, however, it is mostly due to we can
not express these keywords, such as `_open` as property in OCaml, so we did the name mangling
43 changes: 32 additions & 11 deletions jscomp/core/js_dump.ml
Original file line number Diff line number Diff line change
Expand Up @@ -243,12 +243,37 @@ let pp_string f ?(quote='"') (* ?(utf=false)*) s =
P.string f quote_s
;;

(** used in printing keys
{[
{"x" : x};;
{x : x }
]}
*)
let property_string f s =
if Ext_ident.property_no_need_convert s then
P.string f s
else
pp_string f ~quote:(best_string_quote s) s

(** used in property access
{[
f.x ;;
f["x"];;
]}
*)
let property_access f s =
if Ext_ident.property_no_need_convert s then
begin
P.string f L.dot;
P.string f s;
end
else
begin
P.bracket_group f 1 @@ fun _ ->
pp_string f ~quote:( best_string_quote s) s
end


(* TODO: check utf's correct semantics *)
let pp_quote_string f s =
pp_string f ~quote:(best_string_quote s ) s
Expand Down Expand Up @@ -581,11 +606,16 @@ and vident cxt f (v : J.vident) =
begin match v with
| Id v | Qualified(v, _, None) ->
ident cxt f v
| Qualified (id,_, Some name) ->
| Qualified (id, (Ml | Runtime), Some name) ->
let cxt = ident cxt f id in
P.string f L.dot;
P.string f (Ext_ident.convert true name);
cxt
| Qualified (id, External _, Some name) ->
let cxt = ident cxt f id in
property_access f name ;
cxt

end

and expression l cxt f (exp : J.expression) : Ext_pp_scope.t =
Expand Down Expand Up @@ -1073,16 +1103,7 @@ and
| Dot (e, s,normal) ->
let action () =
let cxt = expression 15 cxt f e in
if Ext_ident.property_no_need_convert s then
begin
P.string f L.dot;
P.string f s;
end
else
begin
P.bracket_group f 1 @@ fun _ ->
pp_string f ~quote:( best_string_quote s) s
end;
property_access f s ;
(* See [Js_program_loader.obj_of_exports]
maybe in the ast level we should have
refer and export
Expand Down
3 changes: 3 additions & 0 deletions jscomp/test/.depend
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ event_ffi.cmj : ../stdlib/list.cmj ../runtime/js_unsafe.cmj \
exception_alias.cmj : ../stdlib/list.cmj
exception_raise_test.cmj : mt.cmj
exception_value_test.cmj :
export_keyword.cmj :
ext_array.cmj : ../stdlib/list.cmj ../stdlib/array.cmj
ext_bytes.cmj : ../stdlib/pervasives.cmj ../stdlib/bytes.cmj
ext_filename.cmj : ../stdlib/sys.cmj ../stdlib/string.cmj literals.cmj \
Expand Down Expand Up @@ -272,6 +273,8 @@ js_val.cmj :
jsoo_400_test.cmj : ../stdlib/string.cmj mt.cmj
jsoo_485.cmj :
jsoo_485_test.cmj :
key_word_property.cmj :
key_word_property2.cmj : export_keyword.cmj
largest_int_flow.cmj :
lazy_test.cmj : mt.cmj ../stdlib/lazy.cmj
lexer_test.cmj : number_lexer.cmj mt.cmj ../stdlib/list.cmj \
Expand Down
3 changes: 3 additions & 0 deletions jscomp/test/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ OTHERS := literals a test_ari test_export2 test_internalOO test_obj_simple_ffi t
js_cast_test\
gpr_1423_nav\
gpr_1423_app_test\
export_keyword\
key_word_property\
key_word_property2\
bb

# bs_uncurry_test
Expand Down
13 changes: 13 additions & 0 deletions jscomp/test/export_keyword.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use strict';


var $$case = 3;

var $$window = 2;

var $$switch = 3;

exports.$$case = $$case;
exports.$$window = $$window;
exports.$$switch = $$switch;
/* No side effect */
7 changes: 7 additions & 0 deletions jscomp/test/export_keyword.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@



let case = 3
let window = 2

let switch = 3
53 changes: 53 additions & 0 deletions jscomp/test/key_word_property.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
'use strict';

var Vscode = require("vscode");
var SomeEs6Module = require("some-es6-module");

var $$default = SomeEs6Module.default;

var $$window = Vscode.window;

function mk($$window, $$default) {
return {
window: $$window,
default: $$default
};
}

function mk2($$window, $$default) {
return /* :: */[
/* record */[
/* window */$$window,
/* default */$$default
],
/* [] */0
];
}

function des(v) {
return {
window: v.window,
default: v.default
};
}

var test = {
case: 3,
window: 3
};

function u() {
return $$window.switch();
}

var $$case = 3;

exports.$$default = $$default;
exports.$$window = $$window;
exports.mk = mk;
exports.mk2 = mk2;
exports.des = des;
exports.$$case = $$case;
exports.test = test;
exports.u = u;
/* default Not a pure module */
26 changes: 26 additions & 0 deletions jscomp/test/key_word_property.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@

type t


external default : t = "default" [@@bs.module "some-es6-module"]

let default = default
type window
external window : window = "" [@@bs.val] [@@bs.module "vscode"]

let window = window
let mk window default = [%obj{window; default ; }]
type t_ = { window : int ; default : int }

let mk2 window default = [{window; default ; }]

let des v = [%obj{window = v##window ; default = v##default }]


let case = 3

let test = [%obj{case ; window = 3}]

external switch : window -> string = "" [@@bs.send]

let u () = switch window
30 changes: 30 additions & 0 deletions jscomp/test/key_word_property2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
'use strict';

var Export_keyword = require("./export_keyword");

function test2(v) {
return {
open: v.open,
window: v.window
};
}

function test(p) {
return /* tuple */[
p.catch,
p.then
];
}

var $$case = Export_keyword.$$case;

var $$window = Export_keyword.$$window;

var $$switch = Export_keyword.$$switch;

exports.test2 = test2;
exports.test = test;
exports.$$case = $$case;
exports.$$window = $$window;
exports.$$switch = $$switch;
/* No side effect */
8 changes: 8 additions & 0 deletions jscomp/test/key_word_property2.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
let test2 v = [%obj{
_open = v##_open ;
window = v##window;
}]

let test p = p##catch, p##_then
let case, window, switch =
Export_keyword.case, Export_keyword.window, Export_keyword.switch