octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Using C++ exceptions in liboctave (was: Re: mapper functions for 3.1)


From: John W. Eaton
Subject: Using C++ exceptions in liboctave (was: Re: mapper functions for 3.1)
Date: Thu, 14 Feb 2008 17:15:41 -0500

On 14-Feb-2008, John W. Eaton wrote:

| But anyway, throwing an exception is probably best, and as you say, if
| we just set liboctave_error_handler to a function that prints the
| message and throws an exception instead of setting it to the error
| function defined in /src/error.cc, then I think that will do most of
| what we need with minimal impact.  We'll need to catch the exception
| and then call Octave's error function again (so that error_state is
| set properly).  We'll also need to do whatever is necessary to handle
| exceptions that are thrown in callback functions that are executed by
| non-C++ code (for example, the user-defined functions used by fsolve,
| quad, lsode, daspk, etc.).  I'm not sure, but I think that means
| catching them in the callback and setting a global variable indicating
| an exception has occurred, calling longjmp, then rethrowing the
| exception.  This is similar to what is supposed to happen for
| allocation exceptions in callbacks, so I think it can be handled the
| same, and will only require changing the macros in f77-fcn.h and
| quit.h in the libcruft/misc directory, possibly with the addition of a
| helper function or two.  Would you like for me to work on this part?

Here is my first attempt at doing making this change.  It seems to
work for me so I checked it in.  A quick test case for the callback
case is

  function y = f (x)
    persistent call = 0;
    if (++call == 3)
      call = 0;
      ## This call to bessel will generate an error from inside liboctave.
      besselj ([1,2], [3,4,5]);
    endif
    y = x.^2;
  endfunction

  quad (@f, 1, 2)

With the patch, I see:

  error: besselj: the sizes of alpha and x must conform
  error: caught execution error in library function
  error: evaluating if command near line 3, column 5
  error: called from `f'
  error: quad: evaluation of user-supplied function failed

The second message "caught execution error ..." should maybe be
fixed or removed when we get to this item from the 3.1 wishlist:

   6. Improve traceback error messages.  The messages should list the
      exact line where an error occurred, then only print function
      traceback info and not information about which
      if/for/while/etc. statement includes the error.  That extra
      information just seems to confuse users, and the current
      messages don't always clearly describe the actual location of
      the error.

If it is removed, we may need to be sure to set error_state to the
correct value even though an error message is not printed.

Also, another possible problem is that we have code like this:

  Matrix
  Matrix::tinverse (MatrixType &mattype, octave_idx_type& info, double& rcond, 
                    int force, int calc_cond) const
  {
    octave_idx_type nr = rows ();
    octave_idx_type nc = cols ();

    if (nr != nc || nr == 0 || nc == 0)
      (*current_liboctave_error_handler) ("inverse requires square matrix");
    else
      {
        int typ = mattype.type ();
        char uplo = (typ == MatrixType::Lower ? 'L' : 'U');
        char udiag = 'N';
        retval = *this;
        double *tmp_data = retval.fortran_vec ();

        F77_XFCN (dtrtri, DTRTRI, (F77_CONST_CHAR_ARG2 (&uplo, 1),
                                   F77_CONST_CHAR_ARG2 (&udiag, 1),
                                   nr, tmp_data, nr, info 
                                   F77_CHAR_ARG_LEN (1)
                                   F77_CHAR_ARG_LEN (1)));

        if (f77_exception_encountered)
          (*current_liboctave_error_handler) ("unrecoverable error in dtrtri");
        else
          {
            // Throw-away extra info LAPACK gives so as to not change output.
            rcond = 0.0;
            if (info != 0) 
              info = -1;
            else if (calc_cond) 
              {
                octave_idx_type dtrcon_info = 0;
                char job = '1';

                OCTAVE_LOCAL_BUFFER (double, work, 3 * nr);
                OCTAVE_LOCAL_BUFFER (octave_idx_type, iwork, nr);

                F77_XFCN (dtrcon, DTRCON, (F77_CONST_CHAR_ARG2 (&job, 1),
                                           F77_CONST_CHAR_ARG2 (&uplo, 1),
                                           F77_CONST_CHAR_ARG2 (&udiag, 1),
                                           nr, tmp_data, nr, rcond, 
                                           work, iwork, dtrcon_info 
                                           F77_CHAR_ARG_LEN (1)
                                           F77_CHAR_ARG_LEN (1)
                                           F77_CHAR_ARG_LEN (1)));

                if (f77_exception_encountered)
                  (*current_liboctave_error_handler) 
                    ("unrecoverable error in dtrcon");

                if (dtrcon_info != 0) 
                  info = -1;
              }
          }

        if (info == -1 && ! force)
          retval = *this; // Restore matrix contents.
      }

    return retval;
  }

in which we currently continue on after a call to the error_handler.
In most cases (including the one above), I think that we are OK to
just throw an execption and bypass any cleanup, but I can't be sure
without looking at all the cases individually.

If I am going to check all of these individually, then maybe we should
go a step further at the same time and replace the use of
f77_exception_encountered with a C++ exception.  I think all of these
cases are internal errors from which we can't recover, so an exception
would be simpler as there would be no need to check after every call
to F77_XFCN and we could just handle the exception at a higher level.
If necessary, we can use try/catch for any of these
f77_exception_encountered cases that need to be handled locally.

Comments?

jwe


# HG changeset patch
# User John W. Eaton <address@hidden>
# Date 1203027263 18000
# Node ID 78f3811155f745e7fc41e9025da3da10dbfd6c4a
# Parent  93826ba0d07898c881df59c7ad9d3df2597ea4c5
use exceptions in liboctave error handler

diff -r 93826ba0d078 -r 78f3811155f7 libcruft/ChangeLog
--- a/libcruft/ChangeLog        Wed Feb 13 00:00:33 2008 -0500
+++ b/libcruft/ChangeLog        Thu Feb 14 17:14:23 2008 -0500
@@ -1,3 +1,18 @@ 2008-02-12  John W. Eaton  <address@hidden
+2008-02-14  John W. Eaton  <address@hidden>
+
+       * misc/f77-fcn.h (F77_XFCN): Call octave_rethrow_exception here
+       instead of checking octave_allocation_error.
+       * misc/quit.cc (octave_execution_exception): New function.
+       (octave_rethrow_exception): New function.
+       (octave_handle_signal): Call octave_rethrow_exception instead of
+       octave_throw_interrupt_exception.
+       * misc/quit.h (octave_execution_error): New variable.
+       (END_INTERRUPT_WITH_EXCEPTIONS): Catch octave_execution_exception.
+       (octave_execution_exception): New class.
+       (octave_exception): New enum.
+       (octave_exception_state): Rename from octave_allocation_error.
+       Change all uses.
+
 2008-02-12  John W. Eaton  <address@hidden>
 
        * lapack-xtra/xilaenv.f: New wrapper for Fortran function ilaenv.
diff -r 93826ba0d078 -r 78f3811155f7 libcruft/misc/cquit.c
--- a/libcruft/misc/cquit.c     Wed Feb 13 00:00:33 2008 -0500
+++ b/libcruft/misc/cquit.c     Thu Feb 14 17:14:23 2008 -0500
@@ -255,7 +255,7 @@ sig_atomic_t octave_interrupt_immediatel
 
 sig_atomic_t octave_interrupt_state = 0;
 
-sig_atomic_t octave_allocation_error = 0;
+sig_atomic_t octave_exception_state = 0;
 
 sig_atomic_t octave_signal_caught = 0;
 
diff -r 93826ba0d078 -r 78f3811155f7 libcruft/misc/f77-fcn.h
--- a/libcruft/misc/f77-fcn.h   Wed Feb 13 00:00:33 2008 -0500
+++ b/libcruft/misc/f77-fcn.h   Thu Feb 14 17:14:23 2008 -0500
@@ -62,10 +62,8 @@ extern "C" {
           octave_restore_current_context (saved_context); \
          if (f77_exception_encountered) \
            F77_XFCN_ERROR (f, F); \
-          else if (octave_allocation_error) \
-            octave_throw_bad_alloc (); \
           else \
-            octave_throw_interrupt_exception (); \
+           octave_rethrow_exception (); \
        } \
       else \
         { \
diff -r 93826ba0d078 -r 78f3811155f7 libcruft/misc/quit.cc
--- a/libcruft/misc/quit.cc     Wed Feb 13 00:00:33 2008 -0500
+++ b/libcruft/misc/quit.cc     Thu Feb 14 17:14:23 2008 -0500
@@ -58,12 +58,47 @@ octave_throw_interrupt_exception (void)
 }
 
 void
+octave_throw_execution_exception (void)
+{
+  // FIXME -- would a hook function be useful here?
+
+  octave_exception_state = octave_exec_exception;
+    
+  throw octave_execution_exception ();
+}
+
+void
 octave_throw_bad_alloc (void)
 {
   if (octave_bad_alloc_hook)
     octave_bad_alloc_hook ();
     
+  octave_exception_state = octave_alloc_exception;
+
   throw std::bad_alloc ();
+}
+
+void
+octave_rethrow_exception (void)
+{
+  if (octave_interrupt_state)
+    octave_throw_interrupt_exception ();
+  else
+    {
+      switch (octave_exception_state)
+       {
+       case octave_exec_exception:
+         octave_throw_execution_exception ();
+         break;
+
+       case octave_alloc_exception:
+         octave_throw_bad_alloc ();
+         break;
+
+       default:
+         break;
+       }
+    }
 }
 
 /*
diff -r 93826ba0d078 -r 78f3811155f7 libcruft/misc/quit.h
--- a/libcruft/misc/quit.h      Wed Feb 13 00:00:33 2008 -0500
+++ b/libcruft/misc/quit.h      Thu Feb 14 17:14:23 2008 -0500
@@ -74,10 +74,22 @@ CRUFT_API extern void octave_restore_sig
 
 #ifdef __cplusplus
 class
+octave_execution_exception
+{
+};
+
+class
 octave_interrupt_exception
 {
 };
 #endif
+
+enum octave_exception
+{
+  octave_no_exception = 0,
+  octave_exec_exception = 1,
+  octave_alloc_exception = 2
+};
 
 CRUFT_API extern sig_atomic_t octave_interrupt_immediately;
 
@@ -88,7 +100,7 @@ CRUFT_API extern sig_atomic_t octave_int
 */
 CRUFT_API extern sig_atomic_t octave_interrupt_state;
 
-CRUFT_API extern sig_atomic_t octave_allocation_error;
+CRUFT_API extern sig_atomic_t octave_exception_state;
 
 CRUFT_API extern sig_atomic_t octave_signal_caught;
 
@@ -96,7 +108,11 @@ CRUFT_API extern void octave_handle_sign
 
 CRUFT_API extern void octave_throw_interrupt_exception (void) 
GCC_ATTR_NORETURN;
 
+CRUFT_API extern void octave_throw_execution_exception (void) 
GCC_ATTR_NORETURN;
+
 CRUFT_API extern void octave_throw_bad_alloc (void) GCC_ATTR_NORETURN;
+
+CRUFT_API extern void octave_rethrow_exception (void) GCC_ATTR_NORETURN;
 
 #define OCTAVE_QUIT \
   do \
@@ -119,7 +135,7 @@ CRUFT_API extern void octave_throw_bad_a
 
      BEGIN_INTERRUPT_IMMEDIATELY_IN_FOREIGN_CODE_1;
      ... custom code here, normally ending in a call to
-         octave_throw_interrupt_exception ...
+         octave_rethrow_exception ...
      BEGIN_INTERRUPT_IMMEDIATELY_IN_FOREIGN_CODE_2;
 
    so that you can perform extra clean up operations before throwing
@@ -127,7 +143,7 @@ CRUFT_API extern void octave_throw_bad_a
 
 #define BEGIN_INTERRUPT_IMMEDIATELY_IN_FOREIGN_CODE \
   BEGIN_INTERRUPT_IMMEDIATELY_IN_FOREIGN_CODE_1; \
-  octave_throw_interrupt_exception (); \
+  octave_rethrow_exception (); \
   BEGIN_INTERRUPT_IMMEDIATELY_IN_FOREIGN_CODE_2
 
 #define BEGIN_INTERRUPT_IMMEDIATELY_IN_FOREIGN_CODE_1 \
@@ -170,10 +186,16 @@ CRUFT_API extern void octave_throw_bad_a
       octave_interrupt_immediately = saved_octave_interrupt_immediately; \
       octave_jump_to_enclosing_context (); \
     } \
+  catch (octave_execution_exception) \
+    { \
+      octave_interrupt_immediately = saved_octave_interrupt_immediately; \
+      octave_exception_state = octave_exec_exception; \
+      octave_jump_to_enclosing_context (); \
+    } \
   catch (std::bad_alloc) \
     { \
       octave_interrupt_immediately = saved_octave_interrupt_immediately; \
-      octave_allocation_error = 1; \
+      octave_exception_state = octave_alloc_exception; \
       octave_jump_to_enclosing_context (); \
     } \
  \
diff -r 93826ba0d078 -r 78f3811155f7 src/ChangeLog
--- a/src/ChangeLog     Wed Feb 13 00:00:33 2008 -0500
+++ b/src/ChangeLog     Thu Feb 14 17:14:23 2008 -0500
@@ -1,3 +1,19 @@ 2008-02-12  David Bateman  <address@hidden
+2008-02-14  John W. Eaton  <address@hidden>
+
+       * sighandlers.cc (user_abort): If interrupting immediately, set
+       octave_interrupt_state if it is not already set.
+
+       * pt-stmt.cc (tree_statement::eval): Catch execution exceptions.
+
+       * octave.cc (lo_error_handler): New static function.
+       (initialize_error_handlers): Set liboctave_error_handler to
+       lo_error_handler, not error.
+
+       * DLD-FUNCTIONS/urlwrite.cc (urlget): Call octave_rethrow_exception
+       instead of octave_throw_interrupt_exception.
+       * utils.cc (BEGIN_INTERRUPT_IMMEDIATELY_IN_FOREIGN_CODE_FOR_VSNPRINTF): 
+       Likewise.
+
 2008-02-12  David Bateman  <address@hidden>
 
        * graphics.h.in: Implement the cdatamapping property in patch and
diff -r 93826ba0d078 -r 78f3811155f7 src/DLD-FUNCTIONS/urlwrite.cc
--- a/src/DLD-FUNCTIONS/urlwrite.cc     Wed Feb 13 00:00:33 2008 -0500
+++ b/src/DLD-FUNCTIONS/urlwrite.cc     Thu Feb 14 17:14:23 2008 -0500
@@ -167,7 +167,7 @@ urlget (const std::string& url, const st
 
   res = CURLE_ABORTED_BY_CALLBACK;
   urlget_cleanup (curl);
-  octave_throw_interrupt_exception ();
+  octave_rethrow_exception ();
 
   BEGIN_INTERRUPT_IMMEDIATELY_IN_FOREIGN_CODE_2;
 
diff -r 93826ba0d078 -r 78f3811155f7 src/octave.cc
--- a/src/octave.cc     Wed Feb 13 00:00:33 2008 -0500
+++ b/src/octave.cc     Thu Feb 14 17:14:23 2008 -0500
@@ -525,9 +525,20 @@ print_version_and_exit (void)
 }
 
 static void
+lo_error_handler (const char *fmt, ...)
+{
+  va_list args;
+  va_start (args, fmt);
+  verror (fmt, args);
+  va_end (args);
+
+  octave_throw_execution_exception ();
+}
+
+static void
 initialize_error_handlers ()
 {
-  set_liboctave_error_handler (error);
+  set_liboctave_error_handler (lo_error_handler);
   set_liboctave_warning_handler (warning);
   set_liboctave_warning_with_id_handler (warning_with_id);
 }
diff -r 93826ba0d078 -r 78f3811155f7 src/pt-stmt.cc
--- a/src/pt-stmt.cc    Wed Feb 13 00:00:33 2008 -0500
+++ b/src/pt-stmt.cc    Thu Feb 14 17:14:23 2008 -0500
@@ -98,36 +98,44 @@ tree_statement::eval (bool silent, int n
 
       maybe_echo_code (in_function_body);
 
-      if (cmd)
-       cmd->eval ();
-      else
+      try
        {
-         expr->set_print_flag (pf);
+         if (cmd)
+           cmd->eval ();
+         else
+           {
+             expr->set_print_flag (pf);
 
-         // FIXME -- maybe all of this should be packaged in
-         // one virtual function that returns a flag saying whether
-         // or not the expression will take care of binding ans and
-         // printing the result.
+             // FIXME -- maybe all of this should be packaged in
+             // one virtual function that returns a flag saying whether
+             // or not the expression will take care of binding ans and
+             // printing the result.
 
-         // FIXME -- it seems that we should just have to
-         // call expr->rvalue () and that should take care of
-         // everything, binding ans as necessary?
+             // FIXME -- it seems that we should just have to
+             // call expr->rvalue () and that should take care of
+             // everything, binding ans as necessary?
 
-         bool do_bind_ans = false;
+             bool do_bind_ans = false;
 
-         if (expr->is_identifier ())
-           {
-             tree_identifier *id = dynamic_cast<tree_identifier *> (expr);
+             if (expr->is_identifier ())
+               {
+                 tree_identifier *id = dynamic_cast<tree_identifier *> (expr);
 
-             do_bind_ans = (! id->is_variable ());
+                 do_bind_ans = (! id->is_variable ());
+               }
+             else
+               do_bind_ans = (! expr->is_assignment_expression ());
+
+             retval = expr->rvalue (nargout);
+
+             if (do_bind_ans && ! (error_state || retval.empty ()))
+               bind_ans (retval(0), pf);
            }
-         else
-           do_bind_ans = (! expr->is_assignment_expression ());
-
-         retval = expr->rvalue (nargout);
-
-         if (do_bind_ans && ! (error_state || retval.empty ()))
-           bind_ans (retval(0), pf);
+       }
+      catch (octave_execution_exception)
+       {
+         octave_exception_state = octave_no_exception;
+         error ("caught execution error in library function");
        }
 
       unwind_protect::run ();
diff -r 93826ba0d078 -r 78f3811155f7 src/sighandlers.cc
--- a/src/sighandlers.cc        Wed Feb 13 00:00:33 2008 -0500
+++ b/src/sighandlers.cc        Thu Feb 14 17:14:23 2008 -0500
@@ -357,7 +357,7 @@ sigwinch_handler (int /* sig */)
 // for SIGINT only.
 
 static
-void user_abort(const char *sig_name, int sig_number)
+void user_abort (const char *sig_name, int sig_number)
 {
   if (! octave_initialized)
     exit (1);
@@ -378,7 +378,12 @@ void user_abort(const char *sig_name, in
        }
 
       if (octave_interrupt_immediately)
-       octave_jump_to_enclosing_context ();
+       {
+         if (octave_interrupt_state == 0)
+           octave_interrupt_state = 1;
+
+         octave_jump_to_enclosing_context ();
+       }
       else
        {
          // If we are already cleaning up from a previous interrupt,
diff -r 93826ba0d078 -r 78f3811155f7 src/toplev.cc
--- a/src/toplev.cc     Wed Feb 13 00:00:33 2008 -0500
+++ b/src/toplev.cc     Thu Feb 14 17:14:23 2008 -0500
@@ -182,7 +182,7 @@ recover_from_exception (void)
   octave_interrupt_immediately = 0;
   octave_interrupt_state = 0;
   octave_signal_caught = 0;
-  octave_allocation_error = 0;
+  octave_exception_state = octave_no_exception;
   octave_restore_signal_mask ();
   octave_catch_interrupts ();
 }
diff -r 93826ba0d078 -r 78f3811155f7 src/utils.cc
--- a/src/utils.cc      Wed Feb 13 00:00:33 2008 -0500
+++ b/src/utils.cc      Thu Feb 14 17:14:23 2008 -0500
@@ -966,7 +966,7 @@ octave_vformat (std::ostream& os, const 
   delete [] buf; \
   buf = 0; \
   size = initial_size; \
-  octave_throw_interrupt_exception (); \
+  octave_rethrow_exception (); \
   BEGIN_INTERRUPT_IMMEDIATELY_IN_FOREIGN_CODE_2
 
 #if defined __GNUC__ && defined __va_copy

reply via email to

[Prev in Thread] Current Thread [Next in Thread]