From cc0f5ae1939299b46dc89c88ea68c557e69d0a81 Mon Sep 17 00:00:00 2001 From: sekiguchi-nagisa Date: Mon, 9 Dec 2024 22:22:03 +0900 Subject: [PATCH] introduce suppressed exceptions. now maintain ignored exception from finally/defer block. close #768 --- CHANGELOG.md | 2 + doc/std.md | 2 + src/builtin.h | 13 +++++ src/constant.h | 1 + src/object.cpp | 55 +++++++++++++------ src/object.h | 9 +++ src/vm.cpp | 7 ++- test/cmdline/cmdline_test.cpp | 34 +++++++++--- test/exec/cases/base/builtin_complete2.ds | 2 +- test/exec/cases/output/thrown1.ds | 8 +-- test/exec/cases/output/thrown2.ds | 12 ++-- test/exec/cases/output/thrown3.ds | 37 ++++--------- test/exec/cases/output/try_finally_except1.ds | 42 +++++--------- test/exec/cases/output/try_finally_except2.ds | 15 ++--- 14 files changed, 137 insertions(+), 102 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 868cad030..f498f2716 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,8 @@ - change ``TERM_HOOK`` interface, now do not pass extra information (exit status, termination kind) - also remove ``ON_ASSERT``, ``ON_ERR``, ``ON_EXIT`` constants - now allow signal handler invocation +- **Breaking Change**: now not ignore exceptions from finally/defer block even if currently thrown + - maintain these exceptions and get theme via ``Throwable#suppressed`` method - send ``SIGHUP`` to manged jobs before process termination (even if subshell) #### Builtin diff --git a/doc/std.md b/doc/std.md index 938e2a764..17367f432 100644 --- a/doc/std.md +++ b/doc/std.md @@ -229,6 +229,8 @@ function status(): Int for Throwable function lineno(): Int for Throwable function source(): String for Throwable + +function suppressed(): [Throwable] for Throwable ``` ## Error type diff --git a/src/builtin.h b/src/builtin.h index 1867819a1..cced2ee23 100644 --- a/src/builtin.h +++ b/src/builtin.h @@ -2198,6 +2198,19 @@ ARSH_METHOD error_source(RuntimeContext &ctx) { RET(Value::createStr(source)); } +//!bind: function suppressed($this: Throwable): Array +ARSH_METHOD error_suppressed(RuntimeContext &ctx) { + SUPPRESS_WARNING(error_suppressed); + auto &suppressed = typeAs(LOCAL(0)).getSuppressed(); + auto typeOrError = ctx.typePool.createArrayType(ctx.typePool.get(TYPE::Throwable)); + assert(typeOrError); + std::vector values(suppressed.size()); + for (unsigned int i = 0; i < suppressed.size(); i++) { + values[i] = suppressed[i]; + } + RET(Value::create(*typeOrError.asOk(), std::move(values))); +} + // ################ // ## FD ## // ################ diff --git a/src/constant.h b/src/constant.h index c9dd62e8c..96add07d6 100644 --- a/src/constant.h +++ b/src/constant.h @@ -505,6 +505,7 @@ constexpr size_t SYS_LIMIT_XTRACE_LINE_LEN = 128; constexpr size_t SYS_LIMIT_JOB_TABLE_SIZE = UINT16_MAX; constexpr size_t SYS_LIMIT_SUBSHELL_LEVEL = 1024; constexpr size_t SYS_LIMIT_ERROR_MSG_MAX = UINT16_MAX; // for internal error message +constexpr size_t SYS_LIMIT_SUPPRESSED_EXCEPT_MAX = UINT8_MAX; constexpr size_t SYS_LIMIT_STRING_MAX = INT32_MAX; constexpr size_t SYS_LIMIT_ARRAY_MAX = INT32_MAX; constexpr size_t SYS_LIMIT_KEY_BINDING_MAX = UINT8_MAX; diff --git a/src/object.cpp b/src/object.cpp index 22bba8e47..1455e4859 100644 --- a/src/object.cpp +++ b/src/object.cpp @@ -749,6 +749,23 @@ bool CmdArgsBuilder::add(Value &&arg) { // ## Error_Object ## // ########################## +static void printMessage(FILE *fp, const ErrorObject &obj) { + auto ref = obj.getName().asStrRef(); + fwrite(ref.data(), sizeof(char), ref.size(), fp); + ref = ": "; + fwrite(ref.data(), sizeof(char), ref.size(), fp); + ref = obj.getMessage().asStrRef(); + fwrite(ref.data(), sizeof(char), ref.size(), fp); + fputc('\n', fp); +} + +static void printStackTraceImpl(FILE *fp, const std::vector &stackTrace) { + for (auto &s : stackTrace) { + fprintf(fp, " from %s:%d '%s()'\n", s.getSourceName().c_str(), s.getLineNum(), + s.getCallerName().c_str()); + } +} + void ErrorObject::printStackTrace(const ARState &state, PrintOp op) const { // print header const auto level = state.subshellLevel(); @@ -782,22 +799,16 @@ void ErrorObject::printStackTrace(const ARState &state, PrintOp op) const { break; } } - - // print message (suppress error) - { - auto ref = state.typePool.get(this->getTypeID()).getNameRef(); - fwrite(ref.data(), sizeof(char), ref.size(), stderr); - ref = ": "; - fwrite(ref.data(), sizeof(char), ref.size(), stderr); - ref = this->message.asStrRef(); - fwrite(ref.data(), sizeof(char), ref.size(), stderr); - fputc('\n', stderr); - } - - // print stack trace - for (auto &s : this->stackTrace) { - fprintf(stderr, " from %s:%d '%s()'\n", s.getSourceName().c_str(), s.getLineNum(), - s.getCallerName().c_str()); + printMessage(stderr, *this); // FIXME: check io error ? + printStackTraceImpl(stderr, this->stackTrace); + + // show suppressed exceptions + if (!this->suppressed.empty()) { + fputs("[note] the following exceptions are suppressed\n", stderr); + for (auto &e : this->suppressed) { + printMessage(stderr, *e); + printStackTraceImpl(stderr, e->getStackTrace()); + } } if (op == PrintOp::IGNORED) { fputs("\n", stderr); @@ -805,6 +816,18 @@ void ErrorObject::printStackTrace(const ARState &state, PrintOp op) const { fflush(stderr); } +ObjPtr ErrorObject::addSuppressed(ObjPtr &&except) { + ObjPtr oldest; + if (reinterpret_cast(this) != reinterpret_cast(except.get())) { + if (this->suppressed.size() == SYS_LIMIT_SUPPRESSED_EXCEPT_MAX) { + oldest = std::move(this->suppressed[0]); + this->suppressed.erase(this->suppressed.begin()); + } + this->suppressed.push_back(std::move(except)); + } + return oldest; +} + ObjPtr ErrorObject::newError(const ARState &state, const Type &type, Value &&message, int64_t status) { std::vector traces; diff --git a/src/object.h b/src/object.h index aefcea42a..ef8a2c7fe 100644 --- a/src/object.h +++ b/src/object.h @@ -1067,6 +1067,7 @@ class ErrorObject : public ObjectWithRtti { Value name; int64_t status; std::vector stackTrace; + std::vector> suppressed; public: ErrorObject(const Type &type, Value &&message, Value &&name, int64_t status, @@ -1096,6 +1097,14 @@ class ErrorObject : public ObjectWithRtti { const std::vector &getStackTrace() const { return this->stackTrace; } + /** + * @param + * @return if suppressed except size reaches limit, remove and return oldest entry + */ + ObjPtr addSuppressed(ObjPtr &&except); + + const auto &getSuppressed() const { return this->suppressed; } + /** * create new Error_Object and create stack trace */ diff --git a/src/vm.cpp b/src/vm.cpp index c56b30b27..c87a990a3 100644 --- a/src/vm.cpp +++ b/src/vm.cpp @@ -2038,8 +2038,9 @@ bool VM::mainLoop(ARState &state) { auto entry = state.stack.exitFinally(); if (entry.hasError()) { if (state.hasError()) { - auto e = state.stack.takeThrownObject(); - e->printStackTrace(state, ErrorObject::PrintOp::IGNORED); + if (auto e = entry.asError()->addSuppressed(state.stack.takeThrownObject())) { + e->printStackTrace(state, ErrorObject::PrintOp::IGNORED); + } } state.stack.setThrownObject(entry.asError()); } @@ -2617,7 +2618,7 @@ void VM::handleUncaughtException(ARState &state, ARError *dsError, bool inTermHo // print error message if (kind == AR_ERROR_KIND_RUNTIME_ERROR || kind == AR_ERROR_KIND_ASSERTION_ERROR || - state.has(RuntimeOption::TRACE_EXIT) || inTermHook) { + state.has(RuntimeOption::TRACE_EXIT) || inTermHook || !except->getSuppressed().empty()) { except->printStackTrace(state, inTermHook ? ErrorObject::PrintOp::IGNORED_TERM : ErrorObject::PrintOp::UNCAUGHT); } diff --git a/test/cmdline/cmdline_test.cpp b/test/cmdline/cmdline_test.cpp index 2cf111611..e0a58e105 100644 --- a/test/cmdline/cmdline_test.cpp +++ b/test/cmdline/cmdline_test.cpp @@ -297,20 +297,38 @@ TEST_F(CmdlineTest, marker4) { } } -TEST_F(CmdlineTest, except) { - const char *msg = R"([warning] -the following exception within finally/defer block is ignored +TEST_F(CmdlineTest, except1) { + const char *msg = R"([runtime error] +AssertionFailed: `$false' + from (string):10 '()' +[note] the following exceptions are suppressed SystemError: execution error: hoge: command not found from (string):7 '()' - -[warning] -the following exception within finally/defer block is ignored ShellExit: terminated by exit 34 from (string):3 '()' +)"; -[runtime error] + const char *script = R"( +defer { + call exit 34 +} + +defer { + hoge +} + +assert $false +)"; + ASSERT_NO_FATAL_FAILURE(this->expect(ds("-c", script), 1, "", msg)); +} + +TEST_F(CmdlineTest, except2) { // suppress its self + const char *msg = R"([runtime error] AssertionFailed: `$false' from (string):10 '()' +[note] the following exceptions are suppressed +ShellExit: terminated by exit 34 + from (string):3 '()' )"; const char *script = R"( @@ -319,7 +337,7 @@ defer { } defer { - hoge + (function() => { if $true { throw $THROWN!;} })() } assert $false diff --git a/test/exec/cases/base/builtin_complete2.ds b/test/exec/cases/base/builtin_complete2.ds index 1b01ffddd..eff07e318 100644 --- a/test/exec/cases/base/builtin_complete2.ds +++ b/test/exec/cases/base/builtin_complete2.ds @@ -155,7 +155,7 @@ $assertArray(['abs', 'toFloat'], $(complete 'var a = try { 34; } finally { retur $assertArray(['abs', 'toFloat'], $(complete 'function f($a: Int) { $a.')) $assertArray(['abs', 'toFloat'], $(complete 'var b = function ($a: Int) => $a.')) $assertArray(['abs', 'toFloat'], $(complete 'typedef AAA($a : Int) { var b = $a.')) -$assertArray(['show', 'source', 'ss', 'ssum', 'status'], $(complete ' typedef ANY : Error; typedef INT : ANY +$assertArray(['show', 'source', 'ss', 'ssum', 'status', 'suppressed'], $(complete ' typedef ANY : Error; typedef INT : ANY function ss() : String for ANY { return "$this"; } # also complete super type method function ssum($a : Int) : String for INT { return $this as String + $a diff --git a/test/exec/cases/output/thrown1.ds b/test/exec/cases/output/thrown1.ds index 73a58bf91..7088f0019 100644 --- a/test/exec/cases/output/thrown1.ds +++ b/test/exec/cases/output/thrown1.ds @@ -20,11 +20,9 @@ try { # CHECK: ArithmeticError is thrown at 5 # CHECK: AssertionFailed is thrown at 17 -# CHECKERR: [warning] -# CHECKERR: the following exception within finally/defer block is ignored -# CHECKERR: AssertionFailed: `! $THROWN' -# CHECKERR_RE: from .+/test/exec/cases/output/thrown1\.ds:17 '\(\)' -# CHECKERR_RE: ^$ # CHECKERR: [runtime error] # CHECKERR: ArithmeticError: zero division # CHECKERR_RE: from .+/test/exec/cases/output/thrown1\.ds:5 '\(\)' +# CHECKERR: [note] the following exceptions are suppressed +# CHECKERR: AssertionFailed: `! $THROWN' +# CHECKERR_RE: from .+/test/exec/cases/output/thrown1\.ds:17 '\(\)' \ No newline at end of file diff --git a/test/exec/cases/output/thrown2.ds b/test/exec/cases/output/thrown2.ds index 4fe786a2c..7908da73c 100644 --- a/test/exec/cases/output/thrown2.ds +++ b/test/exec/cases/output/thrown2.ds @@ -1,4 +1,4 @@ -# RUN: exec $cmd --trace-exit $self +# RUN: exec $cmd $self # STATUS: 56 function ff() { @@ -30,17 +30,17 @@ try { $THROWN!.message()[999] } +# show ShellExit having suppressed exceptions even if trace-exit=off + # CHECK: in ff: ShellExit is thrown at 9 # CHECK: ShellExit is thrown at 9 # CHECK: OutOfRangeError is thrown at 30 # CHECK: in defer: ShellExit is thrown at 9 -# CHECKERR: [warning] -# CHECKERR: the following exception within finally/defer block is ignored -# CHECKERR: OutOfRangeError: size is 21, but index is 999 -# CHECKERR_RE: from .+/test/exec/cases/output/thrown2\.ds:30 '\(\)' -# CHECKERR_RE: ^$ # CHECKERR: [runtime error] # CHECKERR: ShellExit: terminated by exit 56 # CHECKERR_RE: from .+/test/exec/cases/output/thrown2\.ds:9 'function ff\(\)' # CHECKERR_RE: from .+/test/exec/cases/output/thrown2\.ds:18 '\(\)' +# CHECKERR: [note] the following exceptions are suppressed +# CHECKERR: OutOfRangeError: size is 21, but index is 999 +# CHECKERR_RE: from .+/test/exec/cases/output/thrown2\.ds:30 '\(\)' \ No newline at end of file diff --git a/test/exec/cases/output/thrown3.ds b/test/exec/cases/output/thrown3.ds index 196d3d5cf..ba291c887 100644 --- a/test/exec/cases/output/thrown3.ds +++ b/test/exec/cases/output/thrown3.ds @@ -13,6 +13,12 @@ function throw(e: Throwable?) { } } +defer { ## check suppressed error + $THROWN!.suppressed().clear() # not affect underlying data + assert $THROWN!.suppressed().size() == 2 + assert $THROWN!.suppressed()[0].suppressed().empty() + assert $THROWN!.suppressed()[1].suppressed().empty() +} defer { defer { @@ -34,33 +40,14 @@ defer { # CHECK: rethrow AssertionFailed # CHECK: rethrow ShellExit -# CHECKERR: [warning] -# CHECKERR: the following exception within finally/defer block is ignored -# CHECKERR: AssertionFailed: `$THROWN is ShellExit' -# CHECKERR: binary expression ` is ' is false -# CHECKERR: : ArithmeticError -# CHECKERR: : ShellExit -# CHECKERR_RE: from .+/test/exec/cases/output/thrown3\.ds:29 '\(\)' -# CHECKERR_RE: ^$ -# CHECKERR: [warning] -# CHECKERR: the following exception within finally/defer block is ignored +# CHECKERR: [runtime error] +# CHECKERR: ArithmeticError: zero division +# CHECKERR_RE: from .+/test/exec/cases/output/thrown3\.ds:38 '\(\)' +# CHECKERR: [note] the following exceptions are suppressed # CHECKERR: AssertionFailed: `$THROWN is ShellExit' # CHECKERR: binary expression ` is ' is false # CHECKERR: : ArithmeticError # CHECKERR: : ShellExit -# CHECKERR_RE: from .+/test/exec/cases/output/thrown3\.ds:29 '\(\)' -# CHECKERR_RE: ^$ -# CHECKERR: [warning] -# CHECKERR: the following exception within finally/defer block is ignored -# CHECKERR: ShellExit: terminated by exit 0 -# CHECKERR_RE: from .+/test/exec/cases/output/thrown3\.ds:21 '\(\)' -# CHECKERR_RE: ^$ -# CHECKERR: [warning] -# CHECKERR: the following exception within finally/defer block is ignored +# CHECKERR_RE: from .+/test/exec/cases/output/thrown3\.ds:35 '\(\)' # CHECKERR: ShellExit: terminated by exit 0 -# CHECKERR_RE: from .+/test/exec/cases/output/thrown3\.ds:21 '\(\)' -# CHECKERR_RE: ^$ -# CHECKERR: [runtime error] -# CHECKERR: ArithmeticError: zero division -# CHECKERR_RE: from .+/test/exec/cases/output/thrown3\.ds:32 '\(\)' - +# CHECKERR_RE: from .+/test/exec/cases/output/thrown3\.ds:27 '\(\)' diff --git a/test/exec/cases/output/try_finally_except1.ds b/test/exec/cases/output/try_finally_except1.ds index baf4d4ac3..b48520504 100644 --- a/test/exec/cases/output/try_finally_except1.ds +++ b/test/exec/cases/output/try_finally_except1.ds @@ -17,13 +17,9 @@ try { $ex = $e } assert($ex is ArithmeticError) - -# CHECKERR: [warning] -# CHECKERR: the following exception within finally/defer block is ignored -# CHECKERR: OutOfRangeError: size is 1, but index is 34 -# CHECKERR_RE: from .+/test/exec/cases/output/try_finally_except1\.ds:9 'function h\(\)' -# CHECKERR_RE: from .+/test/exec/cases/output/try_finally_except1\.ds:15 '\(\)' -# CHECKERR_RE: ^$ +assert ($ex as Error).suppressed().size() == 1 +assert ($ex as Error).suppressed()[0] is OutOfRangeError +assert ($ex as Error).suppressed()[0].lineno() == 9 var count = 0 function g() { @@ -46,6 +42,7 @@ try { } assert $ex is ArithmeticError assert $count == 1 +assert ($ex as ArithmeticError).suppressed().empty() function f() { try { @@ -61,14 +58,9 @@ try { $ex = $e } assert $ex is SystemError - -# CHECKERR: [warning] -# CHECKERR: the following exception within finally/defer block is ignored -# CHECKERR: ShellExit: terminated by exit 0 -# CHECKERR_RE: from .+/test/exec/cases/output/try_finally_except1\.ds:54 'function \(\)' -# CHECKERR_RE: from .+/test/exec/cases/output/try_finally_except1\.ds:54 'function f\(\)' -# CHECKERR_RE: from .+/test/exec/cases/output/try_finally_except1\.ds:59 '\(\)' -# CHECKERR_RE: ^$ +assert ($ex as SystemError).suppressed().size() == 1 +assert ($ex as SystemError).suppressed()[0] is ShellExit +assert ($ex as SystemError).suppressed()[0].lineno() == 51 function ff() { try { @@ -84,13 +76,9 @@ try { $ex = $e } assert $ex is TildeError - -# CHECKERR: [warning] -# CHECKERR: the following exception within finally/defer block is ignored -# CHECKERR: AssertionFailed: `$false' -# CHECKERR_RE: from .+/test/exec/cases/output/try_finally_except1\.ds:77 'function ff\(\)' -# CHECKERR_RE: from .+/test/exec/cases/output/try_finally_except1\.ds:82 '\(\)' -# CHECKERR_RE: ^$ +assert ($ex as Error).suppressed().size() == 1 +assert ($ex as Error).suppressed()[0] is AssertionFailed +assert ($ex as Error).suppressed()[0].lineno() == 69 # ignore in subshell assert $({ @@ -101,11 +89,9 @@ assert $({ /fejfar/fejir }).empty() -# CHECKERR: [warning at subshell=1] -# CHECKERR: the following exception within finally/defer block is ignored -# CHECKERR: ExecError: `false' command exits with non-zero status: `1' -# CHECKERR_RE: from .+/test/exec/cases/output/try_finally_except1\.ds:99 '\(\)' -# CHECKERR_RE: ^$ # CHECKERR: [runtime error at subshell=1] # CHECKERR: SystemError: execution error: /fejfar/fejir: command not found -# CHECKERR_RE: from .+/test/exec/cases/output/try_finally_except1\.ds:101 '\(\)' +# CHECKERR_RE: from .+/test/exec/cases/output/try_finally_except1\.ds:89 '\(\)' +# CHECKERR: [note] the following exceptions are suppressed +# CHECKERR: ExecError: `false' command exits with non-zero status: `1' +# CHECKERR_RE: from .+/test/exec/cases/output/try_finally_except1\.ds:87 '\(\)' \ No newline at end of file diff --git a/test/exec/cases/output/try_finally_except2.ds b/test/exec/cases/output/try_finally_except2.ds index a90a6f8a4..cb3009ac2 100644 --- a/test/exec/cases/output/try_finally_except2.ds +++ b/test/exec/cases/output/try_finally_except2.ds @@ -17,17 +17,12 @@ defer { $? = 111 -# CHECKERR: [warning] -# CHECKERR: the following exception within finally/defer block is ignored -# CHECKERR: KeyNotFoundError: not found key: hello -# CHECKERR_RE: from .+/test/exec/cases/output/try_finally_except2\.ds:11 '\(\)' -# CHECKERR_RE: ^$ -# CHECKERR: [warning] -# CHECKERR: the following exception within finally/defer block is ignored -# CHECKERR: ArithmeticError: zero division -# CHECKERR_RE: from .+/test/exec/cases/output/try_finally_except2\.ds:6 '\(\)' -# CHECKERR_RE: ^$ # CHECKERR: [runtime error] # CHECKERR: ShellExit: terminated by exit 166 # CHECKERR_RE: from .+/test/exec/cases/output/try_finally_except2\.ds:15 'function \(\)' # CHECKERR_RE: from .+/test/exec/cases/output/try_finally_except2\.ds:15 '\(\)' +# CHECKERR: [note] the following exceptions are suppressed +# CHECKERR: KeyNotFoundError: not found key: hello +# CHECKERR_RE: from .+/test/exec/cases/output/try_finally_except2\.ds:11 '\(\)' +# CHECKERR: ArithmeticError: zero division +# CHECKERR_RE: from .+/test/exec/cases/output/try_finally_except2\.ds:6 '\(\)'