Mercurial > octave
changeset 24977:2b040946dc69 stable
improve handling of clear for globals, forwarded symbols (bug #53027)
* symrec.h (symbol_record::symbol_record_rep::clear): Don't return
early after unbinding global. Don't check for forwarded symbol here.
* symrec.h (symbol_record::is_forwarded,
symbol_record::symbol_record_rep::is_forwarded): New functions.
* symscope.h (symbol_scope_rep::refresh): Don't delete forwarded symbols.
* test/bug-53027/bug-53027.tst, test/bug-53027/module.mk,
test/bug-53027/ntest53027a.m, test/bug-53027/ntest53027b.m:
New test files.
* test/module.mk: Include test/bug-53027/module.mk.
author | John W. Eaton <jwe@octave.org> |
---|---|
date | Thu, 22 Mar 2018 17:34:41 -0400 |
parents | 241b80a7d8c7 |
children | 19cf547b1453 |
files | libinterp/corefcn/symrec.h libinterp/corefcn/symscope.h test/bug-53027/bug-53027.tst test/bug-53027/module.mk test/bug-53027/ntest53027a.m test/bug-53027/ntest53027b.m test/module.mk |
diffstat | 7 files changed, 86 insertions(+), 19 deletions(-) [+] |
line wrap: on
line diff
--- a/libinterp/corefcn/symrec.h Wed Mar 21 21:53:40 2018 -0500 +++ b/libinterp/corefcn/symrec.h Thu Mar 22 17:34:41 2018 -0400 @@ -199,31 +199,19 @@ void clear (context_id context) { - // There is no need to do anything with a fowarded - // symbol_record_rep here. - // - // For scripts, we are never executing in the script "scope". - // - // For globals, we are only interested in breaking the link to - // the global value and clearing the local value, not the - // global one. - - // For persistent values, we clear the value then unmark so - // that we clear the first element of the value stack. + // For globals, break the link to the global value first, then + // clear the local value. if (is_global ()) - { - unbind_fwd_rep (); - return; - } - - if (auto t_fwd_rep = m_fwd_rep.lock ()) - return; + unbind_fwd_rep (); if (! (is_hidden () || is_inherited ())) { assign (octave_value (), context); + // For persistent values, we clear the value then unmark so + // that we clear the first element of the value stack. + if (is_persistent ()) unmark_persistent (); } @@ -282,6 +270,11 @@ return m_storage_class & inherited; } + bool is_forwarded (void) const + { + return ! m_fwd_rep.expired (); + } + bool is_global (void) const { if (auto t_fwd_rep = m_fwd_rep.lock ()) @@ -499,6 +492,12 @@ void unbind_fwd_rep (void) { + if (auto t_fwd_rep = m_fwd_rep.lock ()) + { + t_fwd_rep->unbind_fwd_rep (); + return; + } + // When unbinding an object, only break the immediate link. // By doing that, we ensure that any variables that are made // global in a script remain linked as globals in the @@ -629,6 +628,7 @@ bool is_global (void) const { return m_rep->is_global (); } bool is_hidden (void) const { return m_rep->is_hidden (); } bool is_inherited (void) const { return m_rep->is_inherited (); } + bool is_forwarded (void) const { return m_rep->is_forwarded (); } bool is_persistent (void) const { return m_rep->is_persistent (); } bool is_added_static (void) const { return m_rep->is_added_static (); }
--- a/libinterp/corefcn/symscope.h Wed Mar 21 21:53:40 2018 -0500 +++ b/libinterp/corefcn/symscope.h Thu Mar 22 17:34:41 2018 -0400 @@ -277,7 +277,7 @@ { symbol_record& sr = nm_sr.second; - if (! sr.is_persistent ()) + if (! (sr.is_persistent () || sr.is_forwarded ())) sr.clear (m_context); } }
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/bug-53027/bug-53027.tst Thu Mar 22 17:34:41 2018 -0400 @@ -0,0 +1,34 @@ +%!function load53027 (fname) +%! global X +%! X = 2; +%! load (fname); +%! assert (X, 1); +%!endfunction + +%!function save53027 (fname) +%! global X +%! X = 1; +%! save (fname, "X"); +%!endfunction + +%!test <*53027> +%! global X +%! X = 0; +%! fname = tmpnam (); +%! save53027 (fname); +%! assert (X, 1); +%! load53027 (fname); +%! assert (X, 1); +%! load53027 (fname); +%! assert (X, 1); +%! clear X +%! assert (exist ("X"), 0); + +%!test <*53027> +%! [a, b] = ntest53027a (); +%! assert ([a, b], [0, 0]) + +## Not fixed yet. +%!test <53027> +%! [a, b] = ntest53027b (); +%! assert ([a, b], [0, 0])
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/bug-53027/module.mk Thu Mar 22 17:34:41 2018 -0400 @@ -0,0 +1,6 @@ +bug_53027_TEST_FILES = \ + %reldir%/bug-53027.tst \ + %reldir%/ntest53027a.m \ + %reldir%/ntest53027b.m + +TEST_FILES += $(bug_53027_TEST_FILES)
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/bug-53027/ntest53027a.m Thu Mar 22 17:34:41 2018 -0400 @@ -0,0 +1,11 @@ +function [m_exist, n_exist] = ntest53027a () + global x + x = 3; + n_exist = nest (); + m_exist = exist ("x", 'var'); + function n_exist = nest () + x = 1; + clear x + n_exist = exist ("x", "var"); + endfunction +endfunction
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/bug-53027/ntest53027b.m Thu Mar 22 17:34:41 2018 -0400 @@ -0,0 +1,15 @@ +function [m_exist, n_exist] = ntest53027b () + global x + x = 3; + n_exist = nest (); + m_exist = exist ("x", 'var'); + function n_exist = nest () + ## The clear statement should operate on the variable in the + ## parent scope even though there is no explicit varabiable + ## reference in teh code (the clear function just sees a string + ## containing the name of the variable and the parser (correctly) + ## does not treat this as a special case. + clear x + n_exist = exist ("x", "var"); + endfunction +endfunction
--- a/test/module.mk Wed Mar 21 21:53:40 2018 -0500 +++ b/test/module.mk Thu Mar 22 17:34:41 2018 -0400 @@ -65,6 +65,7 @@ include %reldir%/bug-51599/module.mk include %reldir%/bug-52075/module.mk include %reldir%/bug-52722/module.mk +include %reldir%/bug-53027/module.mk include %reldir%/class-concat/module.mk include %reldir%/classdef/module.mk include %reldir%/classdef-multiple-inheritance/module.mk