Skip to content

Commit 6178dce

Browse files
committed
Fix issue with comparison of string constants
String constants were compare literally, as part of constant propagation. This means that for example `"\a" == "a"` was compiled to `false` even though the two represent the same strings. Unicode and hex escape codes are other examples where literal comparison is not correct. Implemented a 3-way string comparison: equality returns Some(true), Some(false), or None where the result is not determined.
1 parent aea9227 commit 6178dce

13 files changed

+1172
-1023
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
- Fix implementation of directives https://github.com/rescript-lang/rescript-compiler/pull/6052
1717
- Fix issue if the `lib` dir is included in the sources of bsconfig.json https://github.com/rescript-lang/rescript-compiler/pull/6055
1818
- Fix issue with string escape in pattern match https://github.com/rescript-lang/rescript-compiler/pull/6062
19+
- Fix issue with literal comparison of string constants https://github.com/rescript-lang/rescript-compiler/pull/6065
1920

2021
#### :rocket: New Feature
2122
- Add support for toplevel `await` https://github.com/rescript-lang/rescript-compiler/pull/6054

jscomp/core/js_exp_make.ml

+21-10
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,15 @@ let zero_float_lit : t =
556556
let float_mod ?comment e1 e2 : J.expression =
557557
{ comment; expression_desc = Bin (Mod, e1, e2) }
558558

559+
560+
let str_equal (txt0:string) (delim0:External_arg_spec.delim) txt1 delim1 =
561+
if delim0 = delim1 then
562+
if Ext_string.equal txt0 txt1 then Some true
563+
else if Ast_utf8_string.simple_comparison txt0 && Ast_utf8_string.simple_comparison txt1
564+
then Some false
565+
else None
566+
else None
567+
559568
let rec triple_equal ?comment (e0 : t) (e1 : t) : t =
560569
match (e0.expression_desc, e1.expression_desc) with
561570
| ( (Null | Undefined),
@@ -566,9 +575,6 @@ let rec triple_equal ?comment (e0 : t) (e1 : t) : t =
566575
(Null | Undefined) )
567576
when no_side_effect e0 ->
568577
false_
569-
| Str { txt = x }, Str { txt = y } ->
570-
(* CF*)
571-
bool (Ext_string.equal x y)
572578
| Number (Int { i = i0; _ }), Number (Int { i = i1; _ }) -> bool (i0 = i1)
573579
| Optional_block (a, _), Optional_block (b, _) -> triple_equal ?comment a b
574580
| Undefined, Optional_block _
@@ -743,9 +749,13 @@ let rec float_equal ?comment (e0 : t) (e1 : t) : t =
743749
let int_equal = float_equal
744750

745751
let string_equal ?comment (e0 : t) (e1 : t) : t =
752+
let default () : t = { expression_desc = Bin (EqEqEq, e0, e1); comment } in
746753
match (e0.expression_desc, e1.expression_desc) with
747-
| Str { txt = a0 }, Str { txt = b0 } -> bool (Ext_string.equal a0 b0)
748-
| _, _ -> { expression_desc = Bin (EqEqEq, e0, e1); comment }
754+
| Str { txt = a0; delim = d0 }, Str { txt = a1; delim = d1 } when d0 = d1 ->
755+
(match str_equal a0 d0 a1 d1 with
756+
| Some b -> bool b
757+
| None -> default ())
758+
| _, _ -> default ()
749759

750760
let is_type_number ?comment (e : t) : t =
751761
string_equal ?comment (typeof e) (str "number")
@@ -827,11 +837,12 @@ let uint32 ?comment n : J.expression =
827837

828838
let string_comp (cmp : J.binop) ?comment (e0 : t) (e1 : t) =
829839
match (e0.expression_desc, e1.expression_desc) with
830-
| Str { txt = a0 }, Str { txt = b0 } -> (
831-
match cmp with
832-
| EqEqEq -> bool (a0 = b0)
833-
| NotEqEq -> bool (a0 <> b0)
834-
| _ -> bin ?comment cmp e0 e1)
840+
| Str { txt = a0; delim = d0 }, Str { txt = a1; delim = d1 } -> (
841+
match cmp, str_equal a0 d0 a1 d1 with
842+
| EqEqEq, Some b -> bool b
843+
| NotEqEq, Some b -> bool (b = false)
844+
| _ ->
845+
bin ?comment cmp e0 e1)
835846
| _ -> bin ?comment cmp e0 e1
836847

837848
let obj_length ?comment e : t =

jscomp/frontend/ast_utf8_string.ml

+12
Original file line numberDiff line numberDiff line change
@@ -195,3 +195,15 @@ let transform loc s =
195195
Buffer.contents buf
196196
with Error (offset, error) ->
197197
Location.raise_errorf ~loc "Offset: %d, %a" offset pp_error error
198+
199+
let rec check_no_escapes_or_unicode (s : string) (byte_offset : int) (s_len : int) =
200+
if byte_offset = s_len then true
201+
else
202+
let current_char = s.[byte_offset] in
203+
match Ext_utf8.classify current_char with
204+
| Single 92 (* '\\' *) -> false
205+
| Single _ -> check_no_escapes_or_unicode s (byte_offset + 1) s_len
206+
| Invalid | Cont _ | Leading _ -> false
207+
208+
let simple_comparison s =
209+
check_no_escapes_or_unicode s 0 (String.length s)

jscomp/frontend/ast_utf8_string.mli

+3
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,6 @@ val pp_error : Format.formatter -> error -> unit
3535
val transform_test : string -> string
3636

3737
val transform : Location.t -> string -> string
38+
39+
(* Check if the string is only == to itself (no unicode or escape tricks) *)
40+
val simple_comparison : string -> bool

jscomp/frontend/external_arg_spec.ml

+2
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424

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

27+
type delim = J.delim
28+
2729
type cst =
2830
| Arg_int_lit of int
2931
| Arg_string_lit of string * J.delim

jscomp/frontend/external_arg_spec.mli

+2
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
* along with this program; if not, write to the Free Software
2323
* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *)
2424

25+
type delim = J.delim
26+
2527
type cst = private
2628
| Arg_int_lit of int
2729
| Arg_string_lit of string * J.delim

jscomp/test/build.ninja

+2-1
Large diffs are not rendered by default.
+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
'use strict';
2+
3+
4+
var a1 = true;
5+
6+
var a2 = false;
7+
8+
var a3 = "'" === "\'";
9+
10+
var a4 = "'" !== "\'";
11+
12+
exports.a1 = a1;
13+
exports.a2 = a2;
14+
exports.a3 = a3;
15+
exports.a4 = a4;
16+
/* a1 Not a pure module */
+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
let a1 = "'" == "'"
2+
let a2 = "'" != "'"
3+
let a3 = "'" == "\'"
4+
let a4 = "'" != "\'"

lib/4.06.1/unstable/all_ounit_tests.ml

+13
Original file line numberDiff line numberDiff line change
@@ -39458,6 +39458,8 @@ val transform_test : string -> string
3945839458

3945939459
val transform : Location.t -> string -> string
3946039460

39461+
(* Check if the string is only == to itself (no unicode or escape tricks) *)
39462+
val simple_comparison : string -> bool
3946139463
end = struct
3946239464
#1 "ast_utf8_string.ml"
3946339465
(* Copyright (C) 2015-2016 Bloomberg Finance L.P.
@@ -39658,6 +39660,17 @@ let transform loc s =
3965839660
with Error (offset, error) ->
3965939661
Location.raise_errorf ~loc "Offset: %d, %a" offset pp_error error
3966039662

39663+
let rec check_no_escapes_or_unicode (s : string) (byte_offset : int) (s_len : int) =
39664+
if byte_offset = s_len then true
39665+
else
39666+
let current_char = s.[byte_offset] in
39667+
match Ext_utf8.classify current_char with
39668+
| Single 92 (* '\\' *) -> false
39669+
| Single _ -> check_no_escapes_or_unicode s (byte_offset + 1) s_len
39670+
| Invalid | Cont _ | Leading _ -> false
39671+
39672+
let simple_comparison s =
39673+
check_no_escapes_or_unicode s 0 (String.length s)
3966139674
end
3966239675
module Ast_compatible : sig
3966339676
#1 "ast_compatible.mli"

0 commit comments

Comments
 (0)