Skip to content

Commit 1102a9e

Browse files
committed
fix boolean negation bug
1 parent ce518f2 commit 1102a9e

16 files changed

+138
-66
lines changed

Diff for: jscomp/bin/whole_compiler.ml

+63-27
Original file line numberDiff line numberDiff line change
@@ -57336,7 +57336,8 @@ and expression_desc =
5733657336
[typeof] is an operator
5733757337
*)
5733857338
| Typeof of expression
57339-
| Not of expression (* !v *)
57339+
| Caml_not of expression (* 1 - v *)
57340+
| Js_not of expression (* !v *)
5734057341
| String_of_small_int_array of expression
5734157342
(* String.fromCharCode.apply(null, args) *)
5734257343
(* Convert JS boolean into OCaml boolean
@@ -59639,7 +59640,8 @@ class virtual fold =
5963959640
(* https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence
5964059641
[typeof] is an operator
5964159642
*)
59642-
(* !v *) (* String.fromCharCode.apply(null, args) *)
59643+
(* 1 - v *) (* !v *)
59644+
(* String.fromCharCode.apply(null, args) *)
5964359645
(* Convert JS boolean into OCaml boolean
5964459646
like [+true], note this ast talks using js
5964559647
terminnology unless explicity stated
@@ -59877,7 +59879,8 @@ class virtual fold =
5987759879
| Anything_to_number _x -> let o = o#expression _x in o
5987859880
| Bool _x -> let o = o#bool _x in o
5987959881
| Typeof _x -> let o = o#expression _x in o
59880-
| Not _x -> let o = o#expression _x in o
59882+
| Caml_not _x -> let o = o#expression _x in o
59883+
| Js_not _x -> let o = o#expression _x in o
5988159884
| String_of_small_int_array _x -> let o = o#expression _x in o
5988259885
| Json_stringify _x -> let o = o#expression _x in o
5988359886
| Anything_to_string _x -> let o = o#expression _x in o
@@ -60178,7 +60181,8 @@ let rec no_side_effect_expression_desc (x : J.expression_desc) =
6017860181
(* | Tag_ml_obj _ *)
6017960182
| Int_of_boolean _
6018060183
| J.Anything_to_number _
60181-
| Not _
60184+
| Caml_not _
60185+
| Js_not _
6018260186
| String_of_small_int_array _
6018360187
| Json_stringify _
6018460188
| Anything_to_string _
@@ -61546,24 +61550,37 @@ let rec or_ ?comment (e1 : t) (e2 : t) =
6154661550
it is right that !(x > 3 ) -> x <= 3 *)
6154761551
let rec not ({expression_desc; comment} as e : t) : t =
6154861552
match expression_desc with
61549-
| Bin(EqEqEq , e0,e1)
61553+
| Number (Int {i; _}) ->
61554+
if i <> 0l then caml_false else caml_true
61555+
| Int_of_boolean x -> js_not x e
61556+
| Caml_not e -> e
61557+
| Js_not e -> e
61558+
(* match expression_desc with *)
61559+
(* can still hapen after some optimizations *)
61560+
| Bin(EqEqEq , e0,e1)
6155061561
-> {expression_desc = Bin(NotEqEq, e0,e1); comment}
6155161562
| Bin(NotEqEq , e0,e1) -> {expression_desc = Bin(EqEqEq, e0,e1); comment}
61552-
61553-
(* Note here the compiled js use primtive comparison only
61554-
for *primitive types*, so it is safe to do such optimization,
61555-
for generic comparison, this does not hold
61556-
*)
6155761563
| Bin(Lt, a, b) -> {e with expression_desc = Bin (Ge,a,b)}
61558-
| Bin(Ge,a,b) -> {e with expression_desc = Bin (Lt,a,b)}
61564+
| Bin(Ge,a,b) -> {e with expression_desc = Bin (Lt,a,b)}
6155961565
| Bin(Le,a,b) -> {e with expression_desc = Bin (Gt,a,b)}
6156061566
| Bin(Gt,a,b) -> {e with expression_desc = Bin (Le,a,b)}
61561-
61562-
| Number (Int {i; _}) ->
61563-
if i <> 0l then caml_false else caml_true
61564-
| Int_of_boolean e -> not e
61565-
| Not e -> e
61566-
| x -> {expression_desc = Not e ; comment = None}
61567+
| x -> {expression_desc = Caml_not e ; comment = None}
61568+
and js_not ({expression_desc; comment} as e : t) origin : t =
61569+
match expression_desc with
61570+
| Bin(EqEqEq , e0,e1)
61571+
->
61572+
to_ocaml_boolean {expression_desc = Bin(NotEqEq, e0,e1); comment}
61573+
| Bin(NotEqEq , e0,e1) ->
61574+
to_ocaml_boolean {expression_desc = Bin(EqEqEq, e0,e1); comment}
61575+
| Bin(Lt, a, b) ->
61576+
to_ocaml_boolean {e with expression_desc = Bin (Ge,a,b)}
61577+
| Bin(Ge,a,b) ->
61578+
to_ocaml_boolean {e with expression_desc = Bin (Lt,a,b)}
61579+
| Bin(Le,a,b) ->
61580+
to_ocaml_boolean {e with expression_desc = Bin (Gt,a,b)}
61581+
| Bin(Gt,a,b) ->
61582+
to_ocaml_boolean {e with expression_desc = Bin (Le,a,b)}
61583+
| _ -> {expression_desc = Caml_not origin; comment = None}
6156761584

6156861585
let rec ocaml_boolean_under_condition (b : t) =
6156961586
match b.expression_desc with
@@ -61579,11 +61596,15 @@ let rec ocaml_boolean_under_condition (b : t) =
6157961596
if x == x' && y == y' then b
6158061597
else {b with expression_desc = Bin(Or,x',y')}
6158161598
(** TODO: settle down Not semantics *)
61582-
| Not u
61599+
| Caml_not u
61600+
->
61601+
let u' = ocaml_boolean_under_condition u in
61602+
{b with expression_desc = Js_not u'}
61603+
| Js_not u
6158361604
->
6158461605
let u' = ocaml_boolean_under_condition u in
6158561606
if u' == u then b
61586-
else {b with expression_desc = Not u'}
61607+
else {b with expression_desc = Js_not u'}
6158761608
| _ -> b
6158861609

6158961610
let rec econd ?comment (b : t) (t : t) (f : t) : t =
@@ -61665,7 +61686,10 @@ let rec econd ?comment (b : t) (t : t) (f : t) : t =
6166561686
(* the same as above except we revert the [cond] expression *)
6166661687
econd (or_ b (not p1')) t branch_code0
6166761688

61668-
| Not e, _, _ -> econd ?comment e f t
61689+
| Caml_not e, _, _
61690+
| Js_not e, _, _
61691+
->
61692+
econd ?comment e f t
6166961693
| Int_of_boolean b, _, _ -> econd ?comment b t f
6167061694
(* | Bin (And ,{expression_desc = Int_of_boolean b0},b1), _, _ -> *)
6167161695
(* econd ?comment { b with expression_desc = Bin (And , b0,b1)} t f *)
@@ -65228,8 +65252,8 @@ let rec if_ ?comment ?declaration ?else_ (e : J.expression) (then_ : J.block)
6522865252
exp (E.econd e b a) :: acc
6522965253
| _, [], []
6523065254
-> exp e :: acc
65231-
65232-
| Not e, _ , _ :: _
65255+
| Caml_not e, _ , _ :: _
65256+
| Js_not e, _ , _ :: _
6523365257
-> aux ?comment e else_ then_ acc
6523465258
| _, [], _
6523565259
->
@@ -66953,8 +66977,17 @@ and
6695366977
if l > 12
6695466978
then P.paren_group f 1 action
6695566979
else action ()
66956-
66957-
| Not e ->
66980+
| Caml_not e ->
66981+
expression_desc cxt l f (Bin (Minus, E.one_int_literal, e))
66982+
(* let action () = *)
66983+
(* P.string f "+!" ; (\* FIXME: priority*\) *)
66984+
(* expression 13 cxt f e *)
66985+
(* in *)
66986+
(* if l > 13 *)
66987+
(* then P.paren_group f 1 action *)
66988+
(* else action () *)
66989+
66990+
| Js_not e ->
6695866991
let action () =
6695966992
P.string f "!" ;
6696066993
expression 13 cxt f e
@@ -67440,7 +67473,8 @@ and statement_desc top cxt f (s : J.statement_desc) : Ext_pp_scope.t =
6744067473
| Typeof _
6744167474
| Bind _
6744267475
| Number _
67443-
| Not _
67476+
| Caml_not _ (* FIXME*)
67477+
| Js_not _
6744467478
| Bool _
6744567479
| New _
6744667480
| J.Anything_to_number _
@@ -68664,7 +68698,8 @@ class virtual map =
6866468698
(* https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence
6866568699
[typeof] is an operator
6866668700
*)
68667-
(* !v *) (* String.fromCharCode.apply(null, args) *)
68701+
(* 1 - v *) (* !v *)
68702+
(* String.fromCharCode.apply(null, args) *)
6866868703
(* Convert JS boolean into OCaml boolean
6866968704
like [+true], note this ast talks using js
6867068705
terminnology unless explicity stated
@@ -68917,7 +68952,8 @@ class virtual map =
6891768952
let _x = o#expression _x in Anything_to_number _x
6891868953
| Bool _x -> let _x = o#bool _x in Bool _x
6891968954
| Typeof _x -> let _x = o#expression _x in Typeof _x
68920-
| Not _x -> let _x = o#expression _x in Not _x
68955+
| Caml_not _x -> let _x = o#expression _x in Caml_not _x
68956+
| Js_not _x -> let _x = o#expression _x in Js_not _x
6892168957
| String_of_small_int_array _x ->
6892268958
let _x = o#expression _x in String_of_small_int_array _x
6892368959
| Json_stringify _x -> let _x = o#expression _x in Json_stringify _x

Diff for: jscomp/core/j.ml

+2-1
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,8 @@ and expression_desc =
119119
[typeof] is an operator
120120
*)
121121
| Typeof of expression
122-
| Not of expression (* !v *)
122+
| Caml_not of expression (* 1 - v *)
123+
| Js_not of expression (* !v *)
123124
| String_of_small_int_array of expression
124125
(* String.fromCharCode.apply(null, args) *)
125126
(* Convert JS boolean into OCaml boolean

Diff for: jscomp/core/js_analyzer.ml

+2-1
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,8 @@ let rec no_side_effect_expression_desc (x : J.expression_desc) =
118118
(* | Tag_ml_obj _ *)
119119
| Int_of_boolean _
120120
| J.Anything_to_number _
121-
| Not _
121+
| Caml_not _
122+
| Js_not _
122123
| String_of_small_int_array _
123124
| Json_stringify _
124125
| Anything_to_string _

Diff for: jscomp/core/js_dump.ml

+13-3
Original file line numberDiff line numberDiff line change
@@ -741,8 +741,17 @@ and
741741
if l > 12
742742
then P.paren_group f 1 action
743743
else action ()
744-
745-
| Not e ->
744+
| Caml_not e ->
745+
expression_desc cxt l f (Bin (Minus, E.one_int_literal, e))
746+
(* let action () = *)
747+
(* P.string f "+!" ; (\* FIXME: priority*\) *)
748+
(* expression 13 cxt f e *)
749+
(* in *)
750+
(* if l > 13 *)
751+
(* then P.paren_group f 1 action *)
752+
(* else action () *)
753+
754+
| Js_not e ->
746755
let action () =
747756
P.string f "!" ;
748757
expression 13 cxt f e
@@ -1228,7 +1237,8 @@ and statement_desc top cxt f (s : J.statement_desc) : Ext_pp_scope.t =
12281237
| Typeof _
12291238
| Bind _
12301239
| Number _
1231-
| Not _
1240+
| Caml_not _ (* FIXME*)
1241+
| Js_not _
12321242
| Bool _
12331243
| New _
12341244
| J.Anything_to_number _

Diff for: jscomp/core/js_exp_make.ml

+36-16
Original file line numberDiff line numberDiff line change
@@ -631,24 +631,37 @@ let rec or_ ?comment (e1 : t) (e2 : t) =
631631
it is right that !(x > 3 ) -> x <= 3 *)
632632
let rec not ({expression_desc; comment} as e : t) : t =
633633
match expression_desc with
634-
| Bin(EqEqEq , e0,e1)
634+
| Number (Int {i; _}) ->
635+
if i <> 0l then caml_false else caml_true
636+
| Int_of_boolean x -> js_not x e
637+
| Caml_not e -> e
638+
| Js_not e -> e
639+
(* match expression_desc with *)
640+
(* can still hapen after some optimizations *)
641+
| Bin(EqEqEq , e0,e1)
635642
-> {expression_desc = Bin(NotEqEq, e0,e1); comment}
636643
| Bin(NotEqEq , e0,e1) -> {expression_desc = Bin(EqEqEq, e0,e1); comment}
637-
638-
(* Note here the compiled js use primtive comparison only
639-
for *primitive types*, so it is safe to do such optimization,
640-
for generic comparison, this does not hold
641-
*)
642644
| Bin(Lt, a, b) -> {e with expression_desc = Bin (Ge,a,b)}
643-
| Bin(Ge,a,b) -> {e with expression_desc = Bin (Lt,a,b)}
645+
| Bin(Ge,a,b) -> {e with expression_desc = Bin (Lt,a,b)}
644646
| Bin(Le,a,b) -> {e with expression_desc = Bin (Gt,a,b)}
645647
| Bin(Gt,a,b) -> {e with expression_desc = Bin (Le,a,b)}
646-
647-
| Number (Int {i; _}) ->
648-
if i <> 0l then caml_false else caml_true
649-
| Int_of_boolean e -> not e
650-
| Not e -> e
651-
| x -> {expression_desc = Not e ; comment = None}
648+
| x -> {expression_desc = Caml_not e ; comment = None}
649+
and js_not ({expression_desc; comment} as e : t) origin : t =
650+
match expression_desc with
651+
| Bin(EqEqEq , e0,e1)
652+
->
653+
to_ocaml_boolean {expression_desc = Bin(NotEqEq, e0,e1); comment}
654+
| Bin(NotEqEq , e0,e1) ->
655+
to_ocaml_boolean {expression_desc = Bin(EqEqEq, e0,e1); comment}
656+
| Bin(Lt, a, b) ->
657+
to_ocaml_boolean {e with expression_desc = Bin (Ge,a,b)}
658+
| Bin(Ge,a,b) ->
659+
to_ocaml_boolean {e with expression_desc = Bin (Lt,a,b)}
660+
| Bin(Le,a,b) ->
661+
to_ocaml_boolean {e with expression_desc = Bin (Gt,a,b)}
662+
| Bin(Gt,a,b) ->
663+
to_ocaml_boolean {e with expression_desc = Bin (Le,a,b)}
664+
| _ -> {expression_desc = Caml_not origin; comment = None}
652665

653666
let rec ocaml_boolean_under_condition (b : t) =
654667
match b.expression_desc with
@@ -664,11 +677,15 @@ let rec ocaml_boolean_under_condition (b : t) =
664677
if x == x' && y == y' then b
665678
else {b with expression_desc = Bin(Or,x',y')}
666679
(** TODO: settle down Not semantics *)
667-
| Not u
680+
| Caml_not u
681+
->
682+
let u' = ocaml_boolean_under_condition u in
683+
{b with expression_desc = Js_not u'}
684+
| Js_not u
668685
->
669686
let u' = ocaml_boolean_under_condition u in
670687
if u' == u then b
671-
else {b with expression_desc = Not u'}
688+
else {b with expression_desc = Js_not u'}
672689
| _ -> b
673690

674691
let rec econd ?comment (b : t) (t : t) (f : t) : t =
@@ -750,7 +767,10 @@ let rec econd ?comment (b : t) (t : t) (f : t) : t =
750767
(* the same as above except we revert the [cond] expression *)
751768
econd (or_ b (not p1')) t branch_code0
752769

753-
| Not e, _, _ -> econd ?comment e f t
770+
| Caml_not e, _, _
771+
| Js_not e, _, _
772+
->
773+
econd ?comment e f t
754774
| Int_of_boolean b, _, _ -> econd ?comment b t f
755775
(* | Bin (And ,{expression_desc = Int_of_boolean b0},b1), _, _ -> *)
756776
(* econd ?comment { b with expression_desc = Bin (And , b0,b1)} t f *)

Diff for: jscomp/core/js_fold.ml

+4-2
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,8 @@ class virtual fold =
134134
(* https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence
135135
[typeof] is an operator
136136
*)
137-
(* !v *) (* String.fromCharCode.apply(null, args) *)
137+
(* 1 - v *) (* !v *)
138+
(* String.fromCharCode.apply(null, args) *)
138139
(* Convert JS boolean into OCaml boolean
139140
like [+true], note this ast talks using js
140141
terminnology unless explicity stated
@@ -372,7 +373,8 @@ class virtual fold =
372373
| Anything_to_number _x -> let o = o#expression _x in o
373374
| Bool _x -> let o = o#bool _x in o
374375
| Typeof _x -> let o = o#expression _x in o
375-
| Not _x -> let o = o#expression _x in o
376+
| Caml_not _x -> let o = o#expression _x in o
377+
| Js_not _x -> let o = o#expression _x in o
376378
| String_of_small_int_array _x -> let o = o#expression _x in o
377379
| Json_stringify _x -> let o = o#expression _x in o
378380
| Anything_to_string _x -> let o = o#expression _x in o

Diff for: jscomp/core/js_map.ml

+4-2
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,8 @@ class virtual map =
147147
(* https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence
148148
[typeof] is an operator
149149
*)
150-
(* !v *) (* String.fromCharCode.apply(null, args) *)
150+
(* 1 - v *) (* !v *)
151+
(* String.fromCharCode.apply(null, args) *)
151152
(* Convert JS boolean into OCaml boolean
152153
like [+true], note this ast talks using js
153154
terminnology unless explicity stated
@@ -400,7 +401,8 @@ class virtual map =
400401
let _x = o#expression _x in Anything_to_number _x
401402
| Bool _x -> let _x = o#bool _x in Bool _x
402403
| Typeof _x -> let _x = o#expression _x in Typeof _x
403-
| Not _x -> let _x = o#expression _x in Not _x
404+
| Caml_not _x -> let _x = o#expression _x in Caml_not _x
405+
| Js_not _x -> let _x = o#expression _x in Js_not _x
404406
| String_of_small_int_array _x ->
405407
let _x = o#expression _x in String_of_small_int_array _x
406408
| Json_stringify _x -> let _x = o#expression _x in Json_stringify _x

Diff for: jscomp/core/js_stmt_make.ml

+2-2
Original file line numberDiff line numberDiff line change
@@ -208,8 +208,8 @@ let rec if_ ?comment ?declaration ?else_ (e : J.expression) (then_ : J.block)
208208
exp (E.econd e b a) :: acc
209209
| _, [], []
210210
-> exp e :: acc
211-
212-
| Not e, _ , _ :: _
211+
| Caml_not e, _ , _ :: _
212+
| Js_not e, _ , _ :: _
213213
-> aux ?comment e else_ then_ acc
214214
| _, [], _
215215
->

Diff for: jscomp/test/flow_parser_reg_test.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -7149,7 +7149,7 @@ function identifier_no_dupe_check(param, param$1) {
71497149

71507150
function strict_post_check(env, strict, simple, id, params) {
71517151
if (strict || !simple) {
7152-
var env$1 = strict ? with_strict(!env[/* in_strict_mode */5], env) : env;
7152+
var env$1 = strict ? with_strict(1 - env[/* in_strict_mode */5], env) : env;
71537153
if (id) {
71547154
var match = id[0];
71557155
var name = match[1][/* name */0];
@@ -7692,7 +7692,7 @@ function assignment(env) {
76927692
error$1(env$1, /* IllegalYield */24);
76937693
}
76947694
var delegate = maybe(env$1, /* T_MULT */97);
7695-
var has_argument = !(+(Curry._2(Parser_env_048[/* token */0], /* None */0, env$1) === /* T_SEMICOLON */7) || Curry._1(Parser_env_048[/* is_implicit_semicolon */6], env$1));
7695+
var has_argument = 1 - (+(Curry._2(Parser_env_048[/* token */0], /* None */0, env$1) === /* T_SEMICOLON */7) || Curry._1(Parser_env_048[/* is_implicit_semicolon */6], env$1));
76967696
var argument = delegate || has_argument ? /* Some */[Curry._1(assignment, env$1)] : /* None */0;
76977697
var end_loc;
76987698
if (argument) {

Diff for: jscomp/test/ocaml_proto_test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -7272,7 +7272,7 @@ function compile(proto_definition) {
72727272
var has_encoded = first ? Curry._3(f, /* None */0, type_, sc) : Curry._3(f, /* Some */[/* () */0], type_, sc);
72737273
line$1(sc, "");
72747274
if (first) {
7275-
return !has_encoded;
7275+
return 1 - has_encoded;
72767276
}
72777277
else {
72787278
return /* false */0;

0 commit comments

Comments
 (0)