Browse Source

Local var renaming improvements (#9045)

* let's see how our targets behave

see #9035

* that wasn't quite it

* add platform config

* update optimization test

* Php is "Nested"
Simon Krajewski 5 years ago
parent
commit
5b849caa08
3 changed files with 50 additions and 14 deletions
  1. 18 4
      src/context/common.ml
  2. 30 8
      src/filters/filters.ml
  3. 2 2
      tests/optimization/src/issues/Issue6715.hx

+ 18 - 4
src/context/common.ml

@@ -94,6 +94,14 @@ type exceptions_config = {
 	ec_base_throw : path;
 }
 
+type nested_function_scoping =
+	(** each TFunction has its own scope in the output **)
+	| Independent
+	(** TFunction nodes are nested in the output **)
+	| Nested
+	(** TFunction nodes are nested in the output and there is var hoisting **)
+	| Hoisted
+
 type platform_config = {
 	(** has a static type system, with not-nullable basic types (Int/Float/Bool) *)
 	pf_static : bool;
@@ -123,6 +131,8 @@ type platform_config = {
 	pf_supports_unicode : bool;
 	(** exceptions handling config **)
 	pf_exceptions : exceptions_config;
+	(** the scoping behavior of nested functions **)
+	pf_nested_function_scoping : nested_function_scoping;
 }
 
 class compiler_callbacks = object(self)
@@ -342,7 +352,8 @@ let default_config =
 			ec_native_catches = [];
 			ec_wildcard_catch = ([],"Dynamic");
 			ec_base_throw = ([],"Dynamic");
-		}
+		};
+		pf_nested_function_scoping = Independent;
 	}
 
 let get_config com =
@@ -366,7 +377,8 @@ let get_config com =
 				ec_native_catches = [];
 				ec_wildcard_catch = ([],"Dynamic");
 				ec_base_throw = ([],"Dynamic");
-			}
+			};
+			pf_nested_function_scoping = Hoisted;
 		}
 	| Lua ->
 		{
@@ -420,7 +432,8 @@ let get_config com =
 				];
 				ec_wildcard_catch = (["php"],"Throwable");
 				ec_base_throw = (["php"],"Throwable");
-			}
+			};
+			pf_nested_function_scoping = Nested;
 		}
 	| Cpp ->
 		{
@@ -487,7 +500,8 @@ let get_config com =
 				];
 				ec_wildcard_catch = ["python";"Exceptions"],"BaseException";
 				ec_base_throw = ["python";"Exceptions"],"BaseException";
-			}
+			};
+			pf_nested_function_scoping = Nested;
 		}
 	| Hl ->
 		{

+ 30 - 8
src/filters/filters.ml

@@ -217,7 +217,8 @@ let collect_reserved_local_names com =
 		!h
 	| _ -> StringMap.empty
 
-let rename_local_vars ctx reserved e =
+let rec rename_local_vars_aux ctx reserved e =
+	let initial_reserved = reserved in
 	let vars = ref [] in
 	let declare v =
 		vars := v :: !vars
@@ -238,6 +239,7 @@ let rename_local_vars ctx reserved e =
 		| TAbstract (a,_) -> check (TAbstractDecl a)
 		| TMono _ | TLazy _ | TAnon _ | TDynamic _ | TFun _ -> ()
 	in
+	let funcs = ref [] in
 	let rec collect e = match e.eexpr with
  		| TVar(v,eo) ->
 			declare v;
@@ -254,8 +256,13 @@ let rename_local_vars ctx reserved e =
 				collect e
 			) catches
 		| TFunction tf ->
-			List.iter (fun (v,_) -> declare v) tf.tf_args;
-			collect tf.tf_expr
+			begin match ctx.com.config.pf_nested_function_scoping with
+			| Hoisted ->
+				funcs := tf :: !funcs;
+			| Nested | Independent ->
+				List.iter (fun (v,_) -> declare v) tf.tf_args;
+				collect tf.tf_expr
+			end
 		| TTypeExpr t ->
 			check t
 		| TNew (c,_,_) ->
@@ -270,11 +277,6 @@ let rename_local_vars ctx reserved e =
 			Type.iter collect e
 	in
 	(* Pass 1: Collect used identifiers and variables. *)
-	reserve "this";
-	if ctx.com.platform = Java then reserve "_";
-	begin match ctx.curclass.cl_path with
-		| s :: _,_ | [],s -> reserve s
-	end;
 	collect e;
 	(* Pass 2: Check and rename variables. *)
 	let count_table = Hashtbl.create 0 in
@@ -294,6 +296,26 @@ let rename_local_vars ctx reserved e =
 		v.v_name <- !name;
 	in
 	List.iter maybe_rename (List.rev !vars);
+	(* Pass 3: Recurse into nested functions. *)
+	List.iter (fun tf ->
+		reserved := initial_reserved;
+		List.iter (fun (v,_) ->
+			maybe_rename v;
+		) tf.tf_args;
+		ignore(rename_local_vars_aux ctx !reserved tf.tf_expr);
+	) !funcs
+
+let rename_local_vars ctx reserved e =
+	let reserved = ref reserved in
+	let reserve name =
+		reserved := StringMap.add name true !reserved
+	in
+	reserve "this";
+	if ctx.com.platform = Java then reserve "_";
+	begin match ctx.curclass.cl_path with
+		| s :: _,_ | [],s -> reserve s
+	end;
+	rename_local_vars_aux ctx !reserved e;
 	e
 
 let mark_switch_break_loops e =

+ 2 - 2
tests/optimization/src/issues/Issue6715.hx

@@ -9,12 +9,12 @@ class Issue6715 {
 		issues_Issue6715.x = f;
 		issues_Issue6715.x = f;
 		issues_Issue6715.x = f;
+		var x = 1;
+		x = 2;
 		var x1 = 1;
 		x1 = 2;
 		var x2 = 1;
 		x2 = 2;
-		var x3 = 1;
-		x3 = 2;
 	')
 	@:analyzer(no_local_dce)
 	static public function test1() {