changeset 28026:262cdfc6faf9

allow reloading of handles to private functions (bug #57439) To allow a handle to a private function to reload the function it references after the function has been cleared and/or when the directory where the function was defined is no longer in the load path, store the directory in the symbol scope for the function, not just in the function itself. Also use the canonical directory name as the key for the private_function map in the load path. * fcn-info.cc (fcn_info::fcn_info_rep::load_private_function): Use canonical directory name as key in private_functions map. (fcn_info::fcn_info_rep::xfind): Get function file and directory names directly from the search scope. (fcn_info::fcn_info_rep::x_builtin_find): Likewise. * load-path.h, load-path.cc (load_path::M_FILE, load_path::OCT_FILE, load_path::MEX_FILE): Now public. (find_private_fcn_file): New static function. (load_path::package_info::remove): Use canonical directory name as key for private function map. (load_path::package_info::add_to_private_fcn_map): Likewise. (load_path::package_info::add_to_method_map): Likewise. (load_path::package_info::remove_private_fcn_map): Likewise. (load_path::package_info::find_private_fcn): Likewise. If function is not found in map, search filesystem. * symscope.h, symscope.cc (symbol_scope_rep::m_fcn_file_name, symbol_scope_rep::m_dir_name): New data members. (symbol_scope::dup): Copy them. (symbol_scope::cache_fcn_file_name, symbol_scope::cache_dir_name, symbol_scope::fcn_file_name, symbol_scope::dir_name, symbol_scope_rep::cache_fcn_file_name, symbol_scope_rep::cache_dir_name, symbol_scope_rep::fcn_file_name, symbol_scope_rep::dir_name): New functions. * oct-parse.yy (base_parser::make_script): Also cache file and directory names in script scope. (base_parser::finish_function): Also cache file and directory names in function scope. * pt-fcn-handle.cc (tree_anon_fcn_handle::evaluate): Also cache file and directory names in new scope associated with the anonymous function object.
author John W. Eaton <jwe@octave.org>
date Thu, 30 Jan 2020 15:12:52 -0500
parents 9d9e01986105
children 2e6dc7e2b191
files libinterp/corefcn/fcn-info.cc libinterp/corefcn/load-path.cc libinterp/corefcn/load-path.h libinterp/corefcn/symscope.cc libinterp/corefcn/symscope.h libinterp/parse-tree/oct-parse.yy libinterp/parse-tree/pt-fcn-handle.cc
diffstat 7 files changed, 142 insertions(+), 55 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/corefcn/fcn-info.cc	Wed Jan 29 08:20:55 2020 -0800
+++ b/libinterp/corefcn/fcn-info.cc	Thu Jan 30 15:12:52 2020 -0500
@@ -86,7 +86,7 @@
 
     tmpfcn->mark_as_private_function (class_name);
 
-    private_functions[dir_name] = ov_fcn;
+    private_functions[octave::sys::canonicalize_file_name (dir_name)] = ov_fcn;
 
     return ov_fcn;
   }
@@ -628,26 +628,20 @@
   fcn_info::fcn_info_rep::xfind (const symbol_scope& search_scope,
                                  const octave_value_list& args)
   {
-    octave_user_code *curr_code
-      = search_scope ? search_scope.user_code () : nullptr;
-
-    // Subfunction.  I think it only makes sense to check for
-    // subfunctions if we are currently executing a function defined
-    // from a .m file.
+    // Subfunction, local function, or private function.
 
     if (search_scope)
       {
+        // Subfunction.
+
         octave_value fcn = search_scope.find_subfunction (name);
 
         if (fcn.is_defined ())
           return fcn;
-      }
-
-    // Local function.
 
-    if (curr_code)
-      {
-        std::string fcn_file = curr_code->fcn_file_name ();
+        // Local function.
+
+        std::string fcn_file = search_scope.fcn_file_name ();
 
         // For anonymous functions we look at the parent scope so that if
         // they were defined within class methods and use local functions
@@ -667,13 +661,10 @@
                 return r->second;
               }
           }
-      }
-
-    // Private function.
 
-    if (curr_code)
-      {
-        std::string dir_name = curr_code->dir_name ();
+        // Private function.
+
+        std::string dir_name = search_scope.dir_name ();
 
         if (! dir_name.empty ())
           {
@@ -847,14 +838,13 @@
     if (cmdline_function.is_defined ())
       return cmdline_function;
 
-    // Private function.
+    // Private function, local function, or subfunction.
 
-    octave_user_code *curr_code
-      = search_scope ? search_scope.user_code () : nullptr;
+    if (search_scope)
+      {
+        // Private function.
 
-    if (curr_code)
-      {
-        std::string dir_name = curr_code->dir_name ();
+        std::string dir_name = search_scope.dir_name ();
 
         if (! dir_name.empty ())
           {
@@ -885,13 +875,10 @@
                   }
               }
           }
-      }
-
-    // Local function.
 
-    if (curr_code)
-      {
-        std::string fcn_file = curr_code->fcn_file_name ();
+        // Local function.
+
+        std::string fcn_file = search_scope.fcn_file_name ();
 
         if (! fcn_file.empty ())
           {
@@ -906,14 +893,11 @@
                 return r->second;
               }
           }
-      }
 
-    // Subfunction.  I think it only makes sense to check for
-    // subfunctions if we are currently executing a function defined
-    // from a .m file.
+        // Subfunction.  I think it only makes sense to check for
+        // subfunctions if we are currently executing a function defined
+        // from a .m file.
 
-    if (search_scope)
-      {
         octave_value val = search_scope.find_subfunction (name);
 
         if (val.is_defined ())
--- a/libinterp/corefcn/load-path.cc	Wed Jan 29 08:20:55 2020 -0800
+++ b/libinterp/corefcn/load-path.cc	Thu Jan 30 15:12:52 2020 -0500
@@ -185,6 +185,46 @@
     return retval;
   }
 
+  static std::string find_private_fcn_file (const std::string& dir,
+                                            const std::string& fcn,
+                                            int type)
+  {
+    std::string nm
+      = sys::file_ops::concat (sys::file_ops::concat (dir, "private"), fcn);
+
+    if (type & load_path::OCT_FILE)
+      {
+        std::string fnm = nm + ".oct";
+
+        sys::file_stat fs (fnm);
+
+        if (fs.exists () && fs.is_reg ())
+          return fnm;
+      }
+
+    if (type & load_path::MEX_FILE)
+      {
+        std::string fnm = nm + ".mex";
+
+        sys::file_stat fs (fnm);
+
+        if (fs.exists () && fs.is_reg ())
+          return fnm;
+      }
+
+    if (type & load_path::M_FILE)
+      {
+        std::string fnm = nm + ".m";
+
+        sys::file_stat fs (fnm);
+
+        if (fs.exists () && fs.is_reg ())
+          return fnm;
+      }
+
+    return "";
+  }
+
   // True if a path is contained in a path list separated by path_sep_char
 
   static bool
@@ -1525,7 +1565,7 @@
 
     remove_fcn_map (dir, fcn_files);
 
-    remove_private_fcn_map (dir);
+    remove_private_fcn_map (sys::canonicalize_file_name (dir));
 
     remove_method_map (dir);
   }
@@ -1656,25 +1696,28 @@
     std::string retval;
 
     //  update ();
-
-    const_private_fcn_map_iterator q = private_fcn_map.find (dir);
+    std::string canon_dir = sys::canonicalize_file_name (dir);
+
+    const_private_fcn_map_iterator q = private_fcn_map.find (canon_dir);
 
     if (q != private_fcn_map.end ())
       {
-        const dir_info::fcn_file_map_type& m = q->second;
-
-        dir_info::const_fcn_file_map_iterator p = m.find (fcn);
-
-        if (p != m.end ())
+        const dir_info::fcn_file_map_type& fcn_file_map = q->second;
+
+        dir_info::const_fcn_file_map_iterator p = fcn_file_map.find (fcn);
+
+        if (p != fcn_file_map.end ())
           {
             std::string fname
-              = sys::file_ops::concat (sys::file_ops::concat (dir, "private"), fcn);
+              = sys::file_ops::concat (sys::file_ops::concat (canon_dir, "private"), fcn);
 
             if (check_file_type (fname, type, p->second, fcn,
                                  "load_path::find_private_fcn"))
               retval = fname;
           }
       }
+    else
+      retval = find_private_fcn_file (canon_dir, fcn, type);
 
     return retval;
   }
@@ -1892,7 +1935,8 @@
     dir_info::fcn_file_map_type private_file_map = di.private_file_map;
 
     if (! private_file_map.empty ())
-      private_fcn_map[di.dir_name] = private_file_map;
+      private_fcn_map[sys::canonicalize_file_name (di.dir_name)]
+        = private_file_map;
   }
 
   void
@@ -1955,7 +1999,8 @@
         dir_info::fcn_file_map_type private_file_map = ci.private_file_map;
 
         if (! private_file_map.empty ())
-          private_fcn_map[full_dir_name] = private_file_map;
+          private_fcn_map[sys::canonicalize_file_name (full_dir_name)]
+            = private_file_map;
       }
   }
 
@@ -2094,7 +2139,7 @@
   void
   load_path::package_info::remove_private_fcn_map (const std::string& dir)
   {
-    auto p = private_fcn_map.find (dir);
+    auto p = private_fcn_map.find (sys::canonicalize_file_name (dir));
 
     if (p != private_fcn_map.end ())
       private_fcn_map.erase (p);
--- a/libinterp/corefcn/load-path.h	Wed Jan 29 08:20:55 2020 -0800
+++ b/libinterp/corefcn/load-path.h	Thu Jan 30 15:12:52 2020 -0500
@@ -209,12 +209,12 @@
 
     std::string system_path (void) const { return sys_path; }
 
-  private:
-
     static const int M_FILE = 1;
     static const int OCT_FILE = 2;
     static const int MEX_FILE = 4;
 
+  private:
+
     class dir_info
     {
     public:
--- a/libinterp/corefcn/symscope.cc	Wed Jan 29 08:20:55 2020 -0800
+++ b/libinterp/corefcn/symscope.cc	Thu Jan 30 15:12:52 2020 -0500
@@ -29,6 +29,8 @@
 
 #include <sstream>
 
+#include "file-ops.h"
+
 #include "fcn-info.h"
 #include "interpreter-private.h"
 #include "interpreter.h"
@@ -169,6 +171,12 @@
     m_primary_parent = std::weak_ptr<symbol_scope_rep> (parent);
   }
 
+  void
+  symbol_scope_rep::cache_dir_name (const std::string& name)
+  {
+    m_dir_name = octave::sys::canonicalize_file_name (name);
+  }
+
   bool
   symbol_scope_rep::is_relative (const std::shared_ptr<symbol_scope_rep>& scope) const
   {
--- a/libinterp/corefcn/symscope.h	Wed Jan 29 08:20:55 2020 -0800
+++ b/libinterp/corefcn/symscope.h	Thu Jan 30 15:12:52 2020 -0500
@@ -40,7 +40,7 @@
 #include "oct-refcount.h"
 
 class tree_argument_list;
-class octave_user_function;
+class octave_user_code;
 
 #include "ov.h"
 #include "ovl.h"
@@ -67,9 +67,9 @@
 
     symbol_scope_rep (const std::string& name = "")
       : m_name (name), m_symbols (), m_subfunctions (),
-        m_persistent_values (), m_code (nullptr), m_parent (),
-        m_primary_parent (), m_children (), m_nesting_depth (0),
-        m_is_static (false)
+        m_persistent_values (), m_code (nullptr), m_fcn_file_name (),
+        m_dir_name (), m_parent (), m_primary_parent (), m_children (),
+        m_nesting_depth (0), m_is_static (false)
     {
       // All scopes have ans as the first symbol, initially undefined.
 
@@ -126,6 +126,8 @@
       new_sid->m_persistent_values = m_persistent_values;
       new_sid->m_subfunction_names = m_subfunction_names;
       new_sid->m_code = m_code;
+      new_sid->m_fcn_file_name = m_fcn_file_name;
+      new_sid->m_dir_name = m_dir_name;
       new_sid->m_parent = m_parent;
       new_sid->m_primary_parent = m_primary_parent;
       new_sid->m_children = m_children;
@@ -252,6 +254,17 @@
 
     void set_primary_parent (const std::shared_ptr<symbol_scope_rep>& parent);
 
+    void cache_fcn_file_name (const std::string& name)
+    {
+      m_fcn_file_name = name;
+    }
+
+    void cache_dir_name (const std::string& name);
+
+    std::string fcn_file_name (void) const { return m_fcn_file_name; }
+
+    std::string dir_name (void) const { return m_dir_name; }
+
     bool is_relative (const std::shared_ptr<symbol_scope_rep>& scope) const;
 
     void update_nest (void);
@@ -300,6 +313,14 @@
 
     octave_user_code *m_code;
 
+    //! The file name associated with m_code.
+
+    std::string m_fcn_file_name;
+
+    //! The directory associated with m_code.
+
+    std::string m_dir_name;
+
     //! Parent of nested function (may be null).
 
     std::weak_ptr<symbol_scope_rep> m_parent;
@@ -550,6 +571,28 @@
         m_rep->set_primary_parent (p.get_rep ());
     }
 
+    void cache_fcn_file_name (const std::string& name)
+    {
+      if (m_rep)
+        m_rep->cache_fcn_file_name (name);
+    }
+
+    void cache_dir_name (const std::string& name)
+    {
+      if (m_rep)
+        m_rep->cache_dir_name (name);
+    }
+
+    std::string fcn_file_name (void) const
+    {
+      return m_rep ? m_rep->fcn_file_name () : "";
+    }
+
+    std::string dir_name (void) const
+    {
+      return m_rep ? m_rep->dir_name () : "";
+    }
+
     bool is_relative (const symbol_scope& scope) const
     {
       return m_rep ? m_rep->is_relative (scope.get_rep ()) : false;
--- a/libinterp/parse-tree/oct-parse.yy	Wed Jan 29 08:20:55 2020 -0800
+++ b/libinterp/parse-tree/oct-parse.yy	Thu Jan 30 15:12:52 2020 -0500
@@ -3424,6 +3424,8 @@
     symbol_scope script_scope = m_lexer.m_symtab_context.curr_scope ();
 
     script_scope.cache_name (m_lexer.m_fcn_file_full_name);
+    script_scope.cache_fcn_file_name (m_lexer.m_fcn_file_full_name);
+    script_scope.cache_dir_name (m_lexer.m_dir_name);
 
     octave_user_script *script
       = new octave_user_script (m_lexer.m_fcn_file_full_name,
@@ -3666,6 +3668,8 @@
 
         symbol_scope fcn_scope = fcn->scope ();
         fcn_scope.cache_name (tmp);
+        fcn_scope.cache_fcn_file_name (file);
+        fcn_scope.cache_dir_name (m_lexer.m_dir_name);
 
         if (lc)
           fcn->stash_leading_comment (lc);
--- a/libinterp/parse-tree/pt-fcn-handle.cc	Wed Jan 29 08:20:55 2020 -0800
+++ b/libinterp/parse-tree/pt-fcn-handle.cc	Thu Jan 30 15:12:52 2020 -0500
@@ -158,6 +158,9 @@
         af->stash_parent_fcn_name (curr_fcn->name ());
         af->stash_dir_name (curr_fcn->dir_name ());
 
+        new_scope.cache_fcn_file_name (curr_fcn->fcn_file_name ());
+        new_scope.cache_dir_name (curr_fcn->dir_name ());
+
         // The following is needed so that class method dispatch works
         // properly for anonymous functions that wrap class methods.