Mercurial > octave
changeset 16070:e603ce23f20c
Fix and improve functions related to class precedence (bug #38290)
* libinterp/octave-value/ov-class.cc(inferiorto): Use only one call to
set_class_relationship() and properly use the returned value. Rewrite
for clarity (fewer braces, shorter lines).
(superiorto): Rewrite similarly to inferiorto() for clarity.
* libinterp/interpfcn/symtab.cc(symbol_table::set_class_relationship):
Reduce code redundancy using a call to is_superiorto(). Add a comment
to make it clear that a new entry in the precedence table is created
if sup_class is not already there.
(symbol_table::is_superiorto): Rewrite more concisely. Add a comment.
* test/classes/@CPrecedenceTester1, test/classes/@CPrecedenceTester2,
test/classes/@CPrecedenceTester3: New classes for precedence tests.
* test/classes/@Snork/tattack.m: New method for precedence tests.
* test/classes/classes.tst: Add precedence tests.
author | Julien Bect <julien.bect@supelec.fr> |
---|---|
date | Tue, 29 Jan 2013 17:55:53 +0100 |
parents | 999f8257313b |
children | 94e95309710c |
files | libinterp/interpfcn/symtab.cc libinterp/octave-value/ov-class.cc test/classes/@CPrecedenceTester1/CPrecedenceTester1.m test/classes/@CPrecedenceTester1/tattack.m test/classes/@CPrecedenceTester2/CPrecedenceTester2.m test/classes/@CPrecedenceTester2/tattack.m test/classes/@CPrecedenceTester3/CPrecedenceTester3.m test/classes/@CPrecedenceTester3/tattack.m test/classes/@Snork/tattack.m test/classes/classes.tst |
diffstat | 10 files changed, 147 insertions(+), 94 deletions(-) [+] |
line wrap: on
line diff
--- a/libinterp/interpfcn/symtab.cc Sat Feb 09 16:13:55 2013 -0500 +++ b/libinterp/interpfcn/symtab.cc Tue Jan 29 17:55:53 2013 +0100 @@ -1013,20 +1013,12 @@ symbol_table::set_class_relationship (const std::string& sup_class, const std::string& inf_class) { - class_precedence_table_const_iterator p - = class_precedence_table.find (inf_class); - - if (p != class_precedence_table.end ()) - { - const std::set<std::string>& inferior_classes = p->second; + if (is_superiorto (inf_class, sup_class)) + return false; - std::set<std::string>::const_iterator q - = inferior_classes.find (sup_class); - - if (q != inferior_classes.end ()) - return false; - } - + // If sup_class doesn't have an entry in the precedence table, + // this will automatically create it, and associate to it a + // singleton set {inf_class} of inferior classes. class_precedence_table[sup_class].insert (inf_class); return true; @@ -1034,7 +1026,7 @@ // Has class A been marked as superior to class B? Also returns // TRUE if B has been marked as inferior to A, since we only keep -// one table, and convert inferiort information to a superiorto +// one table, and convert inferiorto information to a superiorto // relationship. Two calls are required to determine whether there // is no relationship between two classes: // @@ -1048,20 +1040,14 @@ bool symbol_table::is_superiorto (const std::string& a, const std::string& b) { - bool retval = false; - - class_precedence_table_const_iterator p = class_precedence_table.find (a); + class_precedence_table_const_iterator p = class_precedence_table.find (a); + // If a has no entry in the precedence table, return false + if (p == class_precedence_table.end ()) + return false; - if (p != class_precedence_table.end ()) - { - const std::set<std::string>& inferior_classes = p->second; - std::set<std::string>::const_iterator q = inferior_classes.find (b); - - if (q != inferior_classes.end ()) - retval = true; - } - - return retval; + const std::set<std::string>& inferior_classes = p->second; + std::set<std::string>::const_iterator q = inferior_classes.find (b); + return (q != inferior_classes.end ()); } static std::string
--- a/libinterp/octave-value/ov-class.cc Sat Feb 09 16:13:55 2013 -0500 +++ b/libinterp/octave-value/ov-class.cc Tue Jan 29 17:55:53 2013 +0100 @@ -2191,43 +2191,35 @@ octave_value retval; octave_function *fcn = octave_call_stack::caller (); - - if (fcn && fcn->is_class_constructor ()) + if ((! fcn) || (! fcn->is_class_constructor ())) { - for (int i = 0; i < args.length (); i++) + error ("superiorto: invalid call from outside class constructor"); + return retval; + } + + for (int i = 0; i < args.length (); i++) + { + std::string inf_class = args(i).string_value (); + if (error_state) { - std::string class_name = args(i).string_value (); - - if (! error_state) - { - if (! is_built_in_class (class_name)) - { - std::string this_class_name = fcn->name (); - - if (! symbol_table::set_class_relationship (this_class_name, - class_name)) - { - error ("superiorto: precedence already set for %s and %s", - this_class_name.c_str (), class_name.c_str ()); - break; - } - } - else - { - // User defined classes always have higher precedence - // than built-in classes. - } - } - else - { error ("superiorto: expecting argument to be class name"); break; - } } - } - else - error ("superiorto: invalid call from outside class constructor"); + + // User defined classes always have higher precedence + // than built-in classes + if (is_built_in_class (inf_class)) + break; + std::string sup_class = fcn->name (); + if (! symbol_table::set_class_relationship (sup_class, inf_class)) + { + error ("superiorto: opposite precedence already set for %s and %s", + sup_class.c_str (), inf_class.c_str ()); + break; + } + } + return retval; } @@ -2241,47 +2233,38 @@ @end deftypefn") { octave_value retval; - + octave_function *fcn = octave_call_stack::caller (); - - if (fcn && fcn->is_class_constructor ()) + if ((! fcn) || (! fcn->is_class_constructor ())) { - for (int i = 0; i < args.length (); i++) + error ("inferiorto: invalid call from outside class constructor"); + return retval; + } + + for (int i = 0; i < args.length (); i++) + { + std::string sup_class = args(i).string_value (); + if (error_state) { - std::string class_name = args(i).string_value (); - - if (! error_state) - { - if (! is_built_in_class (class_name)) - { - std::string this_class_name = fcn->name (); - - symbol_table::set_class_relationship (class_name, - this_class_name); - - if (! symbol_table::set_class_relationship (this_class_name, - class_name)) - { - error ("inferiorto: precedence already set for %s and %s", - this_class_name.c_str (), class_name.c_str ()); - break; - } - } - else - { - error ("inferiorto: cannot give user-defined class lower precedence than built-in class"); - break; - } - } - else - { - error ("inferiorto: expecting argument to be class name"); - break; - } + error ("inferiorto: expecting argument to be class name"); + break; + } + + if (is_built_in_class (sup_class)) + { + error ("inferiorto: cannot give user-defined class lower " + "precedence than built-in class"); + break; + } + + std::string inf_class = fcn->name (); + if (! symbol_table::set_class_relationship (sup_class, inf_class)) + { + error ("inferiorto: opposite precedence already set for %s and %s", + inf_class.c_str (), sup_class.c_str ()); + break; } } - else - error ("inferiorto: invalid call from outside class constructor"); return retval; }
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/classes/@CPrecedenceTester1/CPrecedenceTester1.m Tue Jan 29 17:55:53 2013 +0100 @@ -0,0 +1,8 @@ +function x = CPrecedenceTester1() + + x = struct('useless_data', pi); + x = class(x, 'CPrecedenceTester1'); + + % don't change anything as far as precedence is concerned + +end
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/classes/@CPrecedenceTester1/tattack.m Tue Jan 29 17:55:53 2013 +0100 @@ -0,0 +1,5 @@ +function s = tattack(x, y) + + s = 'CPrecedenceTester1'; + +end
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/classes/@CPrecedenceTester2/CPrecedenceTester2.m Tue Jan 29 17:55:53 2013 +0100 @@ -0,0 +1,15 @@ +function x = CPrecedenceTester2(flag) + + x = struct('useless_data', pi^2); + x = class(x, 'CPrecedenceTester2'); + + switch flag, + case 1, % CPrecedencetester2 > Snork + superiorto('Snork'); + case 2, % CPrecedencetester2 < Snork + inferiorto('Snork'); + otherwise, + error('Incorrect value for argument flag: %d', flag); + end + +end
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/classes/@CPrecedenceTester2/tattack.m Tue Jan 29 17:55:53 2013 +0100 @@ -0,0 +1,5 @@ +function s = tattack(x, y) + + s = 'CPrecedenceTester2'; + +end
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/classes/@CPrecedenceTester3/CPrecedenceTester3.m Tue Jan 29 17:55:53 2013 +0100 @@ -0,0 +1,15 @@ +function x = CPrecedenceTester3(flag) + + x = struct('useless_data', pi^3); + x = class(x, 'CPrecedenceTester3'); + + switch flag, + case 1, % CPrecedencetester3 > Snork + superiorto('Snork'); + case 2, % CPrecedencetester3 < Snork + inferiorto('Snork'); + otherwise, + error('Incorrect value for argument flag: %d', flag); + end + +end
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/classes/@CPrecedenceTester3/tattack.m Tue Jan 29 17:55:53 2013 +0100 @@ -0,0 +1,5 @@ +function s = tattack(x, y) + + s = 'CPrecedenceTester3'; + +end
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/classes/@Snork/tattack.m Tue Jan 29 17:55:53 2013 +0100 @@ -0,0 +1,5 @@ +function s = tattack(x, y) + + s = 'Snork'; + +end
--- a/test/classes/classes.tst Sat Feb 09 16:13:55 2013 -0500 +++ b/test/classes/classes.tst Tue Jan 29 17:55:53 2013 +0100 @@ -316,3 +316,29 @@ %!xtest s = [s1 x2]; assert (isa (s, 'Snork') && isequal (s.gick, [x1 x2])); %!xtest s = [x1 s2]; assert (isa (s, 'Snork') && isequal (s.gick, [x1 x2])); +%%%%%%%%%%%%%%%%%%%%%%%% +%% Testing precedence %% +%%%%%%%%%%%%%%%%%%%%%%%% + +%% default: leftmost object wins +%!shared A, B +%!test A = Snork(rand(2)); +%!test B = CPrecedenceTester1(); % no call to inferiorto/superiorto +%!assert (isequal (tattack (A, B), 'Snork')) +%!assert (isequal (tattack (B, A), 'CPrecedenceTester1')) % idem + +%!shared A, B +%!test A = Snork(rand(2)); +%!test B = CPrecedenceTester2(1); % CPrecedenceTester2 > Snork +%!assert (isequal (tattack (A, B), 'CPrecedenceTester2')) +%!assert (isequal (tattack (B, A), 'CPrecedenceTester2')) +%% Trying to change to CPrecendenceTester < Snork +%!error D = CPrecedenceTester2(2); + +%!shared A, B +%!test A = Snork(rand(2)); +%!test B = CPrecedenceTester3(2); % CPrecedenceTester3 < Snork +%!assert (isequal (tattack (A, B), 'Snork')) +%!assert (isequal (tattack (B, A), 'Snork')) +%% Trying to change to CPrecendenceTester3 > Snork +%!error D = CPrecedenceTester3(1);