Small cleanup (#1893)

* Compiler: better heuristic for read_lines

* Misc: review open_in vs open_in_bin

* Compiler: refactor reading lines from files

* fixup! Compiler: refactor reading lines from files

* fixup! fixup! Compiler: refactor reading lines from files

Author: hhugo

compiler/bin-js_of_ocaml/compile.ml

@@ -184,11 +184,11 @@ let run
     | Some file ->
         if not (Sys.file_exists file)
         then failwith (Printf.sprintf "export file %S does not exist" file);
-        let ic = open_in file in
+        let ic = open_in_text file in
         let t = Hashtbl.create 17 in
         (try
            while true do
-             Hashtbl.add t (input_line ic) ()
+             Hashtbl.add t (String.trim (In_channel.input_line_exn ic)) ()
            done;
            assert false
          with End_of_file -> ());

compiler/bin-wasm_of_ocaml/preprocess.ml

@@ -22,19 +22,6 @@ open Wasm_of_ocaml_compiler
 
 let () = Sys.catch_break true
 
-let read_contents ch =
-  let buf = Buffer.create 65536 in
-  let b = Bytes.create 65536 in
-  let rec read () =
-    let n = input ch b 0 (Bytes.length b) in
-    if n > 0
-    then (
-      Buffer.add_subbytes buf b 0 n;
-      read ())
-  in
-  read ();
-  Buffer.contents buf
-
 type variables =
   { enable : string list
   ; disable : string list
@@ -102,7 +89,7 @@ let preprocess { input_file; output_file; variables } =
     match input_file with
     | None -> f stdin
     | Some file ->
-        let ch = open_in file in
+        let ch = open_in_text file in
         let res = f ch in
         close_in ch;
         res
@@ -112,7 +99,7 @@ let preprocess { input_file; output_file; variables } =
     | Some "-" | None -> f stdout
     | Some file -> Filename.gen_file file f
   in
-  let contents = with_input read_contents in
+  let contents = with_input In_channel.input_all in
   let res =
     Wat_preprocess.f
       ~filename:(Option.value ~default:"-" input_file)

compiler/lib-wasm/binaryen.ml

@@ -76,13 +76,13 @@ let generate_dependencies ~dependencies primitives =
           (Yojson.Basic.Util.to_list (Yojson.Basic.from_string dependencies))))
 
 let filter_unused_primitives primitives usage_file =
-  let ch = open_in usage_file in
+  let ch = open_in_bin usage_file in
   let s = ref primitives in
   (try
      while true do
        let l = input_line ch in
        match String.drop_prefix ~prefix:"unused: js:" l with
-       | Some nm -> s := StringSet.remove nm !s
+       | Some nm -> s := StringSet.remove (String.trim nm) !s
        | None -> ()
      done
    with End_of_file -> ());

compiler/lib-wasm/link.ml

@@ -106,7 +106,7 @@ module Wasm_binary = struct
   let check ~contents = String.starts_with ~prefix:header contents
 
   let check_file ~file =
-    let ch = open_in file in
+    let ch = open_in_bin file in
     let res =
       try
         let s = really_input_string ch 8 in

compiler/lib/link_js.ml

@@ -57,21 +57,7 @@ end = struct
     t.lnum <- 0
 
   let open_ fname =
-    let lines =
-      (* If we are not on 32-bit hardware (where the max string length is too
-         small), read the entire file and split it in lines. This is faster
-         than reading it line by line. On 32 bits, we fall back to a
-         line-by-line read. *)
-      let ic = open_in_bin fname in
-      let x =
-        if Sys.word_size >= 64
-        then
-          really_input_string ic (in_channel_length ic) |> String.split_on_char ~sep:'\n'
-        else In_channel.input_lines ic
-      in
-      close_in ic;
-      x
-    in
+    let lines = file_lines_bin fname in
     { lines; lnum = 0; fname; current = lines }
 
   let next t =

compiler/lib/parse_js.ml

@@ -67,7 +67,7 @@ end = struct
     { l; env = Flow_lexer.Lex_env.create l; report_error }
 
   let of_file file : t =
-    let ic = open_in file in
+    let ic = open_in_bin file in
     let lexbuf = Sedlexing.Utf8.from_channel ic in
     Sedlexing.set_filename lexbuf file;
     create lexbuf

compiler/lib/source_map.ml

@@ -773,3 +773,22 @@ type info =
   ; sources : string list
   ; names : string list
   }
+
+let find_in_js_file file =
+  let lines =
+    file_lines_bin file
+    |> List.filter_map ~f:(String.drop_prefix ~prefix:"//# sourceMappingURL=")
+  in
+  match lines with
+  | [ line ] ->
+      let content =
+        match String.drop_prefix ~prefix:"data:application/json;base64," line with
+        | None ->
+            let ic = open_in_bin (String.trim line) in
+            let c = In_channel.input_all ic in
+            close_in ic;
+            c
+        | Some base64 -> Base64.decode_exn (String.trim base64)
+      in
+      Some (of_string content)
+  | _ -> None

compiler/lib/source_map.mli

@@ -149,6 +149,8 @@ val of_file : ?tmp_buf:Buffer.t -> string -> t
 
 val invariant : t -> unit
 
+val find_in_js_file : string -> t option
+
 type info =
   { mappings : Mappings.decoded
   ; sources : string list

compiler/lib/stdlib.ml

@@ -16,6 +16,20 @@
  * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
  *)
 
+let open_in_text = open_in
+
+let open_out_text = open_out
+
+module Deprecated : sig
+  val open_in : string -> in_channel [@@deprecated "use open_int_text/open_int_bin"]
+
+  val open_out : string -> out_channel [@@deprecated "use open_out_text/open_out_bin"]
+end = struct
+  let open_in = open_in
+
+  let open_out = open_out
+end
+
 module Poly = struct
   external ( < ) : 'a -> 'a -> bool = "%lessthan"
 
@@ -1234,6 +1248,8 @@ module Fun = struct
 end
 
 module In_channel = struct
+  let stdlib_input_line = input_line
+
   (* Read up to [len] bytes into [buf], starting at [ofs]. Return total bytes
      read. *)
   let read_upto ic buf ofs len =
@@ -1327,6 +1343,8 @@ module In_channel = struct
       | exception End_of_file -> acc
     in
     List.rev (aux [])
+
+  let input_line_exn = stdlib_input_line
 end
 [@@if ocaml_version < (4, 14, 0)]
 
@@ -1341,9 +1359,50 @@ module In_channel = struct
     | line -> line :: input_lines ic
     | exception End_of_file -> []
   [@@if ocaml_version < (5, 1, 0)]
+
+  let input_line_exn = stdlib_input_line
 end
 [@@if ocaml_version >= (4, 14, 0)]
 
+let split_lines s =
+  if String.equal s ""
+  then []
+  else
+    let sep = '\n' in
+    let r = ref [] in
+    let j = ref (String.length s) in
+    (* ignore trailing new line *)
+    if Char.equal (String.unsafe_get s (!j - 1)) sep then decr j;
+    for i = !j - 1 downto 0 do
+      if Char.equal (String.unsafe_get s i) sep
+      then (
+        r := String.sub s ~pos:(i + 1) ~len:(!j - i - 1) :: !r;
+        j := i)
+    done;
+    String.sub s ~pos:0 ~len:!j :: !r
+
+let input_lines_read_once ic len = really_input_string ic len |> split_lines
+
+let file_lines_bin fname =
+  (* If possible, read the entire file and split it in lines.
+     This is faster than reading it line by line.
+     Otherwise, we fall back to a line-by-line read. *)
+  let ic = open_in_bin fname in
+  let len = in_channel_length ic in
+  let x =
+    if len < Sys.max_string_length
+    then input_lines_read_once ic len
+    else In_channel.input_lines ic
+  in
+  close_in ic;
+  x
+
+let file_lines_text file =
+  let ic = open_in_text file in
+  let c = In_channel.input_lines ic in
+  close_in ic;
+  c
+
 let generated_name = function
   | "param" | "match" | "switcher" -> true
   | s -> String.is_prefix ~prefix:"cst_" s

compiler/tests-compiler/util/util.ml

@@ -103,7 +103,7 @@ module Filetype : Filetype_intf.S = struct
     Bytes.unsafe_to_string s
 
   let write_file name contents =
-    let channel = open_out name in
+    let channel = open_out_bin name in
     Printf.fprintf channel "%s" contents;
     close_out channel
 
@@ -254,38 +254,14 @@ let run_bytecode file =
 let swap_extention filename ~ext =
   Format.sprintf "%s.%s" (Filename.remove_extension filename) ext
 
-let input_lines file =
-  let rec loop acc ic =
-    match input_line ic with
-    | line -> loop (line :: acc) ic
-    | exception End_of_file -> List.rev acc
-  in
-  let ic = open_in file in
-  let lines = loop [] ic in
-  close_in ic;
-  lines
-
 let print_file file =
   let open Js_of_ocaml_compiler.Stdlib in
-  let all = input_lines file in
+  let all = file_lines_text file in
   Printf.printf "$ cat %S\n" file;
   List.iteri all ~f:(fun i line -> Printf.printf "%3d: %s\n" (i + 1) line)
 
 let extract_sourcemap file =
-  let open Js_of_ocaml_compiler.Stdlib in
-  let lines =
-    input_lines (Filetype.path_of_js_file file)
-    |> List.filter_map ~f:(String.drop_prefix ~prefix:"//# sourceMappingURL=")
-  in
-  match lines with
-  | [ line ] ->
-      let content =
-        match String.drop_prefix ~prefix:"data:application/json;base64," line with
-        | None -> String.concat ~sep:"\n" (input_lines line)
-        | Some base64 -> Js_of_ocaml_compiler.Base64.decode_exn base64
-      in
-      Some (Js_of_ocaml_compiler.Source_map.of_string content)
-  | _ -> None
+  Js_of_ocaml_compiler.Source_map.find_in_js_file (Filetype.path_of_js_file file)
 
 let compile_to_javascript
     ?(flags = [])