factor out affection checker

Use it in the analyzer too.

analyzer.ml

@@ -825,19 +825,17 @@ module TexprTransformer = struct
 			| TContinue | TBreak | TThrow _ | TReturn _ | TVar _ ->
 				assert false
 		and ordered_value_list bb el =
-			let might_be_affected e = match e.eexpr with
-				| TConst _ | TTypeExpr _ | TFunction _ -> false
-				| _ -> true
-			in
+			let might_be_affected,collect_modified_locals = Optimizer.create_affection_checker() in
 			let can_be_optimized e = match e.eexpr with
 				| TBinop _ | TArray _ | TCall _ -> true
 				| _ -> false
 			in
 			let _,el = List.fold_left (fun (had_side_effect,acc) e ->
 				if had_side_effect then
-					(true,(might_be_affected e,can_be_optimized e,e) :: acc)
+					(true,(might_be_affected e || Optimizer.has_side_effect e,can_be_optimized e,e) :: acc)
 				else begin
 					let had_side_effect = Optimizer.has_side_effect e in
+					if had_side_effect then collect_modified_locals e;
 					(had_side_effect,(false,can_be_optimized e,e) :: acc)
 				end
 			) (false,[]) (List.rev el) in

optimizer.ml

@@ -220,6 +220,36 @@ let api_inline ctx c field params p = match c.cl_path, field, params with
 	| _ ->
 		api_inline2 ctx.com c field params p
 
+let create_affection_checker () =
+	let is_affected_type t = match follow t with
+		| TAbstract({a_path = [],("Int" | "Float" | "Bool")},_) -> true
+		| _ -> false
+	in
+	let modified_locals = Hashtbl.create 0 in
+	let rec might_be_affected e =
+		let rec loop e = match e.eexpr with
+			| TConst _ | TFunction _ | TTypeExpr _ -> ()
+			| TLocal v when Hashtbl.mem modified_locals v.v_id -> raise Exit
+			| TField _ when is_affected_type e.etype -> raise Exit
+			| _ -> Type.iter loop e
+		in
+		try
+			loop e;
+			false
+		with Exit ->
+			true
+	in
+	let rec collect_modified_locals e = match e.eexpr with
+		| TUnop((Increment | Decrement),_,{eexpr = TLocal v}) when is_affected_type v.v_type ->
+			Hashtbl.add modified_locals v.v_id true
+		| TBinop((OpAssign | OpAssignOp _),{eexpr = TLocal v},e2) when is_affected_type v.v_type ->
+			collect_modified_locals e2;
+			Hashtbl.add modified_locals v.v_id true
+		| _ ->
+			Type.iter collect_modified_locals e
+	in
+	might_be_affected,collect_modified_locals
+
 (* ---------------------------------------------------------------------- *)
 (* INLINING *)
 
@@ -341,33 +371,7 @@ let rec type_inline ctx cf f ethis params tret config p ?(self_calling_closure=f
 	*)
 	let ethis = (match ethis.eexpr with TConst TSuper -> { ethis with eexpr = TConst TThis } | _ -> ethis) in
 	let vthis = alloc_var "_this" ethis.etype in
-	let is_affected_type t = match follow t with
-		| TAbstract({a_path = [],("Int" | "Float" | "Bool")},_) -> true
-		| _ -> false
-	in
-	let modified_locals = Hashtbl.create 0 in
-	let rec might_be_affected e =
-		let rec loop e = match e.eexpr with
-			| TConst _ | TFunction _ | TTypeExpr _ -> ()
-			| TLocal v when Hashtbl.mem modified_locals v.v_id -> raise Exit
-			| TField _ when is_affected_type e.etype -> raise Exit
-			| _ -> Type.iter loop e
-		in
-		try
-			loop e;
-			false
-		with Exit ->
-			true
-	in
-	let rec collect_modified_locals e = match e.eexpr with
-		| TUnop((Increment | Decrement),_,{eexpr = TLocal v}) when is_affected_type v.v_type ->
-			Hashtbl.add modified_locals v.v_id true
-		| TBinop((OpAssign | OpAssignOp _),{eexpr = TLocal v},e2) when is_affected_type v.v_type ->
-			collect_modified_locals e2;
-			Hashtbl.add modified_locals v.v_id true
-		| _ ->
-			Type.iter collect_modified_locals e
-	in
+	let might_be_affected,collect_modified_locals = create_affection_checker() in
 	let had_side_effect = ref false in
 	let inlined_vars = List.map2 (fun e (v,_) ->
 		let l = local v in

tests/optimization/src/TestJs.hx

@@ -238,5 +238,55 @@ class TestJs {
 		use(i);
 		// This is not const-propagated because StringMap introduced unbound variables
 	}
+
+	@:js('
+		var x = TestJs.getInt();
+		var tmp;
+		TestJs.getInt();
+		tmp = TestJs.getInt();
+		TestJs.call([x,"foo"],tmp);
+	')
+	static function testMightBeAffected1() {
+		var x = getInt();
+		call([x, "foo"], {
+			getInt();
+			getInt();
+		});
+	}
+
+	@:js('
+		var x = TestJs.getInt();
+		var tmp = [x,"foo"];
+		var tmp1;
+		x = TestJs.getInt();
+		tmp1 = TestJs.getInt();
+		TestJs.call(tmp,tmp1);
+	')
+	static function testMightBeAffected2() {
+		var x = getInt();
+		call([x, "foo"], {
+			x = getInt();
+			getInt();
+		});
+	}
+
+	@:js('
+		var x = TestJs.getInt();
+		var tmp = x;
+		var tmp1;
+		++x;
+		tmp1 = TestJs.getInt();
+		TestJs.call(tmp,tmp1);
+	')
+	static function testMightBeAffected3() {
+		var x = getInt();
+		call(x, {
+			x++;
+			getInt();
+		});
+	}
+
+	static function getInt(?d:Dynamic) { return 1; }
+	static function call(d1:Dynamic, d2:Dynamic) { return d1; }
 	static function use<T>(t:T) { }
 }