Parcourir la source

(Cpp) fix inconsistent side-effect detection

Simon Krajewski il y a 11 ans
Parent
commit
4986fb60b9
2 fichiers modifiés avec 53 ajouts et 14 suppressions
  1. 8 14
      filters.ml
  2. 45 0
      tests/unit/issues/Issue2792.hx

+ 8 - 14
filters.ml

@@ -127,23 +127,17 @@ let handle_side_effects com gen_temp e =
 				e
 			end
 		in
-		let rec no_side_effect e = match e.eexpr with
-			| TNew _ | TCall _ | TArrayDecl _ | TObjectDecl _ | TBinop ((OpAssignOp _ | OpAssign),_,_) | TUnop ((Increment|Decrement),_,_) ->
-				bind e;
-			| TIf _ | TTry _ | TSwitch _ ->
+		let rec no_side_effect e =
+			match e.eexpr with
+				| _ when Optimizer.has_side_effect e ->
+					bind e
 				(* Technically these are not side-effects, but we have to move them out anyway because their blocks code have side-effects.
 				   This also probably improves readability of the generated code. We can ignore TWhile and TFor because their type is Void,
 				   so they could never appear in a place where side-effects matter. *)
-				bind e
-			| TBinop(op,e1,e2) when Optimizer.has_side_effect e1 || Optimizer.has_side_effect e2 ->
-				bind e;
-			| TConst _ | TLocal _ | TTypeExpr _ | TFunction _
-			| TReturn _ | TBreak | TContinue | TThrow _ | TCast (_,Some _) ->
-				e
-			| TBlock _ ->
-				bind e
-			| _ ->
-				Type.map_expr no_side_effect e
+				| TIf _ | TTry _ | TSwitch _ ->
+					bind e
+				| _ ->
+					e
 		in
 		let rec loop2 acc el = match el with
 			| e :: el ->

+ 45 - 0
tests/unit/issues/Issue2792.hx

@@ -0,0 +1,45 @@
+package unit.issues;
+import unit.Test;
+
+private class Obj {
+    public var w:Float;
+    public var h:Float;
+    public function new(w:Float = 0, h:Float = 0) {
+        this.w = w;
+        this.h = h;
+    }
+}
+
+private class Node {
+    public var obj:Obj;
+
+    public function new() { }
+
+    public inline function canPlace(w:Float, h:Float):Bool {
+        return (obj.w == w && obj.h == h);
+    }
+}
+
+
+private class Root {
+    public var left:Node;
+    public var right:Node;
+
+    public function new() {}
+}
+
+class Issue2792 extends Test {
+	function test() {
+        var root:Root = new Root();
+        root.left = new Node();
+        root.left.obj = new Obj();
+        var check1:Bool = (root.right != null && root.right.canPlace(0, 0)) && (root.left != null && root.left.canPlace(0, 0));
+        eq(false, check1);
+        var check2:Bool = (root.left != null && root.left.canPlace(0, 0)) && (root.right != null && root.right.canPlace(0, 0));
+        eq(false, check2);
+        var right:Bool = (root.right != null && root.right.canPlace(0, 0));
+        var left:Bool = (root.left != null && root.left.canPlace(0, 0));
+        var both:Bool = right && left;
+        eq(false, both);
+	}
+}