Re: Using exceptions in the core

[John W. Eaton]

On 09/25/2015 02:11 PM, Rik wrote:
> 9/25/15
>
> jwe,
>
> More convincing evidence that this is the way to go.  There is an obscure
> bug where a uicontrol object (button) with a callback function that calls
> error() can produce a segfault
> (https://savannah.gnu.org/bugs/index.php?46038).  With your exception patch
> applied it no longer segfaults.  I wouldn't want to try and debug where in
> the old code the error_state variable was not getting checked correctly.

I agree that we should make the change.  The delay in committing it has 
happened because I need to clean up some parts of the patch, update some 
documentation, write a NEWS file entry and a commit message.

I've also been looking at the way error_state is used and seeing what 
will be necessary to remove it from the Octave sources.  Again, it's not 
necessary to do that immediately, but starting to do that job has shown 
me that some code will not work the same as before without a little bit 
of effort.

Some code is trivial to convert.  For example, things like

  DEFUN (foobar, args, nargout, "foobar doc string", )
  {
    octave_value retval;

    some_operation_that_may_call_error ();

    if (error_state)
      return retval;

    // normal processing...
  }

are easy to handle because they just expect the function 
some_operation_that_may_call_error to print the error message and then 
they return.  So rewriting this function as

  DEFUN (foobar, args, nargout, "foobar doc string", )
  {
    octave_value retval;

    some_operation_that_may_call_error ();

    // normal processing...
  }

is all that is needed.

But in other cases, more changes are needed, or we have to accept 
different behavior.  For example we currently have things like this

  std::string val = args(0).string_value ();

  if (! error_state)
    {
      // normal processing...
    }
  else
    error ("foobar: first argument must be a character string");

With the change to have the error function throw an exception, the 
octave_value::string_value extractor will throw an error and so the 
subsequent more useful message will not appear.  I think in these cases, 
the fix is to write

  if (! args(0).is_string ())
    error ("foobar: first argument must be a character string");

  // normal processing

But there are other cases of extracting values that are not as clear and 
may need more work.  I'll try to classify and summarize the most common 
cases and explain them here before I commit my changes.  Then at least 
we could come up with some guidelines for how to handle the various 
cases so that we can try to maintain some consistency in the Octave 
sources.  Once we've decided, we can also add some style pointers to the 
manual.

jwe