Re: Nested functions patch

[Max Brister]

I have submitted a revised patch
(https://savannah.gnu.org/bugs/?35772). Unfortunately, I still had to
modify the symbol_table heavily. I think this was unavoidable as it is
important that symbol_table::insert return the correct symbol_record.

If symbol_table::insert does not return the correct symbol, then
executing scripts from nested scopes becomes challenging. This would
require the introduction of some new function, call it
symbol_table::real_insert. symbol_table::real_insert would have to do
everything symbol_table::insert does now, but handle nonlocals
correctly. Furthermore, any time a variable is accessed, the
symbol_table::real_insert function would have to be called in order to
resolve the correct variable.

| 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.

In order to ensure symbol_table::insert returns the correct symbol
symbol_table::update_nest updates child symbol tables to refer to the
correct symbol_record. This is different then in my last patch, which
simply marked them as nonlocal. This prevents the need for searching
through multiple symbol tables each time a function is evaluated.

Which adds a problem that symbol_record::pop and symbol_record::push
will incorrectly pop/push nonlocal variables. I fixed this by letting
symbol_records keep track of their declaration scope, and only
executing a pop and push if it was requested from their declaration
scope.

Another issue is that when evaluating nested functions,
xcurrent_context may not be the current context of a nonlocal
variable. I solved this problem by storing the current function of the
declaration scope in symbol_records. symbol_records can then look up
the current value using the active_context of their current function.
As a side effect, xcurrent_context can no longer be passed to
symbol_records by default. This would cause the symbol_record to use
xcurrent_context, instead of the active_context of their current
function. I added an xdefault_context to indicate that symbol_records
should use their active context.

I do not think adding xdefault_context is a clean solution. I think
the usage of xcurernt_context is the underlying problem, but I am not
comfortable reworking that code right now. Maybe this can be left
until closures are implemented, as contexts will have to be reworked
then anyways.

| 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?

Instead of doing an extra walk after parsing, I added
symbol_table::is_valid which is checked at parse time. I think this is
cleaner, but if you want to prevent the extra check each time
tree_identifier::xsym is called I could replace this with a tree walk.