changeset 25372:97f1d513aaf6 stable

defer deletion of temporaries in argument lists (bug #53844) * pt-eval.h, pt-eval.cc (tree_evaluator::deferred_delete_stack): New class. (tree_evaluator::m_deferred_delete_stack): New data member. (tree_evaluator::tree_evaluator): Initialize it. (tree_evaluator::reset): Clear it. (tree_evaluator::defer_deletion): New function. (tree_evaluator::visit_statement): Arrange to clear deferred deletion stack at end of each statement evaluation. * pt-arg-list.cc (tree_argument_list::convert_to_const_vector): Defer deletion of temporary values in argument lists.
author John W. Eaton <jwe@octave.org>
date Mon, 14 May 2018 14:47:19 -0400
parents 2205c0ca02e7
children c8f49ee7a687
files libinterp/parse-tree/pt-arg-list.cc libinterp/parse-tree/pt-eval.cc libinterp/parse-tree/pt-eval.h
diffstat 3 files changed, 83 insertions(+), 3 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/parse-tree/pt-arg-list.cc	Fri May 11 16:46:27 2018 -0400
+++ b/libinterp/parse-tree/pt-arg-list.cc	Mon May 14 14:47:19 2018 -0400
@@ -247,12 +247,29 @@
 
         if (elt)
           {
+            bool is_assignment = elt->is_assignment_expression ();
+
             octave_value tmp = tw->evaluate (elt);
 
             if (tmp.is_cs_list ())
               args.push_back (tmp.list_value ());
             else if (tmp.is_defined ())
-              args.push_back (tmp);
+              {
+                args.push_back (tmp);
+
+                // Defer deletion of any temporary values until the end
+                // of the containing statement.  That way destructors
+                // for temporary classdef handle objects will be called
+                // when it is safe to do so.
+                //
+                // FIXME: We could further limit this action to classdef
+                // handle objects, but we don't currently have a
+                // octave_value predicate for that so should add it on
+                // the default branch, not stable.
+
+                if (! is_assignment)
+                  tw->defer_deletion (tmp);
+              }
           }
         else
           {
--- a/libinterp/parse-tree/pt-eval.cc	Fri May 11 16:46:27 2018 -0400
+++ b/libinterp/parse-tree/pt-eval.cc	Mon May 14 14:47:19 2018 -0400
@@ -81,6 +81,7 @@
     m_result_type = RT_UNDEFINED;
     m_expr_result_value = octave_value ();
     m_expr_result_value_list = octave_value_list ();
+    m_deferred_delete_stack.clear ();
     m_lvalue_list_stack.clear ();
     m_nargout_stack.clear ();
   }
@@ -2270,6 +2271,13 @@
               cmd->accept (*this);
             else
               {
+                unwind_protect frame;
+
+                frame.add_method (m_deferred_delete_stack,
+                                  &deferred_delete_stack::pop_frame);
+
+                m_deferred_delete_stack.mark ();
+
                 if (m_echo_state)
                   {
                     size_t line = stmt.line ();
--- a/libinterp/parse-tree/pt-eval.h	Fri May 11 16:46:27 2018 -0400
+++ b/libinterp/parse-tree/pt-eval.h	Mon May 14 14:47:19 2018 -0400
@@ -67,6 +67,54 @@
       ECHO_ALL = 4
     };
 
+    class deferred_delete_stack
+    {
+    public:
+
+      deferred_delete_stack (void) = default;
+
+      deferred_delete_stack (const deferred_delete_stack&) = default;
+
+      deferred_delete_stack& operator = (const deferred_delete_stack&) = default;
+
+      ~deferred_delete_stack (void) = default;
+
+      // An undefined value on the stack marks the boundary of the
+      // current frame.
+
+      void mark (void) { push (octave_value ()); }
+
+      void push (const octave_value& val) { m_stack.push (val); }
+
+      void pop_frame (void)
+      {
+        while (! m_stack.empty ())
+          {
+            octave_value val = val_pop ();
+
+            if (val.is_undefined ())
+              break;
+          }
+      }
+
+      void clear (void)
+      {
+        while (! m_stack.empty ())
+          m_stack.pop ();
+      }
+
+    private:
+
+      std::stack<octave_value> m_stack;
+
+      octave_value val_pop (void)
+      {
+        octave_value retval = m_stack.top ();
+        m_stack.pop ();
+        return retval;
+      }
+    };
+
     template <typename T>
     class value_stack
     {
@@ -125,8 +173,8 @@
     tree_evaluator (interpreter& interp)
       : m_interpreter (interp), m_result_type (RT_UNDEFINED),
         m_expr_result_value (), m_expr_result_value_list (),
-        m_lvalue_list_stack (), m_nargout_stack (),
-        m_call_stack (interp), m_profiler (),
+        m_deferred_delete_stack (), m_lvalue_list_stack (),
+        m_nargout_stack (), m_call_stack (interp), m_profiler (),
         m_max_recursion_depth (256), m_silent_functions (false),
         m_string_fill_char (' '), m_PS4 ("+ "), m_echo (ECHO_OFF),
         m_echo_state (false), m_echo_file_name (), m_echo_file_pos (1),
@@ -267,6 +315,11 @@
     // TRUE means we are evaluating some kind of looping construct.
     static bool in_loop_command;
 
+    void defer_deletion (const octave_value& val)
+    {
+      m_deferred_delete_stack.push (val);
+    }
+
     Matrix ignored_fcn_outputs (void) const;
 
     bool isargout (int nargout, int iout) const;
@@ -487,6 +540,8 @@
     octave_value m_expr_result_value;
     octave_value_list m_expr_result_value_list;
 
+    deferred_delete_stack m_deferred_delete_stack;
+
     value_stack<const std::list<octave_lvalue>*> m_lvalue_list_stack;
 
     value_stack<int> m_nargout_stack;