Browse Source

Fix bad optimizations when comparing null to basic types (#12442)

* deal with null == basic in optimize_binop

closes #12435

* also test straightforward propagation

* add missing type-hint

* don't actually try to optimize this, let the target deal with it

* restrict to static targets

* don't propagate null-values into non-nullable places

* avoid cppia failure because I don't know how to fix it

* disable the right thing
Simon Krajewski 3 days ago
parent
commit
ea8d4dc242

+ 8 - 4
src/optimization/analyzer.ml

@@ -429,8 +429,12 @@ module ConstPropagationImpl = struct
 			| TLocal v ->
 				if (follow v.v_type) == t_dynamic || has_var_flag v VCaptured then
 					Bottom
-				else
-					get_cell ctx v.v_id
+				else begin match get_cell ctx v.v_id with
+					| Null _ when not (is_nullable v.v_type) ->
+						Bottom
+					| ct ->
+						ct
+				end
 			| TBinop(OpAssign,_,e2) ->
 				eval bb e2
 			| TBinop(op,e1,e2) ->
@@ -439,7 +443,7 @@ module ConstPropagationImpl = struct
 				let e1 = wrap cl1 in
 				let e2 = wrap cl2 in
 				let e = {e with eexpr = TBinop(op,e1,e2)} in
-				let e' = optimize_binop e op e1 e2 in
+				let e' = optimize_binop actx.com e op e1 e2 in
 				if e != e' then
 					eval bb e'
 				else
@@ -1122,7 +1126,7 @@ module Run = struct
 			in
 			begin try
 				let e = run_on_expr actx e in
-				let e = reduce_control_flow com.platform e in
+				let e = reduce_control_flow com e in
 				maybe_debug();
 				cf.cf_expr <- Some e;
 			with

+ 7 - 7
src/optimization/analyzerTexpr.ml

@@ -1005,18 +1005,18 @@ module Fusion = struct
 end
 
 module Cleanup = struct
-	let apply com e =
+	let apply scom e =
 		let if_or_op e e1 e2 e3 = match (Texpr.skip e1).eexpr,(Texpr.skip e2).eexpr,(Texpr.skip e3).eexpr with
 			| _,TReturn(Some b),TReturn(Some {eexpr = TConst (TBool false)}) ->
-				let binop = optimize_binop {e with eexpr = TBinop(OpBoolAnd,e1,b)} OpBoolAnd e1 b in
+				let binop = optimize_binop scom {e with eexpr = TBinop(OpBoolAnd,e1,b)} OpBoolAnd e1 b in
 				{e with eexpr = TReturn(Some binop)}
 			| TUnop(Not,Prefix,e1),TReturn(Some b),TReturn(Some {eexpr = TConst (TBool true)}) ->
-				let binop = optimize_binop {e with eexpr = TBinop(OpBoolOr,e1,b)} OpBoolOr e1 b in
+				let binop = optimize_binop scom {e with eexpr = TBinop(OpBoolOr,e1,b)} OpBoolOr e1 b in
 				{e with eexpr = TReturn(Some binop)}
 			| TUnop(Not,Prefix,e1),_,TConst (TBool true) ->
-				optimize_binop {e with eexpr = TBinop(OpBoolOr,e1,e2)} OpBoolOr e1 e2
+				optimize_binop scom {e with eexpr = TBinop(OpBoolOr,e1,e2)} OpBoolOr e1 e2
 			| _,_,TConst (TBool false) ->
-				optimize_binop {e with eexpr = TBinop(OpBoolAnd,e1,e2)} OpBoolAnd e1 e2
+				optimize_binop scom {e with eexpr = TBinop(OpBoolAnd,e1,e2)} OpBoolAnd e1 e2
 			| _,_,TBlock [] ->
 				{e with eexpr = TIf(e1,e2,None)}
 			| _ -> match (Texpr.skip e2).eexpr with
@@ -1041,7 +1041,7 @@ module Cleanup = struct
 				let el = List.map (fun e ->
 					let e = loop e in
 					match e.eexpr with
-					| TIf _ -> {e with etype = com.basic.tvoid}
+					| TIf _ -> {e with etype = scom.basic.tvoid}
 					| _ -> e
 				) el in
 				{e with eexpr = TBlock el}
@@ -1076,7 +1076,7 @@ module Cleanup = struct
 						in
 						let rec loop2 el = match el with
 							| [{eexpr = TBreak}] when is_true_expr e1 && not has_continue ->
-								do_while := Some (Texpr.Builder.make_bool com.basic true e1.epos);
+								do_while := Some (Texpr.Builder.make_bool scom.basic true e1.epos);
 								[]
 							| [{eexpr = TIf(econd,{eexpr = TBlock[{eexpr = TBreak}]},None)}] when is_true_expr e1 && not (references_local econd) && not has_continue ->
 								do_while := Some econd;

+ 4 - 4
src/optimization/optimizer.ml

@@ -95,7 +95,7 @@ in
 			else
 				None
 
-let reduce_control_flow platform e = match e.eexpr with
+let reduce_control_flow scom e = match e.eexpr with
 	| TIf ({ eexpr = TConst (TBool t) },e1,e2) ->
 		(if t then e1 else match e2 with None -> { e with eexpr = TBlock [] } | Some e -> e)
 	| TWhile ({ eexpr = TConst (TBool false) },sub,flag) ->
@@ -108,7 +108,7 @@ let reduce_control_flow platform e = match e.eexpr with
 		| None -> e
 		end
 	| TBinop (op,e1,e2) ->
-		optimize_binop e op e1 e2
+		optimize_binop scom e op e1 e2
 	| TUnop (op,flag,esub) ->
 		optimize_unop e op flag esub
 	| TCall ({ eexpr = TField (o,FClosure (c,cf)) } as f,el) ->
@@ -120,7 +120,7 @@ let reduce_control_flow platform e = match e.eexpr with
 		(try List.nth el i with Failure _ -> e)
 	| TCast(e1,None) ->
 		(* TODO: figure out what's wrong with these targets *)
-		let require_cast = match platform with
+		let require_cast = match scom.platform with
 			| Cpp | Flash -> true
 			| Jvm -> true
 			| _ -> false
@@ -167,7 +167,7 @@ let rec reduce_loop (scom : SafeCom.t) stack e =
 				reduce_expr scom e
 		end
 	| _ ->
-		reduce_expr scom (reduce_control_flow scom.platform e))
+		reduce_expr scom (reduce_control_flow scom e))
 
 let reduce_expression scom e =
 	if scom.foptimize then begin

+ 2 - 1
src/optimization/optimizerTexpr.ml

@@ -69,7 +69,7 @@ let create_affection_checker () =
 	in
 	might_be_affected,collect_modified_locals
 
-let optimize_binop e op e1 e2 =
+let optimize_binop scom e op e1 e2 =
 	let is_float t =
 		match follow t with
 		| TAbstract({ a_path = [],"Float" },_) -> true
@@ -185,6 +185,7 @@ let optimize_binop e op e1 e2 =
 		in
 		(match a, b with
 		| TInt a, TFloat b | TFloat b, TInt a -> ebool (Int32.to_float a = float_of_string b)
+		| TNull, (TInt _ | TFloat _ | TBool _) | (TInt _ | TFloat _ | TBool _), TNull when scom.SafeCom.platform_config.pf_static -> e
 		| _ -> ebool (a = b))
 	| TConst (TBool a), _ ->
 		(match op with

+ 35 - 0
tests/unit/src/unit/issues/Issue12435.hx

@@ -0,0 +1,35 @@
+package unit.issues;
+
+class Issue12435 extends Test {
+	function testCast() {
+		var i:Int = cast null;
+		#if static
+		#if !cppia // TODO
+		t(i == 0);
+		#end
+		t(i == cast null);
+		#else
+		f(i == 0);
+		t(i == null);
+		#end
+	}
+
+	function testPropagation() {
+		var Null:Null<Int> = null;
+		var i:Int = Null;
+		#if static
+		t(i == 0);
+		f(i == Null);
+		#else
+		f(i == 0);
+		t(i == null);
+		#end
+	}
+
+	function testPropagationNullable() {
+		var Null:Null<Int> = null;
+		var i:Null<Int> = Null;
+		f(i == 0);
+		t(i == null);
+	}
+}