Re: Nested functions patch

[John W. Eaton]

On  9-Mar-2012, Max Brister wrote:

| Thank you for taking a look at my patch. Sorry about the lack of
| conformance to style. I will fix this.
| 
| | For
| |
| |  function f ()
| |    x = 1
| |    g (x)
| |    x
| |    function g (x)
| |      x = 2
| |    end
| |  end
| |  f ()
| |
| | I see the following:
| |
| |  x =  1
| |  x =  2
| |  error: `x' undefined near line 4 column 3
| |  error: called from:
| |  error:   f at line 4, column 3
| |
| | I think X should still be defined to be 1 after the call to G.
| 
| You are correct. I forgot to take into account the fact that
| parameters are never nonlocal (the segfault is disturbing). I will
| look into how parameters are handled, and hopefully this will not be
| too difficult to fix. Additionally, I will try to add some more
| aggressive test cases.
| 
| Furthermore, adding new variables in a parent function may cause
| issues. For example,
| function f ()
|   eval ('x = 1');
|       g ()
|   function g ()
|     x
|   endfunction
| endfunction
| 
| I am not sure how this should be handled. Matlab handles this by
| simply erroring on eval('x = 1'). I could just mark all children of g
| as having nonlocal x instead. Also, a similar problem comes up if a
| variable in a parent function is cleared.

I think it is reasonable to issue an error if a new symbol is created
when there are child functions.  A related question is what should
happen if we have

  eval ("global x")

or

  eval ("persistent x")

?  It seems to me that those could also cause trouble.

| Yes, I did mean that nest_parent is the symbol table for the parent
| function. Unfortunatly, a simple collaplsing of the symbol table will
| not work. For example,
| 
| function f ()
|   g ()
|   function g ()
|     y = 1;
|     e ();
|     y
|   endfunction
|   function e ()
|     y = 2;
|   endfunction
| endfunction
| 
| should result should be y = 1 after the call to e. However, if y was
| used in f the result would be y = 2. Resolving these differences gets
| more complex as the nesting depth and breath increase. I think the
| simplest approach in this case is to just continue the practice of
| using one symbol table per scope. Otherwise, I think our scope class
| would end up looking almost exactly like the symbol table class.
| 
| I think a better way to resolve nonlocal lookups. symbol_record_ref
| could be modified to add a symbol_record local_ref field. For nonlocal
| variables this would be a reference to the symbol_table record for
| which they are local.

Instead of handling this in the symbol table lookup code, how about
simply ensuring that the symbols are all resolved to the proper scope
at the time of parsing the functions?  I'm thinking that after parsing
a function that includes a set of nested functions, you would walk the
parse tree of each child function, look for tree_identifier objects
and fix up the references to the symbol table so that they are in the
proper scope.  Then I think you are done, and no significant changes
need to be made to the symbol table class itself.   Symbol record
objects are already tagged if they are formal parameters, so you
shouldn't need to specifically tag "nonlocal" objects.  Would that
work?

| | What is the intent of the update_nest function?
| 
| The intent of the update_nest function is to determine which variables
| are nonlocal in a given symbol_table. update_nest gets executed once
| on the parent function after parsing is complete. It then recursively
| calls update_nest sending each child a set of local variables. Then
| each child marks the local variables of its parent as nonlocal.

OK.  So instead of marking variables as nonlocal, I think this
function should be fixing the scope of variables.  I'm not sure
whether you can do this only by looking at the symbol tables because
tree_identfier objects hold a reference to symbol_record objects, so I
think you'll need to update those.  Hmm, looking at the
tree_identifier class, it seems that the current definition of
tree_identifier::xsym will cause trouble because when executing a
child function, the current scope (at eval time) would be different
from the scope stored at parse time.  So then the way the xsym
currently works, the variable would be put back into the child scope,
which is the one current at the time the child function is evaluated.

So I'm not sure what the right fix is.  But it seems to me that we
shouldn't have to search through multiple symbol tables each time a
function is evaluated.  We should be able to make it so that the
scope of a variable is resolved when nested functions are parsed.

jwe