Mercurial > jwe > octave
changeset 27476:fd32c1a9b1bd
revamp error handling
In "error" and similar functions that ultimately call it, simply throw
an exception that contains information about the error (message, id,
stack info) instead of printing an error message immediately and then
throwing an exception. The new approach is more flexible and sllows
for some simplification of the error message routines as they no
longer need feedback from the interpreter to know when to display or
buffer messages. It is now the responsibility of any code that
catches an execution exception to determine whether and when to
display error messages.
* quit.h, quit.cc (class frame_info): New class.
(execution_exception::m_err_type, execution_exception::m_id,
execution_exception::m_message, execution_exception::m_stack_info):
New data members.
(class execution_exception): Store error type, message, id, and stack
info. Provide methods setting and accessing data as needed and for
generating stack trace message from stack info.
(execution_exception::m_stack_trace): Delete data member.
execution_exception::set_stack_trace): Delete method.
(execution_exception::set_err_type, execution_exception::err_type,
execution_exception::stack_trace, execution_exception::set_identifier,
execution_exception::identifier, execution_exception::message,
execution_exception::set_stack_info, execution_exception::display):
New methods.
* call-stack.cc, call-stack.h (call_stack::backtrace_info):
New functions.
* oct-parse.yy (maybe_print_last_error_message): Delete function and
all uses.
* pt-eval.h, pt-eval.cc (tree_evaluator::backtrace_info,
tree_evaluator::backtrace_message): New functions.
(tree_evaluator::backtrace): Now const.
(tree_evaluator::visit_unwind_protect_command,
tree_evaluator::do_unwind_protect_cleanup_code,
tree_evaluator::visit_try_catch_command, tree_evaluator::evalin,
tree_evaluator::eval, tree_evaluator::repl, debugger::repl): Save
current exception info.
* interpreter.h, interpreter.cc (interpreter::handle_exception):
New function. Use it in place of direct calls to
error_system::save_exception, error_system::display_exception (or
execution_exception::display) and interpreter::recover_from_exception,
so that we have uniform behavior when responding to an execution
exception.
* error.h, error.cc (error_system::m_buffer_error_messages,
error_system::m_discard_error_messages, error_system::m_in_try_catch):
Delete data members and associated functions. Remove all uses.
Because the error system no longer displays messages immediately,
it does not need to track whether to discard or buffer error messages
or know whether error and warning functions are invoked inside of
try-catch blocks. Everywhere that catches execution_exceptions must
now handle saving the exception state (for lasterror) and displaying
the error message and traceback as needed.
(): Delete functions and all uses.
(error_stack_frame): Delete struct definition.
(verror, vpr_where, pr_where_internal, pr_where, maybe_enter_debugger,
make_execution_exception, vmessage_with_id, message_with_id,
error_system::maybe_enter_debugger, reset_error_handler,
error_system::reset): Delete functions and all uses.
(error_system::try_option): Delete enum and all uses.
(vusage, error_1, error_system::vwarning,
error_system::rethrow_error, error_system::interpreter_try):
Simplify.
(format_message, make_stack_map, error_system::throw_error,
error_system::save_exception, error_system::display_exception):
New functions.
(Ferror): Update for error_system changes.
| author | John W. Eaton <jwe@octave.org> |
|---|---|
| date | Fri, 04 Oct 2019 01:15:13 -0400 |
| parents | 96d4094585da |
| children | c0883bfc0f36 |
| files | libgui/src/m-editor/file-editor-tab.cc libgui/src/variable-editor-model.cc libinterp/corefcn/call-stack.cc libinterp/corefcn/call-stack.h libinterp/corefcn/cellfun.cc libinterp/corefcn/error.cc libinterp/corefcn/error.h libinterp/corefcn/errwarn.cc libinterp/corefcn/graphics.cc libinterp/corefcn/input.cc libinterp/corefcn/interpreter.cc libinterp/corefcn/interpreter.h libinterp/corefcn/load-path.cc libinterp/corefcn/mex.cc libinterp/corefcn/utils.cc libinterp/corefcn/variables.cc libinterp/octave-value/cdef-object.cc libinterp/octave-value/ov-oncleanup.cc libinterp/parse-tree/oct-parse.yy libinterp/parse-tree/pt-eval.cc libinterp/parse-tree/pt-eval.h libinterp/parse-tree/pt.cc liboctave/util/quit.cc liboctave/util/quit.h |
| diffstat | 24 files changed, 655 insertions(+), 779 deletions(-) [+] |
line wrap: on
line diff
--- a/libgui/src/m-editor/file-editor-tab.cc Fri Oct 04 00:30:34 2019 -0400 +++ b/libgui/src/m-editor/file-editor-tab.cc Fri Oct 04 01:15:13 2019 -0400 @@ -425,20 +425,20 @@ { // INTERPRETER THREAD + error_system& es = interp.get_error_system (); + + unwind_protect frame; + + // Prevent an error in the evaluation here from sending us + // into the debugger. + + es.interpreter_try (frame); + bool eval_error = false; - - error_system& es = interp.get_error_system (); + std::string msg; try { - // Suppress error messages on the console. - unwind_protect frame; - - int bem = es.buffer_error_messages (); - frame.add_method (es, &error_system::set_buffer_error_messages, bem); - - es.buffer_error_messages (bem + 1); - tree_evaluator& tw = interp.get_evaluator (); bp_table& bptab = tw.get_bp_table (); @@ -449,17 +449,26 @@ emit request_add_breakpoint (line, new_cond); } - catch (const index_exception& e) { eval_error = true; } - catch (const execution_exception& e) { eval_error = true; } - catch (const interrupt_exception&) { eval_error = true; } + catch (const execution_exception& e) + { + interpreter::recover_from_exception (); + + msg = e.message (); + eval_error = true; + } + catch (const interrupt_exception&) + { + interpreter::recover_from_exception (); + + msg = "evaluation interrupted"; + eval_error = true; + } if (eval_error) { // Try again with a prompt that indicates the last // attempt was an error. - std::string msg = es.last_error_message (); - QString new_prompt = (tr ("ERROR: ") + QString::fromStdString (msg) + "\n\ndbstop if"); @@ -2291,6 +2300,8 @@ } catch (const execution_exception& e) { + interpreter::recover_from_exception (); + // Ignore syntax error. It was in the old file on disk; // the user may have fixed it already. }
--- a/libgui/src/variable-editor-model.cc Fri Oct 04 00:30:34 2019 -0400 +++ b/libgui/src/variable-editor-model.cc Fri Oct 04 01:15:13 2019 -0400 @@ -1027,7 +1027,7 @@ emit update_data_signal (val); } - catch (execution_exception&) + catch (const execution_exception&) { clear_update_pending (); @@ -1148,7 +1148,7 @@ emit update_data_signal (val); } - catch (execution_exception&) + catch (const execution_exception&) { QString msg = (QString ("variable '%1' is invalid or undefined") .arg (QString::fromStdString (nm))); @@ -1174,7 +1174,7 @@ init_from_oct (interp); } - catch (execution_exception&) + catch (const execution_exception&) { evaluation_error (expr); }
--- a/libinterp/corefcn/call-stack.cc Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/corefcn/call-stack.cc Fri Oct 04 01:15:13 2019 -0400 @@ -621,6 +621,35 @@ return backtrace_frames (curr_user_frame); } + std::list<frame_info> + call_stack::backtrace_info (octave_idx_type& curr_user_frame, + bool print_subfn) const + { + std::list<stack_frame *> frames = backtrace_frames (curr_user_frame); + + std::list<frame_info> retval; + + for (const auto *frm : frames) + { + if (frm->is_user_script_frame () || frm->is_user_fcn_frame () + || frm->is_scope_frame ()) + { + retval.push_back (frame_info (frm->fcn_file_name (), + frm->fcn_name (print_subfn), + frm->line (), frm->column ())); + } + } + + return retval; + } + + std::list<frame_info> call_stack::backtrace_info (void) const + { + octave_idx_type curr_user_frame = -1; + + return backtrace_info (curr_user_frame, true); + } + octave_map call_stack::backtrace (octave_idx_type& curr_user_frame, bool print_subfn) const {
--- a/libinterp/corefcn/call-stack.h Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/corefcn/call-stack.h Fri Oct 04 01:15:13 2019 -0400 @@ -35,6 +35,8 @@ class octave_value; class octave_value_list; +#include "quit.h" + #include "stack-frame.h" #include "symscope.h" @@ -227,8 +229,21 @@ std::list<stack_frame *> backtrace_frames (octave_idx_type& curr_user_frame) const; + // List of raw stack frames. + std::list<stack_frame *> backtrace_frames (void) const; + // List of stack_info objects that can be used in liboctave and + // stored in the execution_exception object. + + std::list<frame_info> backtrace_info (octave_idx_type& curr_user_frame, + bool print_subfn = true) const; + + std::list<frame_info> backtrace_info (void) const; + + // The same as backtrace_info but in the form of a struct array + // object that may be used in the interpreter. + octave_map backtrace (octave_idx_type& curr_user_frame, bool print_subfn = true) const;
--- a/libinterp/corefcn/cellfun.cc Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/corefcn/cellfun.cc Fri Oct 04 01:15:13 2019 -0400 @@ -106,8 +106,6 @@ octave_value_list errlist = inputlist; errlist.prepend (msg); - es.buffer_error_messages (es.buffer_error_messages () - 1); - tmp = octave::feval (error_handler, errlist, nargout); } else @@ -541,14 +539,6 @@ octave::error_system& es = interp.get_error_system (); - octave::unwind_protect frame; - - int bem = es.buffer_error_messages (); - frame.add_method (es, &octave::error_system::set_buffer_error_messages, bem); - - if (error_handler.is_defined ()) - es.buffer_error_messages (bem + 1); - // Apply functions. if (uniform_output) @@ -1234,15 +1224,6 @@ octave::error_system& es = interp.get_error_system (); - octave::unwind_protect frame; - - int bem = es.buffer_error_messages (); - frame.add_method (es, &octave::error_system::set_buffer_error_messages, - bem); - - if (error_handler.is_defined ()) - es.buffer_error_messages (bem + 1); - // Apply functions. if (uniform_output)
--- a/libinterp/corefcn/error.cc Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/corefcn/error.cc Fri Oct 04 01:15:13 2019 -0400 @@ -33,6 +33,8 @@ #include <sstream> #include <string> +#include "quit.h" + #include "bp-table.h" #include "builtin-defun-decls.h" #include "defun.h" @@ -51,156 +53,36 @@ #include "utils.h" #include "variables.h" -static void verror (bool save_last_error, std::ostream& os, const char *name, - const char *id, const char *fmt, va_list args, - bool with_cfn = false) -{ - octave::error_system& es = octave::__get_error_system__ ("verror"); - - es.verror (save_last_error, os, name, id, fmt, args, with_cfn); -} - -static void -vpr_where (std::ostream& os, const char *fmt, va_list args) -{ - if (fmt) - { - if (*fmt) - { - size_t len = strlen (fmt); - - if (len > 0) - { - if (fmt[len - 1] == '\n') - { - if (len > 1) - { - std::string tmp_fmt (fmt, len - 1); - verror (false, os, nullptr, "", tmp_fmt.c_str (), args); - } - } - else - verror (false, os, nullptr, "", fmt, args); - } - } - } - else - panic ("vpr_where: invalid format"); -} - -static void -pr_where_internal (std::ostream& os, const char *fmt, ...) -{ - va_list args; - va_start (args, fmt); - vpr_where (os, fmt, args); - va_end (args); -} - -struct -error_stack_frame -{ - std::string name; - int line; - int column; -}; - -static void -pr_where (std::ostream& os, const char *who, - const std::list<error_stack_frame>& frames) +static std::string +format_message (const char *fmt, va_list args) { - size_t nframes = frames.size (); - - if (nframes > 0) - pr_where_internal (os, "%s: called from\n", who); - - for (const auto& frm : frames) - { - std::string fcn_name = frm.name; - int line = frm.line; - int column = frm.column; - - if (line > 0) - { - if (column > 0) - pr_where_internal (os, " %s at line %d column %d\n", - fcn_name.c_str (), line, column); - else - pr_where_internal (os, " %s at line %d\n", - fcn_name.c_str (), line); - } - else - pr_where_internal (os, " %s\n", fcn_name.c_str ()); - } -} - -static void -pr_where (std::ostream& os, const char *who) -{ - octave::tree_evaluator& tw = octave::__get_evaluator__ ("pr_where"); + if (! fmt) + return ""; - std::list<octave::stack_frame *> call_stack_frames = tw.backtrace_frames (); - - // Print the error message only if it is different from the previous one; - // Makes the output more concise and readable. - call_stack_frames.unique (); - - std::list<error_stack_frame> frames; - for (const auto *frm : call_stack_frames) - { - error_stack_frame frame; - - frame.name = frm->fcn_name (); - frame.line = frm->line (); - frame.column = frm->column (); + std::ostringstream output_buf; - frames.push_back (frame); - } - - pr_where (os, who, frames); -} + octave::vformat (output_buf, fmt, args); -static void -maybe_enter_debugger (octave::execution_exception& e, - bool show_stack_trace = false) -{ - octave::error_system& es - = octave::__get_error_system__ ("maybe_enter_debugger"); - - es.maybe_enter_debugger (e, show_stack_trace); + return output_buf.str (); } OCTAVE_NORETURN static void -vusage (octave::execution_exception& e, const char *id, - const char *fmt, va_list args) +error_1 (octave::execution_exception& e, const char *id, const char *fmt, + va_list args) { - verror (true, std::cerr, "usage", id, fmt, args); + octave::error_system& es = octave::__get_error_system__ ("error_1"); - maybe_enter_debugger (e); - - throw e; + es.error_1 (e, id, fmt, args); } OCTAVE_NORETURN static void -error_1 (octave::execution_exception& e, std::ostream& os, - const char *name, const char *id, const char *fmt, - va_list args, bool with_cfn = false) +error_1 (const char *id, const char *fmt, va_list args) { octave::error_system& es = octave::__get_error_system__ ("error_1"); - es.error_1 (e, os, name, id, fmt, args, with_cfn); -} - -OCTAVE_NORETURN -static void -error_1 (std::ostream& os, const char *name, const char *id, - const char *fmt, va_list args, bool with_cfn = false) -{ - octave::error_system& es = octave::__get_error_system__ ("error_1"); - - es.error_1 (os, name, id, fmt, args, with_cfn); + es.error_1 (id, fmt, args); } static int @@ -229,11 +111,56 @@ es.vwarning (id, fmt, args); } -static std::list<error_stack_frame> +// Use static fields for the best efficiency. +// NOTE: C++0x will allow these two to be merged into one. +static const char *bt_fieldnames[] = + { "file", "name", "line", "column", nullptr }; + +static const octave_fields bt_fields (bt_fieldnames); + +static octave_map +make_stack_map (const std::list<octave::frame_info>& frames) +{ + size_t nframes = frames.size (); + + octave_map retval (dim_vector (nframes, 1), bt_fields); + + Cell& file = retval.contents (0); + Cell& name = retval.contents (1); + Cell& line = retval.contents (2); + Cell& column = retval.contents (3); + + bool have_column = false; + + octave_idx_type k = 0; + + for (const auto& frm : frames) + { + file(k) = frm.file_name (); + name(k) = frm.fcn_name (); + line(k) = frm.line (); + int c = frm.column (); + if (c > 0) + { + have_column = true; + column(k) = c; + } + + k++; + } + + if (! have_column) + retval.rmfield ("column"); + + return retval; +} + +static std::list<octave::frame_info> make_stack_frame_list (const octave_map& stack) { - std::list<error_stack_frame> frames; + std::list<octave::frame_info> frames; + Cell file = stack.contents ("file"); Cell name = stack.contents ("name"); Cell line = stack.contents ("line"); Cell column; @@ -247,15 +174,11 @@ octave_idx_type nel = name.numel (); for (octave_idx_type i = 0; i < nel; i++) - { - error_stack_frame frame; - - frame.name = name(i).string_value (); - frame.line = line(i).int_value (); - frame.column = (have_column ? column(i).int_value () : -1); - - frames.push_back (frame); - } + frames.push_back (octave::frame_info (file(i).string_value (), + name(i).string_value (), + line(i).int_value (), + (have_column + ? column(i).int_value () : -1))); return frames; } @@ -265,7 +188,7 @@ { va_list args; va_start (args, fmt); - error_1 (octave_stdout, nullptr, "", fmt, args); + error_1 ("", fmt, args); va_end (args); } @@ -403,9 +326,6 @@ m_debug_on_error (false), m_debug_on_caught (false), m_debug_on_warning (false), - m_buffer_error_messages (0), - m_in_try_catch (0), - m_discard_error_messages (false), m_discard_warning_messages (false), m_beep_on_error (false), m_backtrace_on_warning (true), @@ -443,29 +363,6 @@ } octave_value - error_system::buffer_error_messages (const octave_value_list& args, - int nargout) - { - return set_internal_variable (m_buffer_error_messages, args, nargout, - "buffer_error_messages"); - } - - octave_value - error_system::in_try_catch (const octave_value_list& args, int nargout) - { - return set_internal_variable (m_in_try_catch, args, nargout, - "in_try_catch"); - } - - octave_value - error_system::discard_error_messages (const octave_value_list& args, - int nargout) - { - return set_internal_variable (m_discard_error_messages, args, nargout, - "discard_error_messages"); - } - - octave_value error_system::discard_warning_messages (const octave_value_list& args, int nargout) { @@ -607,196 +504,95 @@ return retval; } - void error_system::verror (bool save_last_error, std::ostream& os, - const char *name, const char *id, - const char *fmt, va_list args, bool with_cfn) + void error_system::vusage (const char *id, const char *fmt, va_list args) { - if (discard_error_messages () && ! debug_on_caught ()) - return; - - if (! buffer_error_messages () || debug_on_caught ()) - flush_stdout (); + std::string str_id = id ? id : ""; + std::string message = format_message (fmt, args); - // FIXME: we really want to capture the message before it has all the - // formatting goop attached to it. We probably also want just the - // message, not the traceback information. - - std::ostringstream output_buf; + throw_error ("usage", id, message); + } - vformat (output_buf, fmt, args); + void error_system::vwarning (const char *name, const char *id, + const char *fmt, va_list args) + { + flush_stdout (); - std::string base_msg = output_buf.str (); - - bool to_beep_or_not_to_beep_p = beep_on_error (); - + std::string base_msg = format_message (fmt, args); std::string msg_string; - if (to_beep_or_not_to_beep_p) - msg_string = "\a"; + if (name) + msg_string = std::string (name) + ": "; + + msg_string += base_msg; + + bool fmt_suppresses_backtrace = false; + size_t fmt_len = (fmt ? strlen (fmt) : 0); + fmt_suppresses_backtrace = (fmt_len > 0 && fmt[fmt_len-1] == '\n'); - if (name) - { - if (in_try_catch () && ! strcmp (name, "error")) - msg_string += "caught error: "; - else - msg_string += std::string (name) + ": "; - } + if (! fmt_suppresses_backtrace) + msg_string += '\n'; + + last_warning_id (id); + last_warning_message (base_msg); + + if (discard_warning_messages ()) + return; tree_evaluator& tw = m_interpreter.get_evaluator (); - // If with_fcn is specified, we'll attempt to prefix the message with the name - // of the current executing function. But we'll do so only if: - // 1. the name is not empty (anonymous function) - // 2. it is not already there (including the following colon) - if (with_cfn) + bool in_user_code = tw.in_user_code (); + + if (! quiet_warning ()) { - std::string cfn = tw.current_function_name (); + octave_diary << msg_string; + std::cerr << msg_string; - if (! cfn.empty ()) + if (! fmt_suppresses_backtrace && in_user_code + && backtrace_on_warning () + && ! discard_warning_messages ()) { - cfn += ':'; - if (cfn.length () > base_msg.length () - || base_msg.compare (0, cfn.length (), cfn) != 0) - msg_string += cfn + ' '; + std::string bt_msg = tw.backtrace_message (); + + if (! bt_msg.empty ()) + bt_msg = "warning: called from\n" + bt_msg; + + octave_diary << bt_msg << std::endl; + std::cerr << bt_msg << std::endl; } } - msg_string += base_msg + '\n'; - - if (save_last_error) - { - // This is the first error in a possible series. - - last_error_id (id); - last_error_message (base_msg); - last_error_stack (tw.in_user_code () - ? tw.backtrace () : tw.empty_backtrace ()); - } - - if (! buffer_error_messages () || debug_on_caught ()) - { - octave_diary << msg_string; - os << msg_string; - } - } - - void error_system::maybe_enter_debugger (execution_exception& e, - bool show_stack_trace) - { - tree_evaluator& tw = m_interpreter.get_evaluator (); bp_table& bptab = tw.get_bp_table (); - if ((m_interpreter.interactive () || application::forced_interactive ()) - && ((debug_on_error () - && bptab.debug_on_err (last_error_id ())) - || (debug_on_caught () - && bptab.debug_on_caught (last_error_id ()))) - && tw.in_user_code ()) + if ((m_interpreter.interactive () + || application::forced_interactive ()) + && debug_on_warning () && in_user_code && bptab.debug_on_warn (id)) { - if (show_stack_trace) - { - std::string stack_trace = e.info (); + unwind_protect frame; - if (! stack_trace.empty ()) - { - std::cerr << stack_trace; - - e.set_stack_trace (); - } - } + frame.protect_var (m_debug_on_warning); + m_debug_on_warning = false; tw.enter_debugger (); } } - void error_system::vwarning (const char *name, const char *id, - const char *fmt, va_list args) + void error_system::error_1 (execution_exception& e, const char *id, + const char *fmt, va_list args) { - if (discard_warning_messages ()) - return; - - flush_stdout (); - - std::ostringstream output_buf; - - vformat (output_buf, fmt, args); - - // FIXME: we really want to capture the message before it has all the - // formatting goop attached to it. We probably also want just the - // message, not the traceback information. + e.set_identifier (id); + e.set_message (format_message (fmt, args)); - std::string base_msg = output_buf.str (); - std::string msg_string; - - if (name) - msg_string = std::string (name) + ": "; - - msg_string += base_msg + '\n'; - - last_warning_id (id); - last_warning_message (base_msg); - - if (! quiet_warning ()) - { - octave_diary << msg_string; - - std::cerr << msg_string; - } + throw_error (e); } - void error_system::error_1 (execution_exception& e, std::ostream& os, - const char *name, const char *id, - const char *fmt, va_list args, bool with_cfn) + void error_system::error_1 (const char *id, const char *fmt, + va_list args) { - bool show_stack_trace = false; - - if (fmt) - { - if (*fmt) - { - size_t len = strlen (fmt); - - if (len > 0) - { - if (fmt[len - 1] == '\n') - { - if (len > 1) - { - std::string tmp_fmt (fmt, len - 1); - verror (true, os, name, id, tmp_fmt.c_str (), - args, with_cfn); - } + std::string message = format_message (fmt, args); - // If format ends with newline, suppress stack trace. - e.set_stack_trace (); - } - else - { - verror (true, os, name, id, fmt, args, with_cfn); - - tree_evaluator& tw = m_interpreter.get_evaluator (); + std::list<frame_info> stack_info; - if (tw.in_user_code () && ! discard_error_messages ()) - show_stack_trace = true; - } - } - } - } - else - panic ("error_1: invalid format"); - - maybe_enter_debugger (e, show_stack_trace); - - throw e; - } - - void error_system::error_1 (std::ostream& os, const char *name, - const char *id, const char *fmt, - va_list args, bool with_cfn = false) - { - execution_exception e = make_execution_exception ("error"); - - error_1 (e, os, name, id, fmt, args, with_cfn); + throw_error ("error", id, message); } void error_system::vwarning (const char *id, const char *fmt, va_list args) @@ -807,101 +603,37 @@ { // Handle this warning as an error. - error_1 (std::cerr, "error", id, fmt, args); + error_1 (id, fmt, args); } else if (warn_opt == 1) - { - bool fmt_suppresses_backtrace = false; - size_t fmt_len = (fmt ? strlen (fmt) : 0); - fmt_suppresses_backtrace = (fmt_len > 0 && fmt[fmt_len-1] == '\n'); - - if (fmt_suppresses_backtrace && fmt_len > 1) - { - // Strip newline before issuing warning - std::string tmp_fmt (fmt, fmt_len - 1); - vwarning ("warning", id, tmp_fmt.c_str (), args); - } - else - vwarning ("warning", id, fmt, args); - - tree_evaluator& tw = m_interpreter.get_evaluator (); - - bool in_user_code = tw.in_user_code (); - - if (! fmt_suppresses_backtrace && in_user_code - && backtrace_on_warning () - && ! discard_warning_messages ()) - pr_where (std::cerr, "warning"); - - bp_table& bptab = tw.get_bp_table (); - - if ((m_interpreter.interactive () - || application::forced_interactive ()) - && debug_on_warning () && in_user_code && bptab.debug_on_warn (id)) - { - unwind_protect frame; - - frame.protect_var (m_debug_on_warning); - m_debug_on_warning = false; - - tw.enter_debugger (); - } - } - } - - void error_system::rethrow_error (const char *id, const char *fmt, ...) - { - va_list args; - va_start (args, fmt); - verror (false, std::cerr, nullptr, id, fmt, args); - va_end (args); + vwarning ("warning", id, fmt, args); } void error_system::rethrow_error (const std::string& id, const std::string& msg, const octave_map& stack) { - execution_exception e = make_execution_exception ("error"); + std::list<frame_info> stack_info; + + execution_exception e ("error", id, msg, stack_info); if (! stack.isempty () && ! (stack.contains ("file") && stack.contains ("name") && stack.contains ("line"))) error ("rethrow: STACK struct must contain the fields 'file', 'name', and 'line'"); - last_error_id (id); - last_error_message (msg); - last_error_stack (stack); - - size_t len = msg.length (); - - std::string tmp_msg (msg); - if (len > 1 && tmp_msg[len-1] == '\n') - { - tmp_msg.erase (len - 1); + if (! stack.isempty ()) + e.set_stack_info (make_stack_frame_list (stack)); - rethrow_error (id.c_str (), "%s\n", tmp_msg.c_str ()); - } - else - rethrow_error (id.c_str (), "%s", tmp_msg.c_str ()); - - if (! stack.isempty ()) - { - std::ostringstream buf; - - pr_where (buf, "error", make_stack_frame_list (stack)); - - e.set_stack_trace (buf.str ()); - } - - throw e; + throw_error (e); } void error_system::vpanic (const char *fmt, va_list args) { - buffer_error_messages (0); - discard_error_messages (false); + // Is there any point in trying to write the panic message to the + // diary? - verror (false, std::cerr, "panic", "", fmt, args); + std::cerr << "panic: " << format_message (fmt, args) << std::endl; abort (); } @@ -1101,7 +833,7 @@ void error_system::initialize_default_warning_state (void) { - warning_options (octave::init_warning_options ("on")); + warning_options (init_warning_options ("on")); // Most people will want to have the following disabled. @@ -1120,22 +852,8 @@ disable_warning ("Octave:variable-switch-label"); } - void error_system::interpreter_try (octave::unwind_protect& frame, - try_option opt) + void error_system::interpreter_try (unwind_protect& frame) { - switch (opt) - { - case buffer: - frame.protect_var (m_buffer_error_messages); - m_buffer_error_messages++; - break; - - case discard: - frame.protect_var (m_discard_error_messages); - m_discard_error_messages = true; - break; - } - frame.protect_var (m_debug_on_error); m_debug_on_error = false; @@ -1145,26 +863,89 @@ // Leave debug_on_caught as it was, so errors in try/catch are still // caught. } -} + + void error_system::throw_error (const std::string& err_type, + const std::string& id, + const std::string& message, + const std::list<frame_info>& stack_info_arg) + { + std::list<frame_info> stack_info = stack_info_arg; + + if (stack_info.empty ()) + { + tree_evaluator& tw = m_interpreter.get_evaluator (); + + stack_info = tw.backtrace_info (); -octave::execution_exception -make_execution_exception (const char *who) -{ - std::ostringstream buf; + // Print the error message only if it is different from the + // previous one; makes the output more concise and readable. + + stack_info.unique (); + } + + execution_exception ex (err_type, id, message, stack_info); + + throw_error (ex); + } + + void error_system::throw_error (execution_exception& ex) + { + tree_evaluator& tw = m_interpreter.get_evaluator (); + + bp_table& bptab = tw.get_bp_table (); - pr_where (buf, who); + if ((m_interpreter.interactive () || application::forced_interactive ()) + && ((debug_on_error () + && bptab.debug_on_err (last_error_id ())) + || (debug_on_caught () + && bptab.debug_on_caught (last_error_id ()))) + && tw.in_user_code ()) + { + save_exception (ex); + display_exception (ex, std::cerr); - octave::execution_exception retval; + tw.enter_debugger (); + } + + // Throw the exception even if we entered the debugger. + + throw ex; + } - retval.set_stack_trace (buf.str ()); + void error_system::save_exception (const execution_exception& e) + { + last_error_id (e.identifier ()); + std::string message = e.message (); + std::string xmsg + = (message.size () > 0 && message.back () == '\n' + ? message.substr (0, message.size () - 1) : message); + last_error_message (xmsg); + last_error_stack (make_stack_map (e.stack_info ())); + } - return retval; + void error_system::display_exception (const execution_exception& e, + std::ostream& os) const + { + if (m_beep_on_error) + os << "\a"; + + e.display (octave_diary); + e.display (os); + } } void vmessage (const char *name, const char *fmt, va_list args) { - verror (false, std::cerr, name, "", fmt, args); + std::string message; + + if (name) + message = std::string (name) + ": "; + + message += format_message (fmt, args); + + octave_diary << message << std::endl; + std::cerr << message << std::endl; } void @@ -1177,27 +958,11 @@ } void -vmessage_with_id (const char *name, const char *id, const char *fmt, - va_list args) -{ - verror (false, std::cerr, name, id, fmt, args); -} - -void -message_with_id (const char *name, const char *id, const char *fmt, ...) -{ - va_list args; - va_start (args, fmt); - vmessage_with_id (name, id, fmt, args); - va_end (args); -} - -void vusage_with_id (const char *id, const char *fmt, va_list args) { - octave::execution_exception e = make_execution_exception ("usage"); + octave::error_system& es = octave::__get_error_system__ ("warning_enabled"); - vusage (e, id, fmt, args); + es.vusage (id, fmt, args); } void @@ -1212,7 +977,7 @@ void verror (const char *fmt, va_list args) { - error_1 (std::cerr, "error", "", fmt, args); + error_1 ("", fmt, args); } void @@ -1227,7 +992,7 @@ void verror (octave::execution_exception& e, const char *fmt, va_list args) { - error_1 (e, std::cerr, "error", "", fmt, args); + error_1 (e, "", fmt, args); } void @@ -1242,7 +1007,7 @@ void verror_with_cfn (const char *fmt, va_list args) { - error_1 (std::cerr, "error", "", fmt, args, true); + error_1 ("", fmt, args); } void @@ -1257,7 +1022,7 @@ void verror_with_id (const char *id, const char *fmt, va_list args) { - error_1 (std::cerr, "error", id, fmt, args); + error_1 (id, fmt, args); } void @@ -1272,7 +1037,7 @@ void verror_with_id_cfn (const char *id, const char *fmt, va_list args) { - error_1 (std::cerr, "error", id, fmt, args, true); + error_1 (id, fmt, args); } void @@ -1324,7 +1089,7 @@ void vparse_error (const char *fmt, va_list args) { - error_1 (std::cerr, nullptr, "", fmt, args); + error_1 ("", fmt, args); } void @@ -1339,7 +1104,7 @@ void vparse_error_with_id (const char *id, const char *fmt, va_list args) { - error_1 (std::cerr, nullptr, id, fmt, args); + error_1 (id, fmt, args); } void @@ -1351,15 +1116,6 @@ va_end (args); } -void -rethrow_error (const char *id, const char *fmt, ...) -{ - va_list args; - va_start (args, fmt); - error_1 (std::cerr, nullptr, id, fmt, args); - va_end (args); -} - OCTAVE_NORETURN void vpanic (const char *fmt, va_list args) @@ -1420,8 +1176,8 @@ return ovl (); } -DEFUN (error, args, , - doc: /* -*- texinfo -*- +DEFMETHOD (error, interp, args, , + doc: /* -*- texinfo -*- @deftypefn {} {} error (@var{template}, @dots{}) @deftypefnx {} {} error (@var{id}, @var{template}, @dots{}) Display an error message and stop m-file execution. @@ -1516,9 +1272,9 @@ octave_value retval; - octave_value_list nargs = args; - std::string id; + std::string message; + std::list<octave::frame_info> stack_info; bool have_fmt = false; @@ -1539,7 +1295,7 @@ octave_value c = m.getfield ("message"); if (c.is_string ()) - nargs(0) = c.string_value (); + message = c.string_value (); } if (m.contains ("identifier")) @@ -1550,15 +1306,50 @@ id = c.string_value (); } - // FIXME: also need to handle "stack" field in error structure, - // but that will require some more significant surgery on - // handle_message, error_with_id, etc. + if (m.contains ("stack")) + { + octave_value c = m.getfield ("stack"); + + if (c.isstruct ()) + stack_info = make_stack_frame_list (c.map_value ()); + } } else - have_fmt = maybe_extract_message_id ("error", args, nargs, id); + { + octave_value_list nargs = args; + + have_fmt = maybe_extract_message_id ("error", args, nargs, id); + + if (nargs.length () == 0) + message = "unspecified error"; + else + { + octave_value arg; - handle_message (error_with_id, id.c_str (), "unspecified error", - nargs, have_fmt); + if (have_fmt) + { + octave_value_list tmp = Fsprintf (nargs, 1); + arg = tmp(0); + } + else + arg = nargs(0); + + if (arg.is_defined ()) + { + if (arg.isempty ()) + message = ""; + else if (arg.is_string ()) + message = arg.string_value (); + } + } + } + + if (message.empty ()) + return retval; + + octave::error_system& es = interp.get_error_system (); + + es.throw_error ("error", id, message, stack_info); return retval; } @@ -2381,25 +2172,15 @@ } void -interpreter_try (octave::unwind_protect& frame, - octave::error_system::try_option opt) +interpreter_try (octave::unwind_protect& frame) { octave::error_system& es = octave::__get_error_system__ ("interpreter_try"); - es.interpreter_try (frame, opt); + es.interpreter_try (frame); } // Deprecated variables and functions. // This variable is obsolete and always has the value 0. int error_state = 0; - -void -reset_error_handler (void) -{ - octave::error_system& es - = octave::__get_error_system__ ("reset_error_handler"); - - es.reset (); -}
--- a/libinterp/corefcn/error.h Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/corefcn/error.h Fri Oct 04 01:15:13 2019 -0400 @@ -45,12 +45,6 @@ { public: - enum try_option - { - buffer = 1, - discard = 2, - }; - error_system (interpreter& interp); error_system (const error_system&) = delete; @@ -59,13 +53,6 @@ ~error_system (void) = default; - void reset (void) - { - m_buffer_error_messages = 0; - m_in_try_catch = 0; - m_discard_error_messages = false; - } - octave_value debug_on_error (const octave_value_list& args, int nargout); void set_debug_on_error (bool flag) { m_debug_on_error = flag; } @@ -105,45 +92,6 @@ return val; } - octave_value buffer_error_messages (const octave_value_list& args, int nargout); - - void set_buffer_error_messages (int val) { m_buffer_error_messages = val; } - - int buffer_error_messages (void) const { return m_buffer_error_messages; } - - int buffer_error_messages (int new_val) - { - int val = m_buffer_error_messages; - m_buffer_error_messages = new_val; - return val; - } - - octave_value in_try_catch (const octave_value_list& args, int nargout); - - void set_in_try_catch (int val) { m_in_try_catch = val; } - - int in_try_catch (void) const { return m_in_try_catch; } - - int in_try_catch (int new_val) - { - int val = m_in_try_catch; - m_in_try_catch = new_val; - return val; - } - - octave_value discard_error_messages (const octave_value_list& args, int nargout); - - void set_discard_error_messages (bool flag) { m_discard_error_messages = flag; } - - bool discard_error_messages (void) const { return m_discard_error_messages; } - - bool discard_error_messages (bool flag) - { - bool val = m_discard_error_messages; - m_discard_error_messages = flag; - return val; - } - octave_value discard_warning_messages (const octave_value_list& args, int nargout); void set_discard_warning_messages (bool flag) { m_discard_warning_messages = flag; } @@ -296,25 +244,22 @@ const char *id, const char *fmt, va_list args, bool with_cfn = false); - void maybe_enter_debugger (execution_exception& e, - bool show_stack_trace = false); - void vwarning (const char *name, const char *id, const char *fmt, va_list args); OCTAVE_NORETURN - void error_1 (execution_exception& e, std::ostream& os, - const char *name, const char *id, const char *fmt, - va_list args, bool with_cfn = false); + void error_1 (execution_exception& e, const char *id, const char *fmt, + va_list args); OCTAVE_NORETURN - void error_1 (std::ostream& os, const char *name, const char *id, - const char *fmt, va_list args, bool with_cfn); + void error_1 (const char *id, const char *fmt, va_list args); + + OCTAVE_NORETURN + void vusage (const char *id, const char *fmt, va_list args); void vwarning (const char *id, const char *fmt, va_list args); - void rethrow_error (const char *id, const char *fmt, ...); - + OCTAVE_NORETURN void rethrow_error (const std::string& id, const std::string& msg, const octave_map& stack); @@ -336,8 +281,25 @@ void initialize_default_warning_state (void); - void interpreter_try (octave::unwind_protect& frame, - try_option = buffer); + void interpreter_try (octave::unwind_protect& frame); + + // Throw execution_exception or, if debug_on_error is TRUE, enter + // debugger. If stack_info is empty, use current call stack. + + OCTAVE_NORETURN + void throw_error (const std::string& err_type, + const std::string& id, + const std::string& message, + const std::list<frame_info>& stack_info + = std::list<frame_info> ()); + + OCTAVE_NORETURN + void throw_error (execution_exception& e); + + void save_exception (const execution_exception& e); + + void display_exception (const execution_exception& e, + std::ostream& os) const; private: @@ -359,21 +321,6 @@ bool m_debug_on_warning; - //! Tell the error handler whether to print messages, or just store - //! them for later. Used for handling errors in eval() and - //! the 'unwind_protect' statement. - - int m_buffer_error_messages; - - //! The number of layers of try / catch blocks we're in. Used to print - //! "caught error" instead of "error" when "dbstop if caught error" is on. - - int m_in_try_catch; - - //! TRUE means error messages are turned off. - - bool m_discard_error_messages; - //! TRUE means warning messages are turned off. bool m_discard_warning_messages; @@ -425,9 +372,6 @@ extern OCTINTERP_API int warning_enabled (const std::string& id); -extern OCTINTERP_API octave::execution_exception -make_execution_exception (const char *who); - extern OCTINTERP_API void vmessage (const char *name, const char *fmt, va_list args); @@ -473,14 +417,6 @@ extern OCTINTERP_API void parse_error (const char *fmt, ...); -extern OCTINTERP_API void -vmessage_with_id (const char *id, const char *name, const char *fmt, - va_list args); - -OCTAVE_FORMAT_PRINTF (3, 4) -extern OCTINTERP_API void -message_with_id (const char *id, const char *name, const char *fmt, ...); - OCTAVE_NORETURN extern OCTINTERP_API void vusage_with_id (const char *id, const char *fmt, va_list args); @@ -543,14 +479,12 @@ extern OCTINTERP_API void disable_warning (const std::string& id); -extern OCTINTERP_API void -interpreter_try (octave::unwind_protect&, - octave::error_system::try_option = octave::error_system::buffer); +extern OCTINTERP_API void interpreter_try (octave::unwind_protect&); OCTAVE_DEPRECATED (6, "this variable is obsolete and always has the value 0") extern OCTINTERP_API int error_state; -OCTAVE_DEPRECATED (6, "use 'error_system::reset' instead") -extern OCTINTERP_API void reset_error_handler (void); +OCTAVE_DEPRECATED (6, "this function is obsolete and should not be needed") +inline void reset_error_handler (void) { } #endif
--- a/libinterp/corefcn/errwarn.cc Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/corefcn/errwarn.cc Fri Oct 04 01:15:13 2019 -0400 @@ -148,7 +148,7 @@ void err_user_supplied_eval (const char *name) { - octave::execution_exception e = make_execution_exception ("error"); + octave::execution_exception e; err_user_supplied_eval (e, name); } @@ -162,7 +162,7 @@ void err_wrong_type_arg (const char *name, const char *s) { - octave::execution_exception e = make_execution_exception ("error"); + octave::execution_exception e; err_wrong_type_arg (e, name, s); } @@ -177,7 +177,7 @@ void err_wrong_type_arg (const char *name, const std::string& s) { - octave::execution_exception e = make_execution_exception ("error"); + octave::execution_exception e; err_wrong_type_arg (e, name, s.c_str ()); } @@ -192,7 +192,7 @@ void err_wrong_type_arg (const char *name, const octave_value& tc) { - octave::execution_exception e = make_execution_exception ("error"); + octave::execution_exception e; err_wrong_type_arg (e, name, tc); } @@ -209,7 +209,7 @@ void err_wrong_type_arg (const std::string& name, const octave_value& tc) { - octave::execution_exception e = make_execution_exception ("error"); + octave::execution_exception e; err_wrong_type_arg (e, name, tc); } @@ -224,7 +224,7 @@ void err_wrong_type_arg (const char *s) { - octave::execution_exception e = make_execution_exception ("error"); + octave::execution_exception e; err_wrong_type_arg (e, s); } @@ -238,7 +238,7 @@ void err_wrong_type_arg (const std::string& s) { - octave::execution_exception e = make_execution_exception ("error"); + octave::execution_exception e; err_wrong_type_arg (e, s); } @@ -252,7 +252,7 @@ void err_wrong_type_arg (const octave_value& tc) { - octave::execution_exception e = make_execution_exception ("error"); + octave::execution_exception e; err_wrong_type_arg (e, tc); }
--- a/libinterp/corefcn/graphics.cc Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/corefcn/graphics.cc Fri Oct 04 01:15:13 2019 -0400 @@ -3726,7 +3726,7 @@ octave::unwind_protect frame; - interpreter_try (frame, octave::error_system::discard); + interpreter_try (frame); try {
--- a/libinterp/corefcn/input.cc Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/corefcn/input.cc Fri Oct 04 01:15:13 2019 -0400 @@ -189,12 +189,9 @@ unwind_protect frame; - frame.add_method (es, &error_system::set_discard_error_messages, - es.discard_error_messages ()); frame.add_method (es, &error_system::set_discard_warning_messages, es.discard_warning_messages ()); - es.discard_error_messages (true); es.discard_warning_messages (true); try @@ -555,13 +552,7 @@ { eval_error = true; - std::string stack_trace = e.info (); - - if (! stack_trace.empty ()) - std::cerr << stack_trace; - - if (m_interpreter.interactive ()) - interpreter::recover_from_exception (); + m_interpreter.handle_exception (e); } flush_stdout ();
--- a/libinterp/corefcn/interpreter.cc Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/corefcn/interpreter.cc Fri Oct 04 01:15:13 2019 -0400 @@ -271,12 +271,9 @@ } catch (const execution_exception& e) { - std::string stack_trace = e.info (); + interpreter& interp = __get_interpreter__ ("safe_source_file"); - if (! stack_trace.empty ()) - std::cerr << stack_trace; - - interpreter::recover_from_exception (); + interp.handle_exception (e); return 1; } @@ -806,12 +803,7 @@ } catch (const execution_exception& e) { - std::string stack_trace = e.info (); - - if (! stack_trace.empty ()) - std::cerr << stack_trace; - - recover_from_exception (); + handle_exception (e); } } @@ -916,9 +908,9 @@ return 1; } - catch (const execution_exception&) + catch (const execution_exception& e) { - recover_from_exception (); + handle_exception (e); return 1; } @@ -1057,8 +1049,6 @@ atexit_functions.pop_front (); - OCTAVE_SAFE_CALL (m_error_system.reset, ()); - OCTAVE_SAFE_CALL (feval, (fcn, octave_value_list (), 0)); OCTAVE_SAFE_CALL (flush_stdout, ()); @@ -1671,6 +1661,18 @@ return m_evaluator.autoloaded_functions (); } + void interpreter::handle_exception (const execution_exception& e) + { + m_error_system.save_exception (e); + + // FIXME: use a separate stream instad of std::cerr directly so that + // error messages can be redirected more easily? Pass the message + // to an event manager function? + m_error_system.display_exception (e, std::cerr); + + recover_from_exception (); + } + void interpreter::recover_from_exception (void) { can_interrupt = true; @@ -1748,9 +1750,9 @@ { interpreter::recover_from_exception (); } - catch (const execution_exception&) + catch (const execution_exception& e) { - interpreter::recover_from_exception (); + handle_exception (e); } } }
--- a/libinterp/corefcn/interpreter.h Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/corefcn/interpreter.h Fri Oct 04 01:15:13 2019 -0400 @@ -408,6 +408,8 @@ std::list<std::string> autoloaded_functions (void) const; + void handle_exception (const execution_exception& e); + static void recover_from_exception (void); static void add_atexit_function (const std::string& fname);
--- a/libinterp/corefcn/load-path.cc Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/corefcn/load-path.cc Fri Oct 04 01:15:13 2019 -0400 @@ -1264,10 +1264,11 @@ initialize (); } } - catch (const execution_exception&) + catch (const execution_exception& ee) { - // Skip updating if we don't know where we are, - // but don't treat it as an error. + // Skip updating if we don't know where we are, but don't + // treat it as an error. + interpreter::recover_from_exception (); } }
--- a/libinterp/corefcn/mex.cc Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/corefcn/mex.cc Fri Oct 04 01:15:13 2019 -0400 @@ -3253,6 +3253,10 @@ { if (mex_context->trap_feval_error) { + // FIXME: is there a way to indicate what error occurred? + // Should the error message be displayed here? Do we need to + // save the exception info for lasterror? + octave::interpreter::recover_from_exception (); execution_error = true;
--- a/libinterp/corefcn/utils.cc Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/corefcn/utils.cc Fri Oct 04 01:15:13 2019 -0400 @@ -1372,8 +1372,8 @@ } } -DEFMETHOD (isindex, interp, args, , - doc: /* -*- texinfo -*- +DEFUN (isindex, args, , + doc: /* -*- texinfo -*- @deftypefn {} {} isindex (@var{ind}) @deftypefnx {} {} isindex (@var{ind}, @var{n}) Return true if @var{ind} is a valid index. @@ -1401,14 +1401,6 @@ octave_value retval; - octave::error_system& es = interp.get_error_system (); - - octave::unwind_protect frame; - - frame.add_method (es, &octave::error_system::set_discard_error_messages, - es.discard_error_messages ()); - es.discard_error_messages (true); - try { idx_vector idx = args(0).index_vector (true);
--- a/libinterp/corefcn/variables.cc Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/corefcn/variables.cc Fri Oct 04 01:15:13 2019 -0400 @@ -1399,10 +1399,8 @@ octave::interpreter& interp = octave::__get_interpreter__ ("maybe_missing_function_hook"); - octave::error_system& es = interp.get_error_system (); - // Don't do this if we're handling errors. - if (es.buffer_error_messages () || Vmissing_function_hook.empty ()) + if (Vmissing_function_hook.empty ()) return; octave::symbol_table& symtab = interp.get_symbol_table ();
--- a/libinterp/octave-value/cdef-object.cc Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/octave-value/cdef-object.cc Fri Oct 04 01:15:13 2019 -0400 @@ -78,16 +78,14 @@ warning ("interrupt occurred in handle class delete method"); } - catch (const execution_exception&) + catch (const execution_exception& ee) { - octave::error_system& es - = octave::__get_error_system__ ("cdef_object::release"); + interpreter::recover_from_exception (); - std::string msg = es.last_error_message (); + std::string msg = ee.message (); warning ("error caught while executing handle class delete method:\n%s\n", msg.c_str ()); - } catch (const exit_exception&) {
--- a/libinterp/octave-value/ov-oncleanup.cc Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/octave-value/ov-oncleanup.cc Fri Oct 04 01:15:13 2019 -0400 @@ -174,12 +174,11 @@ warning ("onCleanup: interrupt occurred in cleanup action"); } - catch (const octave::execution_exception&) + catch (const octave::execution_exception& ee) { - octave::error_system& es - = octave::__get_error_system__ ("octave_oncleanup::call_object_destructor"); + octave::interpreter::recover_from_exception (); - std::string msg = es.last_error_message (); + std::string msg = ee.message (); warning ("onCleanup: error caught while executing cleanup function:\n%s\n", msg.c_str ());
--- a/libinterp/parse-tree/oct-parse.yy Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/parse-tree/oct-parse.yy Fri Oct 04 01:15:13 2019 -0400 @@ -2504,12 +2504,9 @@ unwind_protect frame; - frame.add_method (es, &error_system::set_discard_error_messages, - es.discard_error_messages ()); frame.add_method (es, &error_system::set_discard_warning_messages, es.discard_warning_messages ()); - es.discard_error_messages (true); es.discard_warning_messages (true); if (! base || ! limit) @@ -4172,12 +4169,9 @@ unwind_protect frame; - frame.add_method (es, &error_system::set_discard_error_messages, - es.discard_error_messages ()); frame.add_method (es, &error_system::set_discard_warning_messages, es.discard_warning_messages ()); - es.discard_error_messages (true); es.discard_warning_messages (true); if (array_list->all_elements_are_constant ()) @@ -5166,25 +5160,6 @@ } static void -maybe_print_last_error_message (bool *doit) -{ - if (doit && *doit) - { - // Print error message again, which was lost because of the stderr - // buffer. Note: this keeps error_state and last_error_stack - // intact. - - octave::error_system& es - = octave::__get_error_system__ ("maybe_print_last_error_message"); - - std::string id = es.last_error_id (); - std::string msg = es.last_error_message (); - - message_with_id ("error", id.c_str (), "%s", msg.c_str ()); - } -} - -static void restore_octave_stdout (std::streambuf *buf) { octave_stdout.flush (); @@ -5227,6 +5202,8 @@ @seealso{eval, diary} @end deftypefn */) { + octave_value_list retval; + int nargin = args.length (); if (nargin == 0 || nargin > 2) @@ -5244,22 +5221,28 @@ std::streambuf* old_out_buf = out_stream.rdbuf (buffer.rdbuf ()); std::streambuf* old_err_buf = err_stream.rdbuf (buffer.rdbuf ()); - bool eval_error_occurred = true; - octave::unwind_protect frame; - frame.add_fcn (maybe_print_last_error_message, &eval_error_occurred); frame.add_fcn (restore_octave_stdout, old_out_buf); frame.add_fcn (restore_octave_stderr, old_err_buf); // call standard eval function - octave_value_list retval; + int eval_nargout = std::max (0, nargout - 1); - retval = Feval (interp, args, eval_nargout); - eval_error_occurred = false; + try + { + retval = Feval (interp, args, eval_nargout); + } + catch (const octave::execution_exception& ee) + { + buffer << "error: " << ee.message () << std::endl; + + throw; + } retval.prepend (buffer.str ()); + return retval; } @@ -5335,11 +5318,13 @@ %!test %! warning ("off", "quiet", "local"); -%! assert (evalc ("warning ('foo')"), "warning: foo\n"); +%! str = evalc ("warning ('foo')"); +%! assert (str(1:13), "warning: foo\n"); %!test %! warning ("off", "quiet", "local"); -%! assert (evalc ("error ('foo')", "warning ('bar')"), "warning: bar\n"); +%! str = evalc ("error ('foo')", "warning ('bar')"); +%! assert (str(1:13), "warning: bar\n"); %!error evalc ("switch = 13;")
--- a/libinterp/parse-tree/pt-eval.cc Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/parse-tree/pt-eval.cc Fri Oct 04 01:15:13 2019 -0400 @@ -216,10 +216,10 @@ if (silent) command_editor::erase_empty_line (true); - std::string msg = buf.str (); - - if (! msg.empty ()) - std::cerr << msg << std::endl; + std::string stopped_in_msg = buf.str (); + + if (! stopped_in_msg.empty ()) + std::cerr << stopped_in_msg << std::endl; std::string tmp_prompt = prompt; if (m_level > 0) @@ -266,8 +266,6 @@ { Vtrack_line_num = false; - es.reset (); - curr_parser.reset (); int retval = curr_parser.run (); @@ -306,10 +304,8 @@ } catch (const execution_exception& ee) { - std::string stack_trace = ee.info (); - - if (! stack_trace.empty ()) - std::cerr << stack_trace; + es.save_exception (ee); + es.display_exception (ee, std::cerr); // Ignore errors when in debugging mode; interpreter::recover_from_exception (); @@ -379,8 +375,6 @@ { unwind_protect_var<bool> upv (m_in_top_level_repl, true); - es.reset (); - repl_parser.reset (); if (at_top_level ()) @@ -443,12 +437,10 @@ << e.message () << " -- trying to return to prompt" << std::endl; } - catch (const execution_exception& e) + catch (const execution_exception& ee) { - std::string stack_trace = e.info (); - - if (! stack_trace.empty ()) - std::cerr << stack_trace; + es.save_exception (ee); + es.display_exception (ee, std::cerr); if (interactive) m_interpreter.recover_from_exception (); @@ -634,13 +626,6 @@ error_system& es = m_interpreter.get_error_system (); - int bem = es.buffer_error_messages (); - unwind_action act ([&es, bem] (void) - { - es.buffer_error_messages (bem); - }); - es.buffer_error_messages (bem + 1); - int parse_status = 0; bool execution_error = false; @@ -651,8 +636,9 @@ { tmp = eval_string (try_code, nargout > 0, parse_status, nargout); } - catch (const execution_exception&) + catch (const execution_exception& ee) { + es.save_exception (ee); interpreter::recover_from_exception (); execution_error = true; @@ -660,11 +646,6 @@ if (parse_status != 0 || execution_error) { - // Set up for letting the user print any messages from - // errors that occurred in the first part of this eval(). - - es.buffer_error_messages (es.buffer_error_messages () - 1); - tmp = eval_string (catch_code, nargout > 0, parse_status, nargout); retval = (nargout > 0) ? tmp : octave_value_list (); @@ -726,13 +707,6 @@ error_system& es = m_interpreter.get_error_system (); - int bem = es.buffer_error_messages (); - unwind_action act2 ([&es, bem] (void) - { - es.buffer_error_messages (bem); - }); - es.buffer_error_messages (bem + 1); - int parse_status = 0; bool execution_error = false; @@ -743,8 +717,9 @@ { tmp = eval_string (try_code, nargout > 0, parse_status, nargout); } - catch (const execution_exception&) + catch (const execution_exception& ee) { + es.save_exception (ee); interpreter::recover_from_exception (); execution_error = true; @@ -752,11 +727,6 @@ if (parse_status != 0 || execution_error) { - // Set up for letting the user print any messages from - // errors that occurred in the first part of this eval(). - - es.buffer_error_messages (es.buffer_error_messages () - 1); - tmp = eval_string (catch_code, nargout > 0, parse_status, nargout); retval = (nargout > 0) ? tmp : octave_value_list (); @@ -1735,6 +1705,18 @@ return m_call_stack.backtrace_frames (); } + std::list<frame_info> + tree_evaluator::backtrace_info (octave_idx_type& curr_user_frame, + bool print_subfn) const + { + return m_call_stack.backtrace_info (curr_user_frame, print_subfn); + } + + std::list<frame_info> tree_evaluator::backtrace_info (void) const + { + return m_call_stack.backtrace_info (); + } + octave_map tree_evaluator::backtrace (octave_idx_type& curr_user_frame, bool print_subfn) const @@ -1742,7 +1724,7 @@ return m_call_stack.backtrace (curr_user_frame, print_subfn); } - octave_map tree_evaluator::backtrace (void) + octave_map tree_evaluator::backtrace (void) const { return m_call_stack.backtrace (); } @@ -1752,6 +1734,34 @@ return m_call_stack.empty_backtrace (); } + std::string tree_evaluator::backtrace_message (void) const + { + std::list<frame_info> frames = backtrace_info (); + + std::ostringstream buf; + + for (const auto& frm : frames) + { + buf << " " << frm.fcn_name (); + + int line = frm.line (); + + if (line > 0) + { + buf << " at line " << line; + + int column = frm.column (); + + if (column > 0) + buf << " column " << column; + + buf << "\n"; + } + } + + return buf.str (); + } + void tree_evaluator::push_dummy_scope (const std::string& name) { symbol_scope dummy_scope (name + "$dummy"); @@ -2962,17 +2972,11 @@ { try { - int itc = es.in_try_catch (); - unwind_action act ([&es, itc] (void) - { - es.in_try_catch (itc); - }); - es.in_try_catch (itc + 1); - try_code->accept (*this); } - catch (const execution_exception&) + catch (const execution_exception& ee) { + es.save_exception (ee); interpreter::recover_from_exception (); execution_error = true; @@ -3041,8 +3045,11 @@ if (list) list->accept (*this); } - catch (const execution_exception&) + catch (const execution_exception& ee) { + error_system& es = m_interpreter.get_error_system (); + + es.save_exception (ee); interpreter::recover_from_exception (); if (m_breaking || m_returning) @@ -3108,12 +3115,15 @@ { unwind_protect_code->accept (*this); } - catch (const execution_exception&) + catch (const execution_exception& ee) { + error_system& es = m_interpreter.get_error_system (); + // FIXME: Maybe we should be able to temporarily set the // interpreter's exception handling state to something "safe" // while the cleanup block runs instead of just resetting it // here? + es.save_exception (ee); interpreter::recover_from_exception (); // Run the cleanup code on exceptions, so that it is run even
--- a/libinterp/parse-tree/pt-eval.h Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/parse-tree/pt-eval.h Fri Oct 04 01:15:13 2019 -0400 @@ -448,13 +448,20 @@ std::list<stack_frame *> backtrace_frames () const; + std::list<frame_info> backtrace_info (octave_idx_type& curr_user_frame, + bool print_subfn = true) const; + + std::list<frame_info> backtrace_info (void) const; + octave_map backtrace (octave_idx_type& curr_user_frame, bool print_subfn = true) const; - octave_map backtrace (); + octave_map backtrace (void) const; octave_map empty_backtrace (void) const; + std::string backtrace_message (void) const; + void push_dummy_scope (const std::string& name); void pop_scope (void);
--- a/libinterp/parse-tree/pt.cc Fri Oct 04 00:30:34 2019 -0400 +++ b/libinterp/parse-tree/pt.cc Fri Oct 04 01:15:13 2019 -0400 @@ -90,10 +90,9 @@ } catch (const execution_exception& e) { - interpreter& interp = tw.get_interpreter (); - error_system& es = interp.get_error_system (); + interpreter::recover_from_exception (); - std::string tmp = es.last_error_message (); + std::string tmp = e.message (); warning ("Error evaluating breakpoint condition:\n %s", tmp.c_str ());
--- a/liboctave/util/quit.cc Fri Oct 04 00:30:34 2019 -0400 +++ b/liboctave/util/quit.cc Fri Oct 04 01:15:13 2019 -0400 @@ -26,6 +26,8 @@ #include <cstring> +#include <ostream> +#include <sstream> #include <new> #include "quit.h" @@ -34,6 +36,60 @@ void (*octave_interrupt_hook) (void) = nullptr; void (*octave_bad_alloc_hook) (void) = nullptr; +namespace octave +{ + std::string execution_exception::stack_trace (void) const + { + size_t nframes = m_stack_info.size (); + + if (nframes == 0) + return std::string (); + + std::ostringstream buf; + + buf << "error: called from\n"; + + for (const auto& frm : m_stack_info) + { + buf << " " << frm.fcn_name (); + + int line = frm.line (); + + if (line > 0) + { + buf << " at line " << line; + + int column = frm.column (); + + if (column > 0) + buf << " column " << column; + } + + buf << "\n"; + } + + return buf.str (); + } + + void execution_exception::display (std::ostream& os) const + { + if (! m_message.empty ()) + { + os << m_err_type << ": " << m_message; + + if (m_message.back () != '\n') + { + os << "\n"; + + std::string st = stack_trace (); + + if (! st.empty ()) + os << st; + } + } + } +} + void octave_handle_signal (void) {
--- a/liboctave/util/quit.h Fri Oct 04 00:30:34 2019 -0400 +++ b/liboctave/util/quit.h Fri Oct 04 01:15:13 2019 -0400 @@ -28,6 +28,8 @@ /* The signal header is just needed for the sig_atomic_t type. */ #if defined (__cplusplus) # include <csignal> +# include <iosfwd> +# include <list> # include <string> extern "C" { #else @@ -38,12 +40,65 @@ namespace octave { + class frame_info + { + public: + + frame_info (void) = default; + + frame_info (const std::string& file_name, const std::string& fcn_name, + int line, int column) + : m_file_name (file_name), m_fcn_name (fcn_name), m_line (line), + m_column (column) + { } + + frame_info (const frame_info&) = default; + + frame_info& operator = (const frame_info&) = default; + + ~frame_info (void) = default; + + std::string file_name (void) const { return m_file_name; } + + std::string fcn_name (void) const { return m_fcn_name; } + + int line (void) const { return m_line; } + + int column (void) const { return m_column; } + + private: + + std::string m_file_name; + + std::string m_fcn_name; + + int m_line; + + int m_column; + }; + + inline bool operator == (const frame_info& a, const frame_info& b) + { + return (a.file_name () == b.file_name () + && a.fcn_name () == b.fcn_name () + && a.line () == b.line () + && a.column () == b.column ()); + } + class execution_exception { public: - execution_exception (void) = default; + typedef std::list<frame_info> stack_info_type; + + execution_exception (const std::string& err_type = "error", + const std::string& id = "", + const std::string& message = "unspecified error", + const stack_info_type& stack_info = stack_info_type ()) + : m_err_type (err_type), m_id (id), m_message (message), + m_stack_info (stack_info) + { } execution_exception (const execution_exception&) = default; @@ -51,24 +106,50 @@ ~execution_exception (void) = default; - virtual void set_stack_trace (const std::string& st) + void set_err_type (const std::string& et) { - m_stack_trace = st; + m_err_type = et; + } + + std::string err_type (void) const { return m_err_type; } + + virtual std::string stack_trace (void) const; + + void set_identifier (const std::string& id) + { + m_id = id; } - virtual void set_stack_trace (void) + virtual std::string identifier (void) const { return m_id; } + + void set_message (const std::string& msg) { - m_stack_trace = ""; + m_message = msg; } - virtual std::string info (void) const + virtual std::string message (void) const { return m_message; } + + virtual stack_info_type stack_info (void) const { - return m_stack_trace; + return m_stack_info; } + void set_stack_info (const stack_info_type& stack_info) + { + m_stack_info = stack_info; + } + + virtual void display (std::ostream& os) const; + private: - std::string m_stack_trace; + std::string m_err_type; + + std::string m_id; + + std::string m_message; + + stack_info_type m_stack_info; }; class