[typer] clean up temp vars when typing operators

closes #9778

src/typing/operators.ml

@@ -21,19 +21,27 @@ object(self)
 		DynArray.add vars (v,e);
 		mk (TLocal v) v.v_type v.v_pos
 
+	method private get_expr_aux depth name e =
+		let rec loop depth name e = match (Texpr.skip e).eexpr with
+			| TLocal _ | TTypeExpr _ | TConst _ ->
+				e
+			| TField(ef,fa) when depth = 0 ->
+				let ef = loop (depth + 1) "fh" ef in
+				{e with eexpr = TField(ef,fa)}
+			| TArray(e1,e2) when depth = 0 ->
+				let e1 = loop (depth + 1) "base" e1 in
+				let e2 = loop (depth + 1) "index" e2 in
+				{e with eexpr = TArray(e1,e2)}
+			| _ ->
+				self#as_var name e
+		in
+		loop depth name e
+
 	method get_expr name e =
-		match (Texpr.skip e).eexpr with
-		| TLocal _ | TTypeExpr _ | TConst _ ->
-			e
-		| TField(ef,fa) ->
-			let ef = self#get_expr "fh" ef in
-			{e with eexpr = TField(ef,fa)}
-		| TArray(e1,e2) ->
-			let e1 = self#get_expr "base" e1 in
-			let e2 = self#get_expr "index" e2 in
-			{e with eexpr = TArray(e1,e2)}
-		| _ ->
-			self#as_var name e
+		self#get_expr_aux 0 name e
+
+	method get_expr_part name e =
+		self#get_expr_aux 1 name e
 
 	method to_texpr e =
 		begin match self#get_vars with
@@ -643,7 +651,6 @@ let process_lhs_expr ctx name e_lhs =
 	let e = vr#get_expr name e_lhs in
 	e,vr
 
-
 let type_assign_op ctx op e1 e2 with_type p =
 	let field_rhs_by_name op name ev with_type =
 		let access_get = type_field_default_cfg ctx ev name p MGet with_type in
@@ -691,7 +698,8 @@ let type_assign_op ctx op e1 e2 with_type p =
 		let e_rhs = type_binop2 ctx op e e2 true (WithType.with_type e.etype) p in
 		assign vr e e_rhs
 	| AKAccessor fa ->
-		let ef,vr = process_lhs_expr ctx "fh" fa.fa_on in
+		let vr = new value_reference ctx in
+		let ef = vr#get_expr_part "fh" fa.fa_on in
 		let t_lhs,e_rhs = field_rhs op fa.fa_field ef in
 		set vr {fa with fa_on = ef} t_lhs e_rhs []
 	| AKUsingAccessor sea ->
@@ -760,7 +768,7 @@ let type_binop ctx op e1 e2 is_assign_op with_type p =
 	| _ ->
 		type_non_assign_op ctx op e1 e2 is_assign_op false with_type p
 
-let type_unop ctx op flag e p =
+let type_unop ctx op flag e with_type p =
 	let try_abstract_unop_overloads e = match follow e.etype with
 		| TAbstract ({a_impl = Some c} as a,tl) ->
 			let rec loop opl = match opl with
@@ -832,11 +840,17 @@ let type_unop ctx op flag e p =
 	| Increment | Decrement ->
 		let binop = if op = Increment then OpAdd else OpSub in
 		let e_one = mk (TConst (TInt Int32.one)) ctx.t.tint p in
+		let maybe_tempvar_postfix vr e_lhs =
+			if flag = Postfix && with_type <> WithType.no_value then begin
+				let e_lhs = vr#get_expr "lhs" e_lhs in
+				e_lhs,Some (vr#as_var "postfix" e_lhs)
+			end else
+				e_lhs,None
+		in
 		let read_on vr ef fa =
 			let access_get = type_field_default_cfg ctx ef fa.fa_field.cf_name p MGet WithType.value in
 			let e_lhs = acc_get ctx access_get p in
-			let e_lhs = vr#get_expr "lhs" e_lhs in
-			let e_out = if flag = Prefix then None else Some (vr#as_var "postfix" e_lhs) in
+			let e_lhs,e_out = maybe_tempvar_postfix vr e_lhs in
 			e_lhs,e_out
 		in
 		let generate vr e_out e = match e_out with
@@ -850,7 +864,8 @@ let type_unop ctx op flag e p =
 		| AKExpr e ->
 			find_overload_or_make e
 		| AKField fa ->
-			let ef,vr = process_lhs_expr ctx "fh" fa.fa_on in
+			let vr = new value_reference ctx in
+			let ef = vr#get_expr_part "fh" fa.fa_on in
 			let access_get = type_field_default_cfg ctx ef fa.fa_field.cf_name p MGet WithType.value in
 			let e,e_out = match access_get with
 			| AKField _ ->
@@ -859,14 +874,14 @@ let type_unop ctx op flag e p =
 			| _ ->
 				let e_set = FieldAccess.get_field_expr {fa with fa_on = ef} FWrite in
 				let e_lhs = acc_get ctx access_get p in
-				let e_lhs = vr#get_expr "lhs" e_lhs in
-				let e_out = if flag = Prefix then None else Some (vr#as_var "postfix" e_lhs) in
+				let e_lhs,e_out = maybe_tempvar_postfix vr e_lhs in
 				let e_op = mk (TBinop(binop,e_lhs,e_one)) e_lhs.etype p in
 				mk (TBinop(OpAssign,e_set,e_op)) e_set.etype p,e_out
 			in
 			generate vr e_out e
 		| AKAccessor fa ->
-			let ef,vr = process_lhs_expr ctx "fh" fa.fa_on in
+			let vr = new value_reference ctx in
+			let ef = vr#get_expr_part "fh" fa.fa_on in
 			let fa = {fa with fa_on = ef} in
 			let e_lhs,e_out = read_on vr ef fa in
 			let e_op = mk (TBinop(binop,e_lhs,e_one)) e_lhs.etype p in

src/typing/typer.ml

@@ -1775,7 +1775,7 @@ and type_expr ?(mode=MGet) ctx (e,p) (with_type:WithType.t) =
 	| ENew (t,el) ->
 		type_new ctx t el with_type false p
 	| EUnop (op,flag,e) ->
-		type_unop ctx op flag e p
+		type_unop ctx op flag e with_type p
 	| EFunction (kind,f) ->
 		type_local_function ctx kind f with_type p
 	| EUntyped e ->

tests/optimization/src/issues/Issue9778.hx

@@ -0,0 +1,166 @@
+package issues;
+
+class Issue9778Class {
+	@:isVar public var x(get, set):Float;
+	public var y(get, default):Float;
+	public var z(default, set):Float;
+
+	function get_x()
+		return x;
+
+	function set_x(v:Float)
+		return x = v;
+
+	function get_y()
+		return y;
+
+	function set_z(v:Float) {
+		return z = v;
+	}
+
+	public var next:Issue9778Class;
+
+	public function new() {}
+}
+
+/*
+	The purpose of this test is to test what kind of output the typer produces, which is why we have
+	@:analyzer(ignore) here.
+*/
+@:analyzer(ignore)
+class Issue9778 {
+	@:js("
+		var c = new issues_Issue9778Class();
+		c.set_x(c.get_x() + 1);
+		c.y = c.get_y() + 1;"
+	)
+	static function testUnnecessaryPostfix() {
+		var c = new Issue9778Class();
+		c.x++;
+		c.y++;
+	}
+
+	@:js('
+		var c = new issues_Issue9778Class();
+		c.set_x(c.get_x() + 1);
+		c.y += 1;
+		c.set_z(c.z + 1);
+		c.set_x(c.get_x() + 1);
+		c.y = c.get_y() + 1;
+		c.set_z(c.z + 1);
+		c.set_x(c.get_x() + 1);
+		c.y = c.get_y() + 1;
+		c.set_z(c.z + 1);
+	')
+	static function testLhsTempvarDepth0NoValue() {
+		var c = new Issue9778Class();
+		c.x += 1;
+		c.y += 1;
+		c.z += 1;
+		++c.x;
+		++c.y;
+		++c.z;
+		c.x++;
+		c.y++;
+		c.z++;
+	}
+
+	@:js('
+		var c = new issues_Issue9778Class();
+		c.next = c;
+		var fh = c.next;
+		fh.set_x(fh.get_x() + 1);
+		var fh = c.next;
+		fh.y += 1;
+		var fh = c.next;
+		fh.set_z(fh.z + 1);
+		var fh = c.next;
+		fh.set_x(fh.get_x() + 1);
+		var fh = c.next;
+		fh.y = fh.get_y() + 1;
+		var fh = c.next;
+		fh.set_z(fh.z + 1);
+		var fh = c.next;
+		fh.set_x(fh.get_x() + 1);
+		var fh = c.next;
+		fh.y = fh.get_y() + 1;
+		var fh = c.next;
+		fh.set_z(fh.z + 1);
+	')
+	static function testLhsTempvarDepth1NoValue() {
+		var c = new Issue9778Class();
+		c.next = c;
+		c.next.x += 1;
+		c.next.y += 1;
+		c.next.z += 1;
+		++c.next.x;
+		++c.next.y;
+		++c.next.z;
+		c.next.x++;
+		c.next.y++;
+		c.next.z++;
+	}
+
+	@:js('
+		var c = new issues_Issue9778Class();
+		issues_Issue9778.use(c.set_x(c.get_x() + 1));
+		issues_Issue9778.use(c.y += 1);
+		issues_Issue9778.use(c.set_z(c.z + 1));
+		issues_Issue9778.use(c.set_x(c.get_x() + 1));
+		issues_Issue9778.use(c.y = c.get_y() + 1);
+		issues_Issue9778.use(c.set_z(c.z + 1));
+	')
+	static function testLhsTempvarDepth0Value() {
+		var c = new Issue9778Class();
+		use(c.x += 1);
+		use(c.y += 1);
+		use(c.z += 1);
+		use(++c.x);
+		use(++c.y);
+		use(++c.z);
+		/*
+			Postfix in value-places generates some nasty code with @:analyzer(ignore) which I don't want to add as a test:
+			issues_Issue9778.use((function($this) {var $r;var lhs = c.get_x();var postfix = lhs;c.set_x(lhs + 1);$r = postfix;return $r;}(this)));
+			issues_Issue9778.use((function($this) {var $r;var lhs = c.get_y();var postfix = lhs;c.y = lhs + 1;$r = postfix;return $r;}(this)));
+			issues_Issue9778.use((function($this) {var $r;var postfix = c.z;c.set_z(c.z + 1);$r = postfix;return $r;}(this)));
+		*/
+		// use(c.x++);
+		// use(c.y++);
+		// use(c.z++);
+	}
+
+	/*
+		This is consistent but really ugly, so we shouldn't assert the exact output.
+
+	@:js('
+		var c = new issues_Issue9778Class();
+		c.next = c;
+		issues_Issue9778.use((function($this) {var $r;var fh = c.next;$r = fh.set_x(fh.get_x() + 1);return $r;}(this)));
+		issues_Issue9778.use((function($this) {var $r;var fh = c.next;$r = fh.y += 1;return $r;}(this)));
+		issues_Issue9778.use((function($this) {var $r;var fh = c.next;$r = fh.set_z(fh.z + 1);return $r;}(this)));
+		issues_Issue9778.use((function($this) {var $r;var fh = c.next;$r = fh.set_x(fh.get_x() + 1);return $r;}(this)));
+		issues_Issue9778.use((function($this) {var $r;var fh = c.next;$r = fh.y = fh.get_y() + 1;return $r;}(this)));
+		issues_Issue9778.use((function($this) {var $r;var fh = c.next;$r = fh.set_z(fh.z + 1);return $r;}(this)));
+		issues_Issue9778.use((function($this) {var $r;var fh = c.next;var lhs = fh.get_x();var postfix = lhs;fh.set_x(lhs + 1);$r = postfix;return $r;}(this)));
+		issues_Issue9778.use((function($this) {var $r;var fh = c.next;var lhs = fh.get_y();var postfix = lhs;fh.y = lhs + 1;$r = postfix;return $r;}(this)));
+		issues_Issue9778.use((function($this) {var $r;var fh = c.next;var postfix = fh.z;fh.set_z(fh.z + 1);$r = postfix;return $r;}(this)));
+	')
+	@:analyzer(ignore)
+	static function testLhsTempvarDepth1Value() {
+		var c = new Issue9778Class();
+		c.next = c;
+		use(c.next.x += 1);
+		use(c.next.y += 1);
+		use(c.next.z += 1);
+		use(++c.next.x);
+		use(++c.next.y);
+		use(++c.next.z);
+		use(c.next.x++);
+		use(c.next.y++);
+		use(c.next.z++);
+	}
+	*/
+
+	@:pure(false)
+	static public function use<T>(t:T) { return t; }
+}