# HG changeset patch
# User Max Brister <address@hidden>
# Date 1331260537 25200
# Node ID fbe54c9e8eb7f94d68603e549211a13988c4cb44
# Parent  c1f9b54350f93099170787589f3ea6b0b178f703
Nested function implementation/tests

diff -r c1f9b54350f9 -r fbe54c9e8eb7 src/oct-parse.yy
--- a/src/oct-parse.yy	Fri Feb 24 20:42:29 2012 -0500
+++ b/src/oct-parse.yy	Thu Mar 08 19:35:37 2012 -0700
@@ -115,6 +115,11 @@
 //     nested function.
 static int current_function_depth = 0;
 
+// A stack holding the nested function scopes being parsed.
+// We don't use std::stack, because we want the clear method. Also, we
+// must access one from the top
+static std::vector<symbol_table::scope_id> function_scopes;
+
 // Maximum function depth detected. Just here to determine whether
 // we have nested functions or just implicitly ended subfunctions.
 static int max_function_depth = 0;
@@ -1226,6 +1231,8 @@
 
                     symbol_table::set_scope (symbol_table::alloc_scope ());
 
+		    function_scopes.push_back (symbol_table::current_scope ());
+
                     if (! reading_script_file && current_function_depth == 1
                         && ! parsing_subfunctions)
                       primary_fcn_scope = symbol_table::current_scope ();
@@ -2862,7 +2869,11 @@
       if (current_function_depth > 1 || parsing_subfunctions)
         {
           fcn->stash_parent_fcn_name (curr_fcn_file_name);
-          fcn->stash_parent_fcn_scope (primary_fcn_scope);
+
+	  if (current_function_depth > 1)
+	    fcn->stash_parent_fcn_scope (function_scopes[function_scopes.size() - 2]);
+	  else
+	    fcn->stash_parent_fcn_scope (primary_fcn_scope);
         }
 
       if (lexer_flags.parsing_class_method)
@@ -2934,14 +2945,28 @@
 
       fcn->define_ret_list (ret_list);
 
+      assert (current_function_depth == function_scopes.size());
       if (current_function_depth > 1 || parsing_subfunctions)
         {
-          fcn->mark_as_subfunction ();
-
-          symbol_table::install_subfunction (nm, octave_value (fcn),
-                                             primary_fcn_scope);
+	  fcn->mark_as_subfunction ();
+	  if (endfunction_found && function_scopes.size () > 1)
+	    {
+	      symbol_table::scope_id pscope =
+		function_scopes[function_scopes.size() - 2];
+	      symbol_table::install_nestfunction (nm, octave_value (fcn), pscope);
+	    }
+	  else
+	    {	      
+	      symbol_table::install_subfunction (nm, octave_value (fcn),
+						 primary_fcn_scope);
+	    }
         }
 
+      if (current_function_depth == 1 && fcn)
+	{
+	  symbol_table::update_nest (fcn->scope ());
+	}
+
       if (! reading_fcn_file)
         {
           // We are either reading a script file or defining a function
@@ -2979,6 +3004,7 @@
     parsing_subfunctions = true;
 
   current_function_depth--;
+  function_scopes.pop_back ();
 
   lexer_flags.defining_func--;
   lexer_flags.parsed_function_name.pop ();
@@ -3448,6 +3474,7 @@
   frame.protect_var (line_editing);
   frame.protect_var (current_class_name);
   frame.protect_var (current_function_depth);
+  frame.protect_var (function_scopes);
   frame.protect_var (max_function_depth);
   frame.protect_var (parsing_subfunctions);
   frame.protect_var (endfunction_found);
@@ -3458,6 +3485,7 @@
   line_editing = false;
   current_class_name = dispatch_type;
   current_function_depth = 0;
+  function_scopes.clear ();
   max_function_depth = 0;
   parsing_subfunctions = false;
   endfunction_found = false;
@@ -3576,11 +3604,6 @@
           if (status != 0)
             error ("parse error while reading %s file %s",
                    file_type.c_str(), ff.c_str ());
-          else if (reading_fcn_file && endfunction_found
-                   && max_function_depth > 1)
-            warning_with_id ("Octave:nested-functions-coerced",
-                             "nested functions are coerced into subfunctions "
-                             "in file %s", ff.c_str ());
         }
       else
         {
@@ -4292,6 +4315,7 @@
   frame.protect_var (line_editing);
   frame.protect_var (current_eval_string);
   frame.protect_var (current_function_depth);
+  frame.protect_var (function_scopes);
   frame.protect_var (max_function_depth);
   frame.protect_var (parsing_subfunctions);
   frame.protect_var (endfunction_found);
@@ -4307,6 +4331,7 @@
   line_editing = false;
   current_function_depth = 0;
   max_function_depth = 0;
+  function_scopes.clear ();
   parsing_subfunctions = false;
   endfunction_found = false;
   reading_fcn_file = false;
diff -r c1f9b54350f9 -r fbe54c9e8eb7 src/ov-usr-fcn.cc
--- a/src/ov-usr-fcn.cc	Fri Feb 24 20:42:29 2012 -0500
+++ b/src/ov-usr-fcn.cc	Thu Mar 08 19:35:37 2012 -0700
@@ -387,7 +387,7 @@
   // Save old and set current symbol table context, for
   // eval_undefined_error().
 
-  int context = is_anonymous_function () ? 0 : call_depth;
+  int context = active_context ();
 
   octave_call_stack::push (this, local_scope, context);
   frame.add_fcn (octave_call_stack::pop);
diff -r c1f9b54350f9 -r fbe54c9e8eb7 src/ov-usr-fcn.h
--- a/src/ov-usr-fcn.h	Fri Feb 24 20:42:29 2012 -0500
+++ b/src/ov-usr-fcn.h	Thu Mar 08 19:35:37 2012 -0700
@@ -177,6 +177,12 @@
 
   ~octave_user_function (void);
 
+  symbol_table::context_id active_context () const
+  {
+    return is_anonymous_function () ? 0
+      : static_cast<symbol_table::context_id>(call_depth);
+  }
+
   octave_function *function_value (bool = false) { return this; }
 
   octave_user_function *user_function_value (bool = false) { return this; }
diff -r c1f9b54350f9 -r fbe54c9e8eb7 src/symtab.cc
--- a/src/symtab.cc	Fri Feb 24 20:42:29 2012 -0500
+++ b/src/symtab.cc	Thu Mar 08 19:35:37 2012 -0700
@@ -625,40 +625,23 @@
       // subfunctions if we are currently executing a function defined
       // from a .m file.
 
-      scope_val_iterator r = subfunctions.find (xcurrent_scope);
-
       octave_user_function *curr_fcn = symbol_table::get_curr_fcn ();
 
-      if (r != subfunctions.end ())
+      for (scope_id scope = xcurrent_scope; scope >= 0;)
         {
-          // FIXME -- out-of-date check here.
+          scope_val_iterator r = subfunctions.find (scope);
+          if (r != subfunctions.end ())
+            {
+              // FIXME -- out-of-date check here.
 
-          return r->second;
-        }
-      else
-        {
-          if (curr_fcn)
-            {
-              // FIXME -- maybe it would be better if we could just get
-              // a pointer to the parent function so we would have
-              // access to all info about it instead of only being able
-              // to query the current function for specific bits of info
-              // about its parent function?
+              return r->second;
+            }
 
-              scope_id pscope = curr_fcn->parent_fcn_scope ();
-
-              if (pscope > 0)
-                {
-                  r = subfunctions.find (pscope);
-
-                  if (r != subfunctions.end ())
-                    {
-                      // FIXME -- out-of-date check here.
-
-                      return r->second;
-                    }
-                }
-            }
+          octave_user_function *temp = get_curr_fcn (scope);
+          if (temp)
+            scope = temp->parent_fcn_scope ();
+          else
+            scope = -1;
         }
 
       // Private function.
@@ -896,29 +879,21 @@
   // subfunctions if we are currently executing a function defined
   // from a .m file.
 
-  scope_val_iterator r = subfunctions.find (xcurrent_scope);
+  for (scope_id scope = xcurrent_scope; scope >= 0;)
+    {
+      scope_val_iterator r = subfunctions.find (scope);
+      if (r != subfunctions.end ())
+        {
+          // FIXME -- out-of-date check here.
 
-  if (r != subfunctions.end ())
-    {
-      // FIXME -- out-of-date check here.
+          return r->second;
+        }
 
-      return r->second;
-    }
-  else if (curr_fcn)
-    {
-      scope_id pscope = curr_fcn->parent_fcn_scope ();
-
-      if (pscope > 0)
-        {
-          r = subfunctions.find (pscope);
-
-          if (r != subfunctions.end ())
-            {
-              // FIXME -- out-of-date check here.
-
-              return r->second;
-            }
-        }
+      octave_user_function *temp = get_curr_fcn (scope);
+      if (temp)
+        scope = temp->parent_fcn_scope ();
+      else
+        scope = -1;
     }
 
   return octave_value ();
@@ -1144,6 +1119,24 @@
     }
 }
 
+void symbol_table::install_nestfunction (const std::string& name,
+                                         const octave_value& fcn,
+                                         scope_id parent_scope)
+{
+  // for now, just treat it as a subfunction of the parent_scope
+  install_subfunction (name, fcn, parent_scope);
+
+  // but we also need to share variables
+  octave_function *fv = fcn.function_value();
+  assert(fv);
+  symbol_table *fcn_table = get_instance (fv->scope());
+  symbol_table *parent_table = get_instance (parent_scope);
+  assert (fcn_table);
+  assert (parent_table);
+
+  fcn_table->stash_nest_parent(*parent_table);
+}
+
 octave_value
 symbol_table::find (const std::string& name,
                     const octave_value_list& args,
@@ -1390,6 +1383,51 @@
   return retval;
 }
 
+symbol_table*
+symbol_table::find_nonlocal (const std::string& name, symbol_record& record)
+{
+  for (symbol_table *tbl = nest_parent; tbl; tbl = tbl->nest_parent)
+    {
+      table_iterator iter = tbl->table.find (name);
+
+      assert (tbl->curr_fcn); // can only have nested functions for user functions
+      if (! (iter == tbl->table.end () || iter->second.is_nonlocal ()))
+        {
+          record = iter->second;
+          if (tbl->curr_fcn->active_context () != static_cast<context_id>(-1))
+            return tbl;
+
+          // returning handles to nested functions requires closures
+          ::error ("Closures are not yet supported.");
+          return 0;
+        }
+    }
+
+  ::error ("unable to resolve nonlocal!");
+  return 0;
+}
+
+octave_value&
+symbol_table::do_nonlocal_varref (const std::string& name)
+{
+  static octave_value foobar;
+  symbol_record record;
+  symbol_table* tbl = find_nonlocal (name, record);
+  if (tbl)
+    return record.varref (tbl->curr_fcn->active_context ());
+  return foobar;
+}
+
+octave_value
+symbol_table::do_nonlocal_varval (const std::string& name)
+{
+  symbol_record record;
+  symbol_table* tbl = find_nonlocal (name, record);
+  if (tbl)
+    return record.varval (tbl->curr_fcn->active_context ());
+  return octave_value ();
+}
+
 void
 symbol_table::do_dump (std::ostream& os)
 {
diff -r c1f9b54350f9 -r fbe54c9e8eb7 src/symtab.h
--- a/src/symtab.h	Fri Feb 24 20:42:29 2012 -0500
+++ b/src/symtab.h	Thu Mar 08 19:35:37 2012 -0700
@@ -199,6 +199,9 @@
     // temporary variables forced into symbol table for parsing
     static const unsigned int forced = 128;
 
+    // variables that are in a higher up nested scope, but not global
+    static const unsigned int nonlocal = 256;
+
   private:
 
     class
@@ -227,6 +230,8 @@
           return symbol_table::global_varref (name);
         else if (is_persistent ())
           return symbol_table::persistent_varref (name);
+	else if (is_nonlocal ())
+	  return symbol_table::nonlocal_varref (name);
         else
           {
             context_id n = value_stack.size ();
@@ -243,6 +248,8 @@
           return symbol_table::global_varval (name);
         else if (is_persistent ())
           return symbol_table::persistent_varval (name);
+	else if (is_nonlocal ())
+	  return symbol_table::nonlocal_varval (name);
         else
           {
             if (context < value_stack.size ())
@@ -254,7 +261,7 @@
 
       void push_context (void)
       {
-        if (! (is_persistent () || is_global ()))
+        if (! (is_persistent () || is_global () || is_nonlocal ()))
           value_stack.push_back (octave_value ());
       }
 
@@ -276,7 +283,7 @@
       {
         size_t retval = 1;
 
-        if (! (is_persistent () || is_global ()))
+        if (! (is_persistent () || is_global () || is_nonlocal ()))
           {
             value_stack.pop_back ();
             retval = value_stack.size ();
@@ -287,7 +294,7 @@
 
       void clear (void)
       {
-        if (! (is_hidden () || is_inherited ()))
+        if (! (is_hidden () || is_inherited () || is_nonlocal ()))
           {
             if (is_global ())
               unmark_global ();
@@ -322,6 +329,7 @@
       bool is_global (void) const { return storage_class & global; }
       bool is_persistent (void) const { return storage_class & persistent; }
       bool is_forced (void) const { return storage_class & forced; }
+      bool is_nonlocal (void) const { return storage_class & nonlocal; }
 
       void mark_local (void) { storage_class |= local; }
       void mark_automatic (void) { storage_class |= automatic; }
@@ -343,6 +351,11 @@
           storage_class |= persistent;
       }
       void mark_forced (void) { storage_class |= forced; }
+      void mark_nonlocal (void)
+      {
+	// FIXME -- check to ensure this is legal
+	storage_class |= nonlocal;
+      }
 
       void unmark_local (void) { storage_class &= ~local; }
       void unmark_automatic (void) { storage_class &= ~automatic; }
@@ -352,6 +365,7 @@
       void unmark_global (void) { storage_class &= ~global; }
       void unmark_persistent (void) { storage_class &= ~persistent; }
       void unmark_forced (void) { storage_class &= ~forced; }
+      void unmark_nonlocal (void) { storage_class &= ~nonlocal; }
 
       void init_persistent (void)
       {
@@ -375,7 +389,7 @@
       symbol_record_rep *dup (void) const
       {
         return new symbol_record_rep (name, varval (xcurrent_context),
-                                      storage_class);
+				      storage_class);
       }
 
       void dump (std::ostream& os, const std::string& prefix) const;
@@ -477,6 +491,7 @@
     bool is_inherited (void) const { return rep->is_inherited (); }
     bool is_persistent (void) const { return rep->is_persistent (); }
     bool is_forced (void) const { return rep->is_forced (); }
+    bool is_nonlocal (void) const { return rep->is_nonlocal (); }
 
     void mark_local (void) { rep->mark_local (); }
     void mark_automatic (void) { rep->mark_automatic (); }
@@ -486,6 +501,7 @@
     void mark_global (void) { rep->mark_global (); }
     void mark_persistent (void) { rep->mark_persistent (); }
     void mark_forced (void) { rep->mark_forced (); }
+    void mark_nonlocal (void) { rep->mark_nonlocal (); }
 
     void unmark_local (void) { rep->unmark_local (); }
     void unmark_automatic (void) { rep->unmark_automatic (); }
@@ -495,6 +511,7 @@
     void unmark_global (void) { rep->unmark_global (); }
     void unmark_persistent (void) { rep->unmark_persistent (); }
     void unmark_forced (void) { rep->unmark_forced (); }
+    void unmark_nonlocal (void) { rep->unmark_nonlocal (); }
 
     void init_persistent (void) { rep->init_persistent (); }
 
@@ -1154,6 +1171,22 @@
       inst->do_erase_persistent (name);
   }
 
+  static octave_value& nonlocal_varref (const std::string& name)
+  {
+    symbol_table *inst = get_instance (xcurrent_scope);
+
+    assert (inst);
+    return inst->do_nonlocal_varref (name);
+  }
+
+  static octave_value nonlocal_varval (const std::string& name)
+  {
+    symbol_table *inst = get_instance (xcurrent_scope);
+
+    assert (inst);
+    return inst->do_nonlocal_varval (name);
+  }
+
   static bool is_variable (const std::string& name)
   {
     symbol_table *inst = get_instance (xcurrent_scope);
@@ -1263,6 +1296,16 @@
       }
   }
 
+  static void install_nestfunction (const std::string& name, const octave_value& fcn,
+				    scope_id parent_scope);
+
+  static void update_nest (scope_id scope)
+  {
+    symbol_table *inst = get_instance (scope);
+    if (inst)
+      inst->do_update_nest ();
+  }
+
   static void install_user_function (const std::string& name,
                                      const octave_value& fcn)
   {
@@ -1961,6 +2004,12 @@
   // Map from symbol names to symbol info.
   std::map<std::string, symbol_record> table;
 
+  // If we are nested, this is our parent function
+  symbol_table *nest_parent;
+
+  // And these are all of our nested child functions
+  std::vector<symbol_table*> nest_children;
+
   // The associated user code (may be null).
   octave_user_function *curr_fcn;
 
@@ -2001,7 +2050,7 @@
   static context_id xcurrent_context;
 
   symbol_table (void)
-    : table_name (), table (), curr_fcn (0), persistent_table () { }
+    : table_name (), table (), nest_parent (0), curr_fcn (0), persistent_table () { }
 
   ~symbol_table (void) { }
 
@@ -2063,16 +2112,11 @@
     return retval;
   }
 
-  void insert_symbol_record (const symbol_record& sr)
-  {
-    table[sr.name ()] = sr;
-  }
-
   void
   do_dup_scope (symbol_table& new_symbol_table) const
   {
     for (table_const_iterator p = table.begin (); p != table.end (); p++)
-      new_symbol_table.insert_symbol_record (p->second.dup ());
+	new_symbol_table.table[p->first] = p->second.dup ();
   }
 
   symbol_record do_find_symbol (const std::string& name)
@@ -2188,6 +2232,12 @@
       persistent_table.erase (p);
   }
 
+  symbol_table* find_nonlocal (const std::string& name, symbol_record &record);
+
+  octave_value& do_nonlocal_varref (const std::string& name);
+
+  octave_value do_nonlocal_varval (const std::string& name);
+
   bool do_is_variable (const std::string& name) const
   {
     bool retval = false;
@@ -2462,6 +2512,41 @@
   void do_dump (std::ostream& os);
 
   void do_cache_name (const std::string& name) { table_name = name; }
+
+  void stash_nest_parent (symbol_table& parent)
+  {
+    assert (!nest_parent);
+    nest_parent = &parent;
+    parent.nest_children.push_back (this);
+  }
+
+  void do_update_nest (const std::set<std::string>& check = std::set<std::string> ())
+  {
+    if (check.size ())
+      {
+      for (table_iterator iter = table.begin (); iter != table.end (); ++iter)
+	{
+	  if (check.count (iter->first))
+	    iter->second.mark_nonlocal ();
+	  else
+	    iter->second.unmark_nonlocal ();
+	}
+      }
+
+    if (nest_children.size ())
+      {
+	std::set<std::string> next = check;
+	for (table_iterator iter = table.begin (); iter != table.end (); ++iter)
+	  {
+	    if (iter->second.is_local ())
+	      next.insert (iter->first);
+	  }
+
+	for (std::vector<symbol_table*>::iterator iter = nest_children.begin ();
+	     iter != nest_children.end (); ++iter)
+	  (*iter)->do_update_nest (next);
+      }
+  }
 };
 
 extern bool out_of_date_check (octave_value& function,
diff -r c1f9b54350f9 -r fbe54c9e8eb7 test/nest/module.mk
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/nest/module.mk	Thu Mar 08 19:35:37 2012 -0700
@@ -0,0 +1,8 @@
+nest_FCN_FILES = \
+  nest/recursive_nest.m \
+  nest/scope0.m \
+  nest/scope1.m \
+  nest/scope2.m \
+  nest/test_nest.m
+
+FCN_FILES += $(nest_FCN_FILES)
diff -r c1f9b54350f9 -r fbe54c9e8eb7 test/nest/recursive_nest.m
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/nest/recursive_nest.m	Thu Mar 08 19:35:37 2012 -0700
@@ -0,0 +1,12 @@
+# recursive_nest.m
+function x = recursive_nest()
+  X = 5;
+  B(20)
+  x = X;
+  function B(n)
+    if n > 0
+      X = X + 1;
+      B(n - 1);
+    end
+  endfunction
+endfunction
\ No newline at end of file
diff -r c1f9b54350f9 -r fbe54c9e8eb7 test/nest/recursive_nest2.m
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/nest/recursive_nest2.m	Thu Mar 08 19:35:37 2012 -0700
@@ -0,0 +1,21 @@
+# recursive_nest2.m
+function x = recursive_nest2()
+  x = B(20)
+  function v = B(n)
+    Y = 0;
+    BB(n);
+    C;
+    v = Y;
+    function BB(m)
+      if m > 0
+	Y = Y + 1;
+	BB(m - 1);
+	C;
+      end
+    endfunction
+  endfunction
+
+  function C
+    Y = 0;
+  endfunction
+endfunction
diff -r c1f9b54350f9 -r fbe54c9e8eb7 test/nest/scope0.m
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/nest/scope0.m	Thu Mar 08 19:35:37 2012 -0700
@@ -0,0 +1,16 @@
+# scope0.m
+function scope0
+  C
+  function A
+    B
+    function B
+    endfunction
+  endfunction
+
+  function C
+    D
+    function D
+      A
+    endfunction
+  endfunction
+endfunction
\ No newline at end of file
diff -r c1f9b54350f9 -r fbe54c9e8eb7 test/nest/scope1.m
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/nest/scope1.m	Thu Mar 08 19:35:37 2012 -0700
@@ -0,0 +1,20 @@
+# scope1.m
+function scope1(n)
+  value = n;
+  if value
+    C
+  end
+  function A
+    B
+    function B
+      scope1(0)
+    endfunction
+  endfunction
+
+  function C
+    D
+    function D
+      A
+    endfunction
+  endfunction
+endfunction
\ No newline at end of file
diff -r c1f9b54350f9 -r fbe54c9e8eb7 test/nest/scope2.m
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/nest/scope2.m	Thu Mar 08 19:35:37 2012 -0700
@@ -0,0 +1,17 @@
+# scope2.m
+function scope2
+  C
+  function A
+    B
+    function B
+      D
+    endfunction
+  endfunction
+
+  function C
+    D
+    function D
+      A
+    endfunction
+  endfunction
+endfunction
diff -r c1f9b54350f9 -r fbe54c9e8eb7 test/nest/scope3.m
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/nest/scope3.m	Thu Mar 08 19:35:37 2012 -0700
@@ -0,0 +1,19 @@
+# scope3.m
+function scope3
+  C
+  function A
+    B
+    function B
+      E
+    endfunction
+    function E
+    endfunction
+  endfunction
+
+  function C
+    D
+    function D
+      A
+    endfunction
+  endfunction
+endfunction
\ No newline at end of file
diff -r c1f9b54350f9 -r fbe54c9e8eb7 test/nest/test_nest.m
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/nest/test_nest.m	Thu Mar 08 19:35:37 2012 -0700
@@ -0,0 +1,37 @@
+## Copyright (C) 2006-2012 John W. Eaton
+##
+## This file is part of Octave.
+##
+## Octave is free software; you can redistribute it and/or modify it
+## under the terms of the GNU General Public License as published by
+## the Free Software Foundation; either version 3 of the License, or (at
+## your option) any later version.
+##
+## Octave is distributed in the hope that it will be useful, but
+## WITHOUT ANY WARRANTY; without even the implied warranty of
+## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+## General Public License for more details.
+##
+## You should have received a copy of the GNU General Public License
+## along with Octave; see the file COPYING.  If not, see
+## <http://www.gnu.org/licenses/>.
+
+%!test
+%! a = recursive_nest;
+%! assert (a, 25);
+
+%!test
+%! a = recursive_nest2;
+%! assert (a, 20);
+
+%!test
+%! scope0;
+
+%!test
+%! scope1(1);
+
+%!test
+% fail ("scope2");
+
+%!test
+%! scope3;