[BUG] Incorrect rewriting rules turning declarations into assignments

[OlivierNicole]

The rewritings at

js_of_ocaml/compiler/lib/js_traverse.ml

Lines 1442 to 1450 in df91fb7

	| Variable_statement (((Var | Let | Const) as k), l1) ->
	let x =
	List.map l1 ~f:(function
	| DeclPattern _ as d -> Variable_statement (k, [ d ]), loc
	| DeclIdent (_, None) as d -> Variable_statement (k, [ d ]), loc
	| DeclIdent (ident, Some (exp, _)) as d -> (
	match assign_op (EVar ident, exp) with
	| Some e -> Expression_statement e, loc
	| None -> Variable_statement (k, [ d ]), loc))

look weird to me.

They attempt to turn a declaration like var x = x + 2; into the statement x += 2 and similarly for other arithmetic operators. It makes sense for assignments (i.e., without var), but declarations? The rewrites don’t preserve the semantics in the general case, e.g. when x is unbound before the statement, it is bound to NaN after var x = x + 2, whereas x += 2 throws a ReferenceError.

In the context of js_of_ocaml, I assume that the compiler never produces var x = x + 2 with x unbound, but then I am wondering:

1. are we ok with relying on this implicit assumption?
2. Does the compiler ever produce var x = x + 2 where x is already bound? I.e., is this rewriting rule ever applied?

[hhugo]

are we ok with relying on this implicit assumption?

I'd prefer to not depend on such implicit assumption.

The following code snippet make jsoo_minify fail

$ dune exe jsoo_minify -- b.js --debug js_assign 
function f(){const b = 2; return function(){<c> += 2; return <c>;};}
Some variables escaped: <c>
_build/install/default/bin/jsoo_minify: You found a bug. Please report it at https://github.com/ocsigen/js_of_ocaml/issues :
Error: File "compiler/lib/js_assign.ml", line 386, characters 5-11: Assertion failed

function f () {
  const c = 2;
  return function () {
    var c = c + 2;
    return c
  }
}

Do you want to make a PR including regression tests ?

[OlivierNicole]

Sure.

[vouillon]

I can write artificial examples where the optimization is triggered:

let rec f i =
  if i land 1 = 0 then f (i lor 1)
let rec g i = if i < 9. then (g (i +. 1.))

I think we can just remove this rewriting.

[hhugo]

fixed in #1471

[OlivierNicole]

Thanks for taking care of it. Sorry, in the end I did not find the time to do it before my week off.