Mercurial > octave-nkf

changeset 16070:e603ce23f20c

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
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);