[hl/eval/neko] Fix exception stack when wrapping native exceptions (#…

…11249)

* [tests] add test for #11247

* Fix for #11247

* [hl] restore callstack fix

* [tests] cpp doesn't like native exceptions in those tests either

* [tests] only run new test for eval/hl/neko

* Fix #11265

* [tests] don't compile test on target that won't run it

src/filters/exceptions.ml

@@ -480,9 +480,11 @@ let catch_native ctx catches t p =
 			)
 		(* Haxe-specific wildcard catches should go to if-fest because they need additional handling *)
 		| (v,_) :: _ when is_haxe_wildcard_catch ctx v.v_type ->
-			(match handle_as_value_exception with
-			| [] ->
+			(match handle_as_value_exception, value_exception_catch with
+			| [], None ->
 				catches_to_ifs ctx catches t p
+			| [], Some catch ->
+				catches_to_ifs ctx [catch] t p
 			| _ ->
 				catches_as_value_exception ctx handle_as_value_exception None t p
 				:: catches_to_ifs ctx catches t p

std/cpp/_std/haxe/Exception.hx

@@ -19,7 +19,10 @@ class Exception {
 		if(Std.isOfType(value, Exception)) {
 			return value;
 		} else {
-			return new ValueException(value, null, value);
+			var e = new ValueException(value, null, value);
+			// Undo automatic __shiftStack()
+			e.__unshiftStack();
+			return e;
 		}
 	}
 
@@ -63,6 +66,12 @@ class Exception {
 		__skipStack++;
 	}
 
+	@:noCompletion
+	@:ifFeature("haxe.Exception.get_stack")
+	inline function __unshiftStack():Void {
+		__skipStack--;
+	}
+
 	function get_message():String {
 		return __exceptionMessage;
 	}

std/eval/_std/haxe/Exception.hx

@@ -18,7 +18,10 @@ class Exception {
 		if(Std.isOfType(value, Exception)) {
 			return value;
 		} else {
-			return new ValueException(value, null, value);
+			var e = new ValueException(value, null, value);
+			// Undo automatic __shiftStack()
+			e.__unshiftStack();
+			return e;
 		}
 	}
 
@@ -63,6 +66,12 @@ class Exception {
 		__skipStack++;
 	}
 
+	@:noCompletion
+	@:ifFeature("haxe.Exception.get_stack")
+	inline function __unshiftStack():Void {
+		__skipStack--;
+	}
+
 	function get_message():String {
 		return __exceptionMessage;
 	}

std/hl/_std/haxe/Exception.hx

@@ -18,7 +18,10 @@ class Exception {
 		if(Std.isOfType(value, Exception)) {
 			return value;
 		} else {
-			return new ValueException(value, null, value);
+			var e = new ValueException(value, null, value);
+			// Undo automatic __shiftStack()
+			e.__unshiftStack();
+			return e;
 		}
 	}
 
@@ -62,6 +65,12 @@ class Exception {
 		__skipStack++;
 	}
 
+	@:noCompletion
+	@:ifFeature("haxe.Exception.get_stack")
+	inline function __unshiftStack():Void {
+		__skipStack--;
+	}
+
 	function get_message():String {
 		return __exceptionMessage;
 	}

std/hl/_std/haxe/NativeStackTrace.hx

@@ -29,7 +29,7 @@ class NativeStackTrace {
 		var count = callStackRaw(null);
 		var arr = new NativeArray(count);
 		callStackRaw(arr);
-		return arr;
+		return arr.sub(1, arr.length - 1);
 	}
 
 	@:hlNative("std", "exception_stack_raw")

std/neko/_std/haxe/Exception.hx

@@ -18,7 +18,10 @@ class Exception {
 		if(Std.isOfType(value, Exception)) {
 			return value;
 		} else {
-			return new ValueException(value, null, value);
+			var e = new ValueException(value, null, value);
+			// Undo automatic __shiftStack()
+			e.__unshiftStack();
+			return e;
 		}
 	}
 
@@ -63,6 +66,12 @@ class Exception {
 		__skipStack++;
 	}
 
+	@:noCompletion
+	@:ifFeature("haxe.Exception.get_stack")
+	inline function __unshiftStack():Void {
+		__skipStack--;
+	}
+
 	function get_message():String {
 		return __exceptionMessage;
 	}

tests/unit/src/unit/TestExceptions.hx

@@ -1,4 +1,4 @@
-package unit;
+package unit;
 
 import haxe.Exception;
 import haxe.exceptions.ArgumentException;
@@ -235,17 +235,16 @@ class TestExceptions extends Test {
 	public function testExceptionStack() {
 		var data = [
 			'_without_ throws' => stacksWithoutThrowLevel1(),
-			'_with_ throws' => stacksWithThrowLevel1()
+			'_with_ throws' => stacksWithThrowLevel1(),
+			#if (eval || hl || neko)
+			'auto wrapped' => stacksAutoWrappedLevel1()
+			#end
 		];
 		for(label => stacks in data) {
 			Assert.isTrue(stacks.length > 1, '$label: wrong stacks.length');
 			var expected = null;
 			var lineShift = 0;
 			for(s in stacks) {
-				// TODO: fix hl vs other targets difference with callstacks
-				// See https://github.com/HaxeFoundation/haxe/issues/10926
-				#if hl @:privateAccess s.asArray().shift(); #end
-
 				if(expected == null) {
 					expected = stackItemData(s[0]);
 				} else {
@@ -295,6 +294,24 @@ class TestExceptions extends Test {
 		return result;
 	}
 
+	#if (eval || hl || neko)
+	function stacksAutoWrappedLevel1() {
+		return stacksAutoWrappedLevel2();
+	}
+
+	function stacksAutoWrappedLevel2():Array<CallStack> {
+		@:pure(false) function wrapNativeError(_) return [];
+
+		var result:Array<CallStack> = [];
+		// It's critical for `testExceptionStack` test to keep the following lines
+		// order with no additional code in between.
+		result.push(try throw new Exception('') catch(e:Exception) e.stack);
+		result.push(try throw "" catch(e:Exception) e.stack);
+		result.push(try wrapNativeError((null:String).length) catch(e:Exception) e.stack);
+		return result;
+	}
+	#end
+
 	function stackItemData(item:StackItem):ItemData {
 		var result:ItemData = {};
 		switch item {
@@ -321,6 +338,16 @@ class TestExceptions extends Test {
 		eq('haxe.Exception', HelperMacros.typeString(try throw new Exception('') catch(e) e));
 	}
 
+	function testCatchValueException() {
+		try {
+			throw "";
+		} catch(e:ValueException) {
+			Assert.pass();
+		} catch(e) {
+			Assert.fail();
+		}
+	}
+
 	function testNotImplemented() {
 		try {
 			futureFeature();
@@ -374,4 +401,4 @@ class TestExceptions extends Test {
 		}
 	}
 #end
-}
+}