[js] avoid shadowing catch vars (closes #9617)

Author: RealyUniqueName

extra/CHANGES.txt

@@ -17,6 +17,7 @@
 	eval : fixed exception message when catching compiler-generated `haxe.macro.Error` as `Dynamic` (#9600)
 	lua : fixed lua code generation without `--main` compilation argument (#9489)
 	php : added an overload signature for `session_set_cookie_params` function (#9507)
+	js : fixed name collisions for catch variables to avoid closure compiler errors (#9617)
 	nullsafety : fixed various scenarios of `if..else` branching (#9474)
 
 2020-22-05 4.1.1

src/context/common.ml

@@ -107,6 +107,10 @@ type var_scoping_flags =
 		It's not allowed to shadow existing variables in a scope.
 	*)
 	| NoShadowing
+	(**
+		It's not allowed to shadow a `catch` variable.
+	*)
+	| NoCatchVarShadowing
 	(**
 		Local vars cannot have the same name as the current top-level package or
 		(if in the root package) current class name
@@ -416,7 +420,7 @@ let get_config com =
 				vs_scope = if es6 then BlockScope else FunctionScope;
 				vs_flags =
 					(if defined Define.JsUnflatten then ReserveAllTopLevelSymbols else ReserveAllTypesFlat)
-					:: if es6 then [NoShadowing; SwitchCasesNoBlocks;] else [VarHoisting];
+					:: if es6 then [NoShadowing; SwitchCasesNoBlocks;] else [VarHoisting; NoCatchVarShadowing];
 			}
 		}
 	| Lua ->

src/filters/renameVars.ml

@@ -8,6 +8,7 @@ type rename_init = {
 	mutable ri_scope : var_scope;
 	mutable ri_hoisting : bool;
 	mutable ri_no_shadowing : bool;
+	mutable ri_no_catch_var_shadowing : bool;
 	mutable ri_switch_cases_no_blocks : bool;
 	mutable ri_reserved : bool StringMap.t;
 	mutable ri_reserve_current_top_level_symbol : bool;
@@ -50,6 +51,7 @@ let init com =
 		ri_reserved = StringMap.empty;
 		ri_hoisting = false;
 		ri_no_shadowing = false;
+		ri_no_catch_var_shadowing = false;
 		ri_switch_cases_no_blocks = false;
 		ri_reserve_current_top_level_symbol = false;
 	} in
@@ -60,6 +62,8 @@ let init com =
 			ri.ri_hoisting <- true;
 		| NoShadowing ->
 			ri.ri_no_shadowing <- true;
+		| NoCatchVarShadowing ->
+			ri.ri_no_catch_var_shadowing <- true;
 		| SwitchCasesNoBlocks ->
 			ri.ri_switch_cases_no_blocks <- true;
 		| ReserveNames names ->
@@ -101,6 +105,7 @@ type scope = {
 type rename_context = {
 	rc_hoisting : bool;
 	rc_no_shadowing : bool;
+	rc_no_catch_var_shadowing : bool;
 	rc_switch_cases_no_blocks : bool;
 	rc_scope : var_scope;
 	mutable rc_reserved : bool StringMap.t;
@@ -223,7 +228,8 @@ let rec collect_vars ?(in_block=false) rc scope e =
 				| TBlock exprs -> { catch_expr with eexpr = TBlock (v_expr :: exprs) }
 				| _ -> { catch_expr with eexpr = TBlock [v_expr; catch_expr] }
 			in
-			collect_vars scope e
+			collect_vars scope e;
+			if rc.rc_no_catch_var_shadowing then use_var rc scope v;
 		) catches
 	| TSwitch (target, cases, default_opt) when rc.rc_switch_cases_no_blocks ->
 		collect_vars scope target;
@@ -307,6 +313,7 @@ let run ctx ri e =
 			rc_scope = ri.ri_scope;
 			rc_hoisting = ri.ri_hoisting;
 			rc_no_shadowing = ri.ri_no_shadowing;
+			rc_no_catch_var_shadowing = ri.ri_no_catch_var_shadowing;
 			rc_switch_cases_no_blocks = ri.ri_switch_cases_no_blocks;
 			rc_reserved = ri.ri_reserved;
 		} in

tests/optimization/src/issues/Issue9617.hx

@@ -0,0 +1,34 @@
+package issues;
+
+class Issue9617 {
+	@:js('
+		try {
+			issues_Issue9617.dummy();
+		} catch( _g ) {
+			var _g1 = haxe_Exception.caught(_g);
+			var e = _g1;
+			var _g2 = issues_Issue9617.process(_g1);
+			if(_g2 != null) {
+				var e = _g2;
+				issues_Issue9617.dummy();
+			}
+		}
+	')
+	@:analyzer(no_local_dce)
+	static public function test() {
+		try {
+			dummy();
+		} catch(e) {
+			switch process(e) {
+				case null:
+				case e: dummy();
+			}
+		}
+	}
+
+	@:pure(false) static function process(e) {
+		return e;
+	}
+
+	@:pure(false) static function dummy() {}
+}