Browse Source

Allow modification of loop var in IntIterator loop (#8581)

* allow modification of loop var in IntIterator loop

* cleanup

* don't gen loop vars in unrolled code without modifications

---------

Co-authored-by: Simon Krajewski <[email protected]>
Aleksandr Kuzmenko 4 months ago
parent
commit
5ee0c7173a

+ 10 - 17
src/typing/forLoop.ml

@@ -297,19 +297,6 @@ module IterationKind = struct
 		let get_array_length arr p =
 			mk (mk_field arr "length") ctx.com.basic.tint p
 		in
-		let check_loop_var_modification vl e =
-			let rec loop e =
-				match e.eexpr with
-				| TBinop (OpAssign,{ eexpr = TLocal l },_)
-				| TBinop (OpAssignOp _,{ eexpr = TLocal l },_)
-				| TUnop (Increment,_,{ eexpr = TLocal l })
-				| TUnop (Decrement,_,{ eexpr = TLocal l })  when List.memq l vl ->
-					raise_typing_error "Loop variable cannot be modified" e.epos
-				| _ ->
-					Type.iter loop e
-			in
-			loop e
-		in
 		let gen_int_iter e1 pt f_get f_length =
 			let index = gen_local ctx t_int v.v_pos in
 			index.v_meta <- (Meta.ForLoopVariable,[],null_pos) :: index.v_meta;
@@ -340,7 +327,6 @@ module IterationKind = struct
 		in
 		match iterator.it_kind with
 		| IteratorIntUnroll(offset,length,ascending,unroll_params) ->
-			check_loop_var_modification [v] e2;
 			if not ascending then raise_typing_error "Cannot iterate backwards" p;
 			let rec unroll acc i =
 				if i = length then
@@ -351,6 +337,8 @@ module IterationKind = struct
 					let rec loop e = match e.eexpr with
 					| TLocal v' when v == v' ->
 						{ei with epos = e.epos}
+					| TUnop ((Decrement | Increment), _, { eexpr = TLocal v' }) when v == v' -> raise Exit
+					| TBinop ((OpAssign | OpAssignOp _), { eexpr = TLocal v' }, _) when v == v' -> raise Exit
 					| TVar(v,eo) when has_var_flag v VStatic ->
 						if acc = [] then
 							local_vars := {e with eexpr = TVar(v,eo)} :: !local_vars;
@@ -358,7 +346,14 @@ module IterationKind = struct
 					| _ ->
 						map_expr loop e
 				in
-				let e2 = loop e2 in
+				let e2 = try
+					loop e2
+				with Exit ->
+					{ e2 with eexpr = TBlock (
+						(mk (TVar(v,Some ei)) t_void p) ::
+						(match e2.eexpr with TBlock el -> el | _ -> [e2])
+					)}
+				in
 				let acc = acc @ !local_vars in
 				let e2 = Texpr.duplicate_tvars e_identity e2 in
 				unroll (e2 :: acc) (i + 1)
@@ -366,7 +361,6 @@ module IterationKind = struct
 			let el = unroll [] 0 in
 			mk (TBlock el) t_void p
 		| IteratorIntConst(a,b,ascending) ->
-			check_loop_var_modification [v] e2;
 			if not ascending then raise_typing_error "Cannot iterate backwards" p;
 			let v_index = gen_local ctx t_int a.epos in
 			let evar_index = mk (TVar(v_index,Some a)) t_void a.epos in
@@ -382,7 +376,6 @@ module IterationKind = struct
 				ewhile;
 			]) t_void p
 		| IteratorInt(a,b) ->
-			check_loop_var_modification [v] e2;
 			let v_index = gen_local ctx t_int a.epos in
 			let evar_index = mk (TVar(v_index,Some a)) t_void a.epos in
 			let ev_index = make_local v_index v_index.v_pos in

+ 0 - 7
tests/misc/projects/Issue7734/Main.hx

@@ -1,7 +0,0 @@
-class Main {
-	static public function main() {
-		for (i in 0...10) {
-			i++;
-		}
-	}
-}

+ 0 - 2
tests/misc/projects/Issue7734/compile-fail.hxml

@@ -1,2 +0,0 @@
---main Main
---interp

+ 0 - 1
tests/misc/projects/Issue7734/compile-fail.hxml.stderr

@@ -1 +0,0 @@
-Main.hx:4: characters 4-7 : Loop variable cannot be modified

+ 32 - 0
tests/misc/projects/Issue8574/Main.hx

@@ -0,0 +1,32 @@
+class Main {
+	static function main() {
+		var a = [];
+		for(i in #if noOpt 0...three() #else 0...3 #end) {
+			a.push(i);
+			i++;
+			a.push(i);
+		}
+		check([0,1, 1,2, 2,3], a);
+	}
+
+	@:pure(false)
+	static public function three():Int {
+		return 3;
+	}
+
+	static function check(a1:Array<Int>, a2:Array<Int>) {
+		if(a1.length != a2.length) {
+			fail(a1, a2);
+		}
+		for(i in 0...a1.length) {
+			if(a1[i] != a2[i]) {
+				fail(a1, a2);
+			}
+		}
+	}
+
+	static function fail(a1:Array<Int>, a2:Array<Int>) {
+		Sys.stderr().writeString('Arrays are not equal: $a1 != $a2\n');
+		Sys.exit(1);
+	}
+}

+ 3 - 0
tests/misc/projects/Issue8574/constIntIterator.hxml

@@ -0,0 +1,3 @@
+-main Main
+-D loop_unroll_max_cost=0
+--interp

+ 3 - 0
tests/misc/projects/Issue8574/noOptimization.hxml

@@ -0,0 +1,3 @@
+-main Main
+-D noOpt
+--interp

+ 3 - 0
tests/misc/projects/Issue8574/unrolledLoop.hxml

@@ -0,0 +1,3 @@
+-main Main
+-D loop_unroll_max_cost=1000
+--interp