Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[hl/eval/cpp/neko] Fix exception stack when wrapping native exceptions #11249

Merged
merged 7 commits into from
Jul 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/filters/exceptions.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 10 additions & 1 deletion std/cpp/_std/haxe/Exception.hx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -63,6 +66,12 @@ class Exception {
__skipStack++;
}

@:noCompletion
@:ifFeature("haxe.Exception.get_stack")
inline function __unshiftStack():Void {
__skipStack--;
}

function get_message():String {
return __exceptionMessage;
}
Expand Down
11 changes: 10 additions & 1 deletion std/eval/_std/haxe/Exception.hx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -63,6 +66,12 @@ class Exception {
__skipStack++;
}

@:noCompletion
@:ifFeature("haxe.Exception.get_stack")
inline function __unshiftStack():Void {
__skipStack--;
}

function get_message():String {
return __exceptionMessage;
}
Expand Down
11 changes: 10 additions & 1 deletion std/hl/_std/haxe/Exception.hx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -62,6 +65,12 @@ class Exception {
__skipStack++;
}

@:noCompletion
@:ifFeature("haxe.Exception.get_stack")
inline function __unshiftStack():Void {
__skipStack--;
}

function get_message():String {
return __exceptionMessage;
}
Expand Down
2 changes: 1 addition & 1 deletion std/hl/_std/haxe/NativeStackTrace.hx
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
11 changes: 10 additions & 1 deletion std/neko/_std/haxe/Exception.hx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -63,6 +66,12 @@ class Exception {
__skipStack++;
}

@:noCompletion
@:ifFeature("haxe.Exception.get_stack")
inline function __unshiftStack():Void {
__skipStack--;
}

function get_message():String {
return __exceptionMessage;
}
Expand Down
41 changes: 34 additions & 7 deletions tests/unit/src/unit/TestExceptions.hx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package unit;
package unit;

import haxe.Exception;
import haxe.exceptions.ArgumentException;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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();
Expand Down Expand Up @@ -374,4 +401,4 @@ class TestExceptions extends Test {
}
}
#end
}
}