# HG changeset patch # User Julien Bect # Date 1359478553 -3600 # Node ID e603ce23f20c3a68b5f7ca44bd5be1e327633bcc # Parent 999f8257313b51bdfe1ca17a758b9f3f291c7de1 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. diff -r 999f8257313b -r e603ce23f20c libinterp/interpfcn/symtab.cc --- 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& inferior_classes = p->second; + if (is_superiorto (inf_class, sup_class)) + return false; - std::set::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& inferior_classes = p->second; - std::set::const_iterator q = inferior_classes.find (b); - - if (q != inferior_classes.end ()) - retval = true; - } - - return retval; + const std::set& inferior_classes = p->second; + std::set::const_iterator q = inferior_classes.find (b); + return (q != inferior_classes.end ()); } static std::string diff -r 999f8257313b -r e603ce23f20c libinterp/octave-value/ov-class.cc --- 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; } diff -r 999f8257313b -r e603ce23f20c test/classes/@CPrecedenceTester1/CPrecedenceTester1.m --- /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 diff -r 999f8257313b -r e603ce23f20c test/classes/@CPrecedenceTester1/tattack.m --- /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 diff -r 999f8257313b -r e603ce23f20c test/classes/@CPrecedenceTester2/CPrecedenceTester2.m --- /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 diff -r 999f8257313b -r e603ce23f20c test/classes/@CPrecedenceTester2/tattack.m --- /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 diff -r 999f8257313b -r e603ce23f20c test/classes/@CPrecedenceTester3/CPrecedenceTester3.m --- /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 diff -r 999f8257313b -r e603ce23f20c test/classes/@CPrecedenceTester3/tattack.m --- /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 diff -r 999f8257313b -r e603ce23f20c test/classes/@Snork/tattack.m --- /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 diff -r 999f8257313b -r e603ce23f20c test/classes/classes.tst --- 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);