changeset 25377:b75d55b3dbb7

maint: Merge stable to default.
author John W. Eaton <jwe@octave.org>
date Tue, 15 May 2018 01:28:40 -0400
parents f9ed57ecd3b4 (current diff) cc40e47d3a44 (diff)
children 71cc81aad717
files libinterp/corefcn/load-path.cc libinterp/corefcn/load-path.h libinterp/octave-value/ov-classdef.cc libinterp/octave-value/ov-classdef.h libinterp/parse-tree/pt-arg-list.cc libinterp/parse-tree/pt-eval.cc libinterp/parse-tree/pt-eval.h
diffstat 9 files changed, 211 insertions(+), 81 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/corefcn/load-path.cc	Mon May 14 03:47:19 2018 +0200
+++ b/libinterp/corefcn/load-path.cc	Tue May 15 01:28:40 2018 -0400
@@ -1105,7 +1105,15 @@
                       t = load_path::MEX_FILE;
 
                     if (t)
-                      retval[base] = t;
+                      {
+                        load_path::dir_info::fcn_file_map_iterator p
+                          = retval.find (base);
+
+                        if (p == retval.end ())
+                          retval[base] = t;
+                        else
+                          p->second |= t;
+                      }
                   }
               }
           }
@@ -1124,6 +1132,9 @@
   {
     sys::file_stat fs (dir_name);
 
+    sys::file_stat pfs (sys::file_ops::concat (dir_name, "private"));
+    bool has_private_dir = pfs && pfs.is_dir ();
+
     if (! fs)
       {
         std::string msg = fs.error ();
@@ -1148,8 +1159,11 @@
                     // slow things down tremendously for large directories.
                     const dir_info& di = p->second;
 
-                    if (fs.mtime () + fs.time_resolution ()
-                        > di.dir_time_last_checked)
+                    if ((fs.mtime () + fs.time_resolution ()
+                         > di.dir_time_last_checked)
+                        || (has_private_dir
+                            && (pfs.mtime () + pfs.time_resolution ()
+                                > dir_time_last_checked)))
                       initialize ();
                     else
                       {
@@ -1179,7 +1193,10 @@
               }
           }
         // Absolute path, check timestamp to see whether it requires re-caching
-        else if (fs.mtime () + fs.time_resolution () > dir_time_last_checked)
+        else if (fs.mtime () + fs.time_resolution () > dir_time_last_checked
+                 || (has_private_dir
+                     && (pfs.mtime () + pfs.time_resolution ()
+                         > dir_time_last_checked)))
           initialize ();
       }
   }
--- a/libinterp/corefcn/load-path.h	Mon May 14 03:47:19 2018 +0200
+++ b/libinterp/corefcn/load-path.h	Tue May 15 01:28:40 2018 -0400
@@ -368,7 +368,7 @@
     typedef fcn_map_type::const_iterator const_fcn_map_iterator;
     typedef fcn_map_type::iterator fcn_map_iterator;
 
-    // <DIR_NAME, <FCN_NAME, TYPE>>
+    // <DIR_NAME, <FCN_NAME, TYPES>>
     typedef std::map<std::string, dir_info::fcn_file_map_type>
       private_fcn_map_type;
 
--- a/libinterp/octave-value/ov-classdef.cc	Mon May 14 03:47:19 2018 +0200
+++ b/libinterp/octave-value/ov-classdef.cc	Tue May 15 01:28:40 2018 -0400
@@ -651,7 +651,7 @@
 {
   octave_value_list retval;
 
-  // FIXME: implement this
+  // FIXME: implement this.  Wait, what is this supposed to do?
 
   return retval;
 }
@@ -1239,6 +1239,72 @@
 
 //----------------------------------------------------------------------------
 
+void
+cdef_object_rep::release (const cdef_object& obj)
+{
+  // We need to be careful to keep a reference to the object if we are
+  // calling the delete method.  The object is passed to the delete
+  // method as an argument and if the count is already zero when we
+  // do that, then we will increment the count while creating the
+  // argument list for the delete method and then it will be decremented
+  // back to zero and we'll find ourselves in an infinite loop.
+
+  if (refcount - 1 > static_count ())
+    {
+      --refcount;
+      return;
+    }
+
+  if (is_handle_object () && ! is_meta_object ())
+    {
+      octave::unwind_protect frame;
+
+      // Clear interrupts.
+      frame.protect_var (octave_interrupt_state);
+      octave_interrupt_state = 0;
+
+      // Disallow quit().
+      frame.protect_var (quit_allowed);
+      quit_allowed = false;
+
+      interpreter_try (frame);
+
+      try
+        {
+          // Call classdef "delete()" method on object
+          get_class ().delete_object (obj);
+        }
+      catch (const octave::interrupt_exception&)
+        {
+          octave::interpreter::recover_from_exception ();
+
+          warning ("interrupt occurred in handle class delete method");
+        }
+      catch (const octave::execution_exception&)
+        {
+          std::string msg = last_error_message ();
+          warning ("error caught while executing handle class delete method:\n%s\n",
+                   msg.c_str ());
+
+        }
+      catch (const octave::exit_exception&)
+        {
+          // This shouldn't happen since we disabled quit above.
+          warning ("exit disabled while executing handle class delete method");
+        }
+      catch (...) // Yes, the black hole.  We're in a d-tor.
+        {
+          // This shouldn't happen, in theory.
+          warning ("internal error: unhandled exception in handle class delete method");
+        }
+    }
+
+  // Now it is safe to set the count to zero.
+  refcount--;
+
+  destroy ();
+}
+
 octave_map
 cdef_object::map_value (void) const
 {
@@ -1813,47 +1879,6 @@
 
 handle_cdef_object::~handle_cdef_object (void)
 {
-  octave::unwind_protect frame;
-
-  // Clear interrupts.
-  frame.protect_var (octave_interrupt_state);
-  octave_interrupt_state = 0;
-
-  // Disallow quit().
-  frame.protect_var (quit_allowed);
-  quit_allowed = false;
-
-  interpreter_try (frame);
-
-  try
-    {
-      // Call classdef "delete()" method on object
-      get_class ().delete_object (get_class ());
-    }
-  catch (const octave::interrupt_exception&)
-    {
-      octave::interpreter::recover_from_exception ();
-
-      warning ("interrupt occurred in handle class delete method");
-    }
-  catch (const octave::execution_exception&)
-    {
-      std::string msg = last_error_message ();
-      warning ("error caught while executing handle class delete method:\n%s\n",
-               msg.c_str ());
-
-    }
-  catch (const octave::exit_exception&)
-    {
-      // This shouldn't happen since we disabled quit above.
-      warning ("exit disabled while executing handle class delete method");
-    }
-  catch (...) // Yes, the black hole.  We're in a d-tor.
-    {
-      // This shouldn't happen, in theory.
-      warning ("internal error: unhandled exception in handle class delete method");
-    }
-
 #if DEBUG_TRACE
   std::cerr << "deleting " << get_class ().get_name ()
             << " object (handle)" << std::endl;
@@ -2322,20 +2347,12 @@
 }
 
 void
-cdef_class::cdef_class_rep::delete_object (cdef_object obj)
+cdef_class::cdef_class_rep::delete_object (const cdef_object& obj)
 {
-  auto it = method_map.find ("delete");
-
-  if (it != method_map.end ())
-    {
-      cdef_class cls = obj.get_class ();
-
-      obj.set_class (wrap ());
-
-      it->second.execute (obj, octave_value_list (), 0, false);
-
-      obj.set_class (cls);
-    }
+  cdef_method dtor = find_method ("delete");
+
+  if (dtor.ok ())
+    dtor.execute (obj, octave_value_list (), 0, true, "destructor");
 
   // FIXME: should we destroy corresponding properties here?
 
@@ -2347,7 +2364,8 @@
     {
       cdef_class cls = lookup_class (super_classes(i));
 
-      cls.delete_object (obj);
+      if (cls.get_name () != "handle")
+        cls.delete_object (obj);
     }
 }
 
--- a/libinterp/octave-value/ov-classdef.h	Mon May 14 03:47:19 2018 +0200
+++ b/libinterp/octave-value/ov-classdef.h	Tue May 15 01:28:40 2018 -0400
@@ -177,11 +177,7 @@
 
   virtual void destroy (void) { delete this; }
 
-  void release (void)
-  {
-    if (--refcount == static_count ())
-      destroy ();
-  }
+  void release (const cdef_object& obj);
 
   virtual dim_vector dims (void) const { return dim_vector (); }
 
@@ -219,13 +215,15 @@
     : rep (r) { }
 
   virtual ~cdef_object (void)
-  { rep->release (); }
+  {
+    rep->release (*this);
+  }
 
   cdef_object& operator = (const cdef_object& obj)
   {
     if (rep != obj.rep)
       {
-        rep->release ();
+        rep->release (*this);
 
         rep = obj.rep;
         rep->refcount++;
@@ -710,7 +708,7 @@
 
     std::string get_directory (void) const { return directory; }
 
-    void delete_object (cdef_object obj);
+    void delete_object (const cdef_object& obj);
 
     octave_value_list
     meta_subsref (const std::string& type,
@@ -885,7 +883,7 @@
   bool is_builtin (void) const
   { return get_directory ().empty (); }
 
-  void delete_object (cdef_object obj)
+  void delete_object (const cdef_object& obj)
   { get_rep ()->delete_object (obj); }
 
   //! Analyze the tree_classdef tree and transform it to a cdef_class
--- a/libinterp/parse-tree/pt-arg-list.cc	Mon May 14 03:47:19 2018 +0200
+++ b/libinterp/parse-tree/pt-arg-list.cc	Tue May 15 01:28:40 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 an
+                // 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	Mon May 14 03:47:19 2018 +0200
+++ b/libinterp/parse-tree/pt-eval.cc	Tue May 15 01:28:40 2018 -0400
@@ -78,6 +78,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 ();
   }
@@ -1366,6 +1367,16 @@
     if (base_expr_val.is_undefined ())
       base_expr_val = evaluate (expr);
 
+    // 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 an octave_value predicate for
+    // that so should add it on the default branch, not stable.
+
+    defer_deletion (base_expr_val);
+
     // If we are indexing an object or looking at something like
     //
     //   classname.static_function (args, ...);
@@ -2267,6 +2278,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	Mon May 14 03:47:19 2018 +0200
+++ b/libinterp/parse-tree/pt-eval.h	Tue May 15 01:28:40 2018 -0400
@@ -70,6 +70,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
     {
@@ -128,12 +176,13 @@
     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_bp_table (*this), m_call_stack (interp), m_profiler (),
-        m_max_recursion_depth (256), m_silent_functions (false),
-        m_string_fill_char (' '), m_PS4 ("+ "), m_dbstep_flag (0),
-        m_echo (ECHO_OFF), m_echo_state (false), m_echo_file_name (),
-        m_echo_file_pos (1), m_echo_files (), m_in_loop_command (false)
+        m_deferred_delete_stack (), m_lvalue_list_stack (),
+        m_nargout_stack (), m_bp_table (*this), m_call_stack (interp),
+        m_profiler (), m_max_recursion_depth (256),
+        m_silent_functions (false), m_string_fill_char (' '),
+        m_PS4 ("+ "), m_dbstep_flag (0), m_echo (ECHO_OFF),
+        m_echo_state (false), m_echo_file_name (), m_echo_file_pos (1),
+        m_echo_files (), m_in_loop_command (false)
     { }
 
     // No copying!
@@ -265,6 +314,11 @@
     // The context for the current evaluation.
     static stmt_list_type statement_context;
 
+    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;
@@ -493,6 +547,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;
--- a/test/bug-46497/bug-46497.tst	Mon May 14 03:47:19 2018 +0200
+++ b/test/bug-46497/bug-46497.tst	Tue May 15 01:28:40 2018 -0400
@@ -18,7 +18,7 @@
 
 %!test
 %! global __bug46497_global__
-%! __bug46497_global__ = 'test_bug46497';
-%! a = class_bug46497 ();
+%! __bug46497_global__ = "test_bug46497";
+%! a = class_bug46497 (13);
 %! a = [];
-%! assert(__bug46497_global__,'deleted');
\ No newline at end of file
+%! assert(__bug46497_global__, struct ("myprop", 13, "status", "deleted"));
--- a/test/bug-46497/class_bug46497.m	Mon May 14 03:47:19 2018 +0200
+++ b/test/bug-46497/class_bug46497.m	Tue May 15 01:28:40 2018 -0400
@@ -1,8 +1,14 @@
 classdef class_bug46497 < handle
+  properties
+    myprop;
+  endproperties
   methods
+    function obj = class_bug46497 (x)
+      obj.myprop = x;
+    endfunction
     function delete (self)
       global __bug46497_global__
-      __bug46497_global__ = 'deleted';
+      __bug46497_global__ = struct ("myprop", self.myprop, "status", "deleted");
     endfunction
   endmethods
 endclassdef