changeset 27472:c0883bfc0f36

simplify evaluator logic for try-catch command * pt-eval.cc (tree_evaluator::visit_try_catch_command): Simplify function logic. * error.h, error.cc (make_stack_map, make_stack_frame_list): Now public static functions in the error_system class. Change all uses. (bt_fieldnames, bt_fields): Move inside octave namespace.
author John W. Eaton <jwe@octave.org>
date Fri, 04 Oct 2019 14:13:06 -0400
parents fd32c1a9b1bd
children d503426130bf
files libinterp/corefcn/error.cc libinterp/corefcn/error.h libinterp/parse-tree/pt-eval.cc
diffstat 3 files changed, 119 insertions(+), 112 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/corefcn/error.cc	Fri Oct 04 01:15:13 2019 -0400
+++ b/libinterp/corefcn/error.cc	Fri Oct 04 14:13:06 2019 -0400
@@ -111,78 +111,6 @@
   es.vwarning (id, fmt, args);
 }
 
-// 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<octave::frame_info> frames;
-
-  Cell file = stack.contents ("file");
-  Cell name = stack.contents ("name");
-  Cell line = stack.contents ("line");
-  Cell column;
-  bool have_column = false;
-  if (stack.contains ("column"))
-    {
-      have_column = true;
-      column = stack.contents ("column");
-    }
-
-  octave_idx_type nel = name.numel ();
-
-  for (octave_idx_type i = 0; i < nel; i++)
-    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;
-}
-
 static void
 defun_usage_message (const char *fmt, ...)
 {
@@ -428,6 +356,78 @@
                                   "last_error_id");
   }
 
+  // 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);
+
+  octave_map
+  error_system::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;
+  }
+
+  std::list<octave::frame_info>
+  error_system::make_stack_frame_list (const octave_map& stack)
+  {
+    std::list<octave::frame_info> frames;
+
+    Cell file = stack.contents ("file");
+    Cell name = stack.contents ("name");
+    Cell line = stack.contents ("line");
+    Cell column;
+    bool have_column = false;
+    if (stack.contains ("column"))
+      {
+        have_column = true;
+        column = stack.contents ("column");
+      }
+
+    octave_idx_type nel = name.numel ();
+
+    for (octave_idx_type i = 0; i < nel; i++)
+      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;
+  }
+
   // For given warning ID, return 0 if warnings are disabled, 1 if
   // enabled, and 2 if the given ID should be an error instead of a
   // warning.
@@ -1311,7 +1311,8 @@
           octave_value c = m.getfield ("stack");
 
           if (c.isstruct ())
-            stack_info = make_stack_frame_list (c.map_value ());
+            stack_info
+              = octave::error_system::make_stack_frame_list (c.map_value ());
         }
     }
   else
--- a/libinterp/corefcn/error.h	Fri Oct 04 01:15:13 2019 -0400
+++ b/libinterp/corefcn/error.h	Fri Oct 04 14:13:06 2019 -0400
@@ -234,6 +234,12 @@
       return val;
     }
 
+    static octave_map
+    make_stack_map (const std::list<octave::frame_info>& frames);
+
+    static std::list<octave::frame_info>
+    make_stack_frame_list (const octave_map& stack);
+
     //! For given warning ID, return 0 if warnings are disabled, 1 if
     //! enabled, and 2 if the given ID should be an error instead of a
     //! warning.
--- a/libinterp/parse-tree/pt-eval.cc	Fri Oct 04 01:15:13 2019 -0400
+++ b/libinterp/parse-tree/pt-eval.cc	Fri Oct 04 14:13:06 2019 -0400
@@ -2953,43 +2953,49 @@
         m_echo_file_pos = line + 1;
       }
 
-    error_system& es = m_interpreter.get_error_system ();
-
     bool execution_error = false;
-
-    {
-      // unwind frame before catch block
-      unwind_protect frame;
-
-      interpreter_try (frame);
-
-      // The catch code is *not* added to unwind_protect stack;
-      // it doesn't need to be run on interrupts.
-
-      tree_statement_list *try_code = cmd.body ();
-
-      if (try_code)
-        {
-          try
-            {
-              try_code->accept (*this);
-            }
-          catch (const execution_exception& ee)
-            {
-              es.save_exception (ee);
-              interpreter::recover_from_exception ();
-
-              execution_error = true;
-            }
-        }
-      // Unwind to let the user print any messages from
-      // errors that occurred in the body of the try_catch statement,
-      // or throw further errors.
-    }
+    octave_scalar_map err_map;
+
+    tree_statement_list *try_code = cmd.body ();
+
+    if (try_code)
+      {
+        // unwind frame before catch block
+
+        unwind_protect frame;
+
+        interpreter_try (frame);
+
+        // The catch code is *not* added to unwind_protect stack; it
+        // doesn't need to be run on interrupts.
+
+        try
+          {
+            try_code->accept (*this);
+          }
+        catch (const execution_exception& ee)
+          {
+            execution_error = true;
+
+            error_system& es = m_interpreter.get_error_system ();
+
+            es.save_exception (ee);
+
+            err_map.assign ("message", es.last_error_message ());
+            err_map.assign ("identifier", es.last_error_id ());
+            err_map.assign ("stack", es.last_error_stack ());
+
+            interpreter::recover_from_exception ();
+          }
+
+        // Actions attached to unwind_protect frame will run here, prior
+        // to executing the catch block.
+      }
 
     if (execution_error)
       {
         tree_statement_list *catch_code = cmd.cleanup ();
+
         if (catch_code)
           {
             tree_identifier *expr_id = cmd.identifier ();
@@ -2998,13 +3004,7 @@
               {
                 octave_lvalue ult = expr_id->lvalue (*this);
 
-                octave_scalar_map err;
-
-                err.assign ("message", es.last_error_message ());
-                err.assign ("identifier", es.last_error_id ());
-                err.assign ("stack", es.last_error_stack ());
-
-                ult.assign (octave_value::op_asn_eq, err);
+                ult.assign (octave_value::op_asn_eq, err_map);
               }
 
             // perform actual "catch" block