[Changeset] Fcellfun problem with ErrorHandler access to message/id and count..

[David Bateman]

The Fcellfun function has the capability of defining an ErrorHandler
function that addresses any errors that occur in the main function. This
error handler is passed the same arguments as the main function but
prepended with a structure containing the last error message, id and the
cell array index at which the error occurred. Thomas Treichl pointed out
to me that in both 3.0.1+ and 3.1.51+ the prepended value is incorrect.

Firstly the index that is passed is zero based, when in should be
one-based, and so we have to add one to the count value in the prepended
structure. Secondly it appears that the error message and id strings
that are passed are empty. The way these messages are currently
generated is to call Flasterr and get the strings from this call. The
reason it was done this way previous is, if I remember correctly, that
John didn't want to expose the Vlast_error_message and Vlast_error_id
variables to be changed outside of the error.cc file. I'm not sure why
the call to lasterr doesn't work, but suggest that we add a few
functions to access the Vlast_error_message and Vlast_error_id variables
without allowing the variables to be changed, as this would be faster
than having to call the lasterr function through feval.

I therefore suggest the attached patch for 3.1.51+.. With this change I
see the following

function n = myhand (S, varargin)
>   disp (S);
>   n = NaN;
> endfunction
octave:2>
octave:2> cellfun(@factorial,{-1,3},'ErrorHandler',@myhand);
{
  identifier =
  message = factorial: n must all be nonnegative integers
  index =  1
}
octave:3> lasterror ()
ans =
{
  message = factorial: n must all be nonnegative integers
  identifier =
  stack =
  {
    file =
/home/adb014/octave/code/octave-build/../octave-hg/scripts/specfun/factorial.m
    name = factorial
    line =  32
    column =  5
  }

}

Unfortunately unless we can figure out why the lasterr call didn't work
I see no fix for 3.0.1+ that doesn't alter the error.h file and thus the
3.0.x API which we want to keep unaltered. So maybe this is a bug that
won't be fixed in 3.0.x.

Regards
David

--
David Bateman                                David.Bateman@...
Motorola Labs - Paris                        [phone] (Ph)
Parc Les Algorithmes, Commune de St Aubin    [phone] (Mob)
91193 Gif-Sur-Yvette FRANCE                  [phone] (Fax)

The information contained in this communication has been classified as:

[x] General Business Information
[ ] Motorola Internal Use Only
[ ] Motorola Confidential Proprietary


# HG changeset patch
# User David Bateman <dbateman@...>
# Date 1217235046 -7200
# Node ID 69c1f8b30600778ffebb02eca00e74d7d3d2b528
# Parent  1f4df6d669cfc63aa0d7af63d3cee3518d7cfbbb
Modify Fcellfun to directly access the error message/id rather than use a call to Flasterr

diff --git a/src/ChangeLog b/src/ChangeLog
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,13 @@ 2008-07-25  John W. Eaton  <jwe@...
[phone]  David Bateman  <dbateman@...>
+
+ * error.cc (last_error_id, last_error_message, last_warning_id,
+ last_warning_message): New functions to return the error/warning
+ message and id without exposing the internal values.
+ * error.h  (last_error_id, last_error_message, last_warning_id,
+ last_warning_message): Declare them.
+ * DLD-FUNCTIONS/cellfun.cc (Fcellfun): Use them to pass the error
+ to the ErrorHandler function.
+
 2008-07-25  John W. Eaton  <jwe@...>
 
  * DLD-FUNCTIONS/det.cc (Fdet): Return calculated determinant for
diff --git a/src/DLD-FUNCTIONS/cellfun.cc b/src/DLD-FUNCTIONS/cellfun.cc
--- a/src/DLD-FUNCTIONS/cellfun.cc
+++ b/src/DLD-FUNCTIONS/cellfun.cc
@@ -364,13 +364,10 @@ cellfun (@@factorial, @{-1,2@},'ErrorHan
 
   if (error_state && have_error_handler)
     {
-      octave_value_list errtmp =
- Flasterr (octave_value_list (), 2);
-
       Octave_map msg;
-      msg.assign ("identifier", errtmp(1));
-      msg.assign ("message", errtmp(0));
-      msg.assign ("index", octave_value(double (count)));
+      msg.assign ("identifier", last_error_id ());
+      msg.assign ("message", last_error_message ());
+      msg.assign ("index", octave_value(double (count + static_cast<octave_idx_type>(1))));
       octave_value_list errlist = inputlist;
       errlist.prepend (msg);
       buffer_error_messages--;
@@ -443,13 +440,10 @@ cellfun (@@factorial, @{-1,2@},'ErrorHan
 
   if (error_state && have_error_handler)
     {
-      octave_value_list errtmp =
- Flasterr (octave_value_list (), 2);
-
       Octave_map msg;
-      msg.assign ("identifier", errtmp(1));
-      msg.assign ("message", errtmp(0));
-      msg.assign ("index", octave_value(double (count)));
+      msg.assign ("identifier", last_error_id ());
+      msg.assign ("message", last_error_message ());
+      msg.assign ("index", octave_value(double (count + static_cast<octave_idx_type>(1))));
       octave_value_list errlist = inputlist;
       errlist.prepend (msg);
       buffer_error_messages--;
diff --git a/src/error.cc b/src/error.cc
--- a/src/error.cc
+++ b/src/error.cc
@@ -1726,6 +1726,26 @@ to enter the debugger when a warning is
   return SET_INTERNAL_VARIABLE (debug_on_warning);
 }
 
+std::string last_error_message (void)
+{
+  return Vlast_error_message;
+}
+
+std::string last_error_id (void)
+{
+  return Vlast_error_id;
+}
+
+std::string last_warning_message (void)
+{
+  return Vlast_warning_message;
+}
+
+std::string last_warning_id (void)
+{
+  return Vlast_warning_id;
+}
+
 /*
 ;;; Local Variables: ***
 ;;; mode: C++ ***
diff --git a/src/error.h b/src/error.h
--- a/src/error.h
+++ b/src/error.h
@@ -114,6 +114,12 @@ extern OCTINTERP_API bool discard_error_
 // TRUE means warning messages are turned off.
 extern OCTINTERP_API bool discard_warning_messages;
 
+// Helper functions to pass last error and warning messages and ids
+extern OCTINTERP_API std::string last_error_message (void);
+extern OCTINTERP_API std::string last_error_id (void);
+extern OCTINTERP_API std::string last_warning_message (void);
+extern OCTINTERP_API std::string last_warning_id (void);
+
 #endif
 
 /*

_______________________________________________
Bug-octave mailing list
Bug-octave@...
https://www.cae.wisc.edu/mailman/listinfo/bug-octave

[John W. Eaton]

On 28-Jul-2008, David Bateman wrote:

| The Fcellfun function has the capability of defining an ErrorHandler
| function that addresses any errors that occur in the main function. This
| error handler is passed the same arguments as the main function but
| prepended with a structure containing the last error message, id and the
| cell array index at which the error occurred. Thomas Treichl pointed out
| to me that in both 3.0.1+ and 3.1.51+ the prepended value is incorrect.
|
| Firstly the index that is passed is zero based, when in should be
| one-based, and so we have to add one to the count value in the prepended
| structure. Secondly it appears that the error message and id strings
| that are passed are empty. The way these messages are currently
| generated is to call Flasterr and get the strings from this call. The
| reason it was done this way previous is, if I remember correctly, that
| John didn't want to expose the Vlast_error_message and Vlast_error_id
| variables to be changed outside of the error.cc file. I'm not sure why
| the call to lasterr doesn't work, but suggest that we add a few
| functions to access the Vlast_error_message and Vlast_error_id variables
| without allowing the variables to be changed, as this would be faster
| than having to call the lasterr function through feval.
|
| I therefore suggest the attached patch for 3.1.51+.. With this change I
| see the following
|
| function n = myhand (S, varargin)
| >   disp (S);
| >   n = NaN;
| > endfunction
| octave:2>
| octave:2> cellfun(@factorial,{-1,3},'ErrorHandler',@myhand);
| {
|   identifier =
|   message = factorial: n must all be nonnegative integers
|   index =  1
| }
| octave:3> lasterror ()
| ans =
| {
|   message = factorial: n must all be nonnegative integers
|   identifier =
|   stack =
|   {
|     file =
| /home/adb014/octave/code/octave-build/../octave-hg/scripts/specfun/factorial.m
|     name = factorial
|     line =  32
|     column =  5
|   }
|
| }
|
| Unfortunately unless we can figure out why the lasterr call didn't work
| I see no fix for 3.0.1+ that doesn't alter the error.h file and thus the
| 3.0.x API which we want to keep unaltered. So maybe this is a bug that
| won't be fixed in 3.0.x.

The following patch seems to fix the problem for me.

I also applied your patch as it seems a cleaner way to do this job
instead of calling Flasterr.

Thanks,

jwe


# HG changeset patch
# User John W. Eaton <jwe@...>
# Date 1217270094 14400
# Node ID 736124a4fa3d2fdaf9c97dd5be26c6ab35f8b349
# Parent  ed4ec7875f98d5edf692400f2845497f1de1fded
lasterr, lasterror: unwind-protect error_state

diff --git a/src/ChangeLog b/src/ChangeLog
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,4 +1,6 @@
 2008-07-28  John W. Eaton  <jwe@...>
+
+ * error.cc (Flasterror, Flasterr): Unwind-protect error_state.
 
  * DLD-FUNCTIONS/__magick_read__.cc (F__magick_write__, write_image):
  New functions.
diff --git a/src/error.cc b/src/error.cc
--- a/src/error.cc
+++ b/src/error.cc
@@ -1441,6 +1441,11 @@
   octave_value retval;
   int nargin = args.length();
 
+  unwind_protect::begin_frame ("Flasterror");
+
+  unwind_protect_int (error_state);
+  error_state = 0;
+
   if (nargin < 2)
     {
       Octave_map err;
@@ -1565,6 +1570,8 @@
   else
     print_usage ();
 
+  unwind_protect::run_frame ("Flasterror");
+
   return retval;  
 }
 
@@ -1577,6 +1584,11 @@
 @end deftypefn")
 {
   octave_value_list retval;
+
+  unwind_protect::begin_frame ("Flasterr");
+
+  unwind_protect_int (error_state);
+  error_state = 0;
 
   int argc = args.length () + 1;
 
@@ -1606,6 +1618,8 @@
     }
   else
     print_usage ();
+
+  unwind_protect::run_frame ("Flasterr");
 
   return retval;  
 }

_______________________________________________
Bug-octave mailing list
Bug-octave@...
https://www.cae.wisc.edu/mailman/listinfo/bug-octave

[dbateman]

John W. Eaton wrote:

On 28-Jul-2008, David Bateman wrote:

The following patch seems to fix the problem for me.

I also applied your patch as it seems a cleaner way to do this job
instead of calling Flasterr.

Could you apply your patch to the 3.0.x tree as well as it will fix the underlying issue there as well.

Regards
D.