From 24410a77281c5170ea45a667b9b4b78dfeba50e5 Mon Sep 17 00:00:00 2001 From: Rudy Ges Date: Thu, 8 Jun 2023 11:34:58 +0200 Subject: [PATCH 1/7] [tests] add test for 11247 --- tests/unit/src/unit/TestExceptions.hx | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/tests/unit/src/unit/TestExceptions.hx b/tests/unit/src/unit/TestExceptions.hx index 98fd5e13a48..5240b96095b 100644 --- a/tests/unit/src/unit/TestExceptions.hx +++ b/tests/unit/src/unit/TestExceptions.hx @@ -1,4 +1,4 @@ -package unit; +package unit; import haxe.Exception; import haxe.exceptions.ArgumentException; @@ -235,7 +235,8 @@ class TestExceptions extends Test { public function testExceptionStack() { var data = [ '_without_ throws' => stacksWithoutThrowLevel1(), - '_with_ throws' => stacksWithThrowLevel1() + '_with_ throws' => stacksWithThrowLevel1(), + 'auto wrapped' => stacksAutoWrappedLevel1() ]; for(label => stacks in data) { Assert.isTrue(stacks.length > 1, '$label: wrong stacks.length'); @@ -295,6 +296,22 @@ class TestExceptions extends Test { return result; } + function stacksAutoWrappedLevel1() { + return stacksAutoWrappedLevel2(); + } + + function stacksAutoWrappedLevel2():Array { + @:pure(false) function wrapNativeError(_) return []; + + var result:Array = []; + // 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); + #if (eval || hl || neko || cpp) result.push(try wrapNativeError((null:String).length) catch(e:Exception) e.stack); #end + return result; + } + function stackItemData(item:StackItem):ItemData { var result:ItemData = {}; switch item { @@ -374,4 +391,4 @@ class TestExceptions extends Test { } } #end -} \ No newline at end of file +} From e4f8cf8f4e0e283acfe511144d743b85b8eebb10 Mon Sep 17 00:00:00 2001 From: Rudy Ges Date: Thu, 8 Jun 2023 14:17:23 +0200 Subject: [PATCH 2/7] Fix for 11247 --- std/cpp/_std/haxe/Exception.hx | 11 ++++++++++- std/eval/_std/haxe/Exception.hx | 11 ++++++++++- std/hl/_std/haxe/Exception.hx | 11 ++++++++++- std/neko/_std/haxe/Exception.hx | 11 ++++++++++- 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/std/cpp/_std/haxe/Exception.hx b/std/cpp/_std/haxe/Exception.hx index 4c244a7e34d..92fbaf553bd 100644 --- a/std/cpp/_std/haxe/Exception.hx +++ b/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; } diff --git a/std/eval/_std/haxe/Exception.hx b/std/eval/_std/haxe/Exception.hx index 814370e7ac8..48467a04484 100644 --- a/std/eval/_std/haxe/Exception.hx +++ b/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; } diff --git a/std/hl/_std/haxe/Exception.hx b/std/hl/_std/haxe/Exception.hx index 8f3ae5fc734..68734c49586 100644 --- a/std/hl/_std/haxe/Exception.hx +++ b/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; } diff --git a/std/neko/_std/haxe/Exception.hx b/std/neko/_std/haxe/Exception.hx index 1302231c141..0579eb835ab 100644 --- a/std/neko/_std/haxe/Exception.hx +++ b/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; } From b6c3b55f36906cb0543bb777331f20ed427a3135 Mon Sep 17 00:00:00 2001 From: Rudy Ges Date: Thu, 8 Jun 2023 14:44:52 +0200 Subject: [PATCH 3/7] [hl] restore callstack fix --- std/hl/_std/haxe/NativeStackTrace.hx | 2 +- tests/unit/src/unit/TestExceptions.hx | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/std/hl/_std/haxe/NativeStackTrace.hx b/std/hl/_std/haxe/NativeStackTrace.hx index e7cf077a142..eada6faf0be 100644 --- a/std/hl/_std/haxe/NativeStackTrace.hx +++ b/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") diff --git a/tests/unit/src/unit/TestExceptions.hx b/tests/unit/src/unit/TestExceptions.hx index 5240b96095b..aec97342d45 100644 --- a/tests/unit/src/unit/TestExceptions.hx +++ b/tests/unit/src/unit/TestExceptions.hx @@ -243,10 +243,6 @@ class TestExceptions extends Test { 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 { From 979a7bc44208a1b124f1ccf152a5c64d22bc79f8 Mon Sep 17 00:00:00 2001 From: Rudy Ges Date: Thu, 8 Jun 2023 15:02:35 +0200 Subject: [PATCH 4/7] [tests] cpp doesn't like natixe exceptions in those tests either --- tests/unit/src/unit/TestExceptions.hx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/src/unit/TestExceptions.hx b/tests/unit/src/unit/TestExceptions.hx index aec97342d45..32bdab3aa28 100644 --- a/tests/unit/src/unit/TestExceptions.hx +++ b/tests/unit/src/unit/TestExceptions.hx @@ -304,7 +304,7 @@ class TestExceptions extends Test { // 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); - #if (eval || hl || neko || cpp) result.push(try wrapNativeError((null:String).length) catch(e:Exception) e.stack); #end + #if (eval || hl || neko) result.push(try wrapNativeError((null:String).length) catch(e:Exception) e.stack); #end return result; } From 47c386eeb8c9805193d7c43d8d7b08d774767b7c Mon Sep 17 00:00:00 2001 From: Rudy Ges Date: Thu, 8 Jun 2023 15:48:00 +0200 Subject: [PATCH 5/7] [tests] only run new test for eval/hl/neko --- tests/unit/src/unit/TestExceptions.hx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/src/unit/TestExceptions.hx b/tests/unit/src/unit/TestExceptions.hx index 32bdab3aa28..b3ce16d619b 100644 --- a/tests/unit/src/unit/TestExceptions.hx +++ b/tests/unit/src/unit/TestExceptions.hx @@ -236,7 +236,9 @@ class TestExceptions extends Test { var data = [ '_without_ throws' => stacksWithoutThrowLevel1(), '_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'); @@ -304,7 +306,7 @@ class TestExceptions extends Test { // 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); - #if (eval || hl || neko) result.push(try wrapNativeError((null:String).length) catch(e:Exception) e.stack); #end + result.push(try wrapNativeError((null:String).length) catch(e:Exception) e.stack); return result; } From 3e3e90d080eea3f5039e0dd85b3ef0fa113670d1 Mon Sep 17 00:00:00 2001 From: Rudy Ges Date: Mon, 3 Jul 2023 18:03:28 +0200 Subject: [PATCH 6/7] Fix #11265 --- src/filters/exceptions.ml | 6 ++++-- tests/unit/src/unit/TestExceptions.hx | 10 ++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/filters/exceptions.ml b/src/filters/exceptions.ml index fa4b0182928..d092e890d7d 100644 --- a/src/filters/exceptions.ml +++ b/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 diff --git a/tests/unit/src/unit/TestExceptions.hx b/tests/unit/src/unit/TestExceptions.hx index b3ce16d619b..316d9acad91 100644 --- a/tests/unit/src/unit/TestExceptions.hx +++ b/tests/unit/src/unit/TestExceptions.hx @@ -336,6 +336,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(); From 90120992d1421b3b50d684630a2b766374281e65 Mon Sep 17 00:00:00 2001 From: Rudy Ges Date: Tue, 4 Jul 2023 08:41:21 +0200 Subject: [PATCH 7/7] [tests] don't compile test on target that won't run it --- tests/unit/src/unit/TestExceptions.hx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/src/unit/TestExceptions.hx b/tests/unit/src/unit/TestExceptions.hx index 316d9acad91..6fb5580af80 100644 --- a/tests/unit/src/unit/TestExceptions.hx +++ b/tests/unit/src/unit/TestExceptions.hx @@ -294,6 +294,7 @@ class TestExceptions extends Test { return result; } + #if (eval || hl || neko) function stacksAutoWrappedLevel1() { return stacksAutoWrappedLevel2(); } @@ -309,6 +310,7 @@ class TestExceptions extends Test { result.push(try wrapNativeError((null:String).length) catch(e:Exception) e.stack); return result; } + #end function stackItemData(item:StackItem):ItemData { var result:ItemData = {};