# HG changeset patch
# User Julien Bect <julien.bect@supelec.fr>
# Date 1359478553 -3600
# Node ID 3d5076f64c24c1a3dc82803e20834d268b02e487
# Parent  a78c1e445cfc2b290d8ca8c84ea1e179f48400a8
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/test_classes.m: Add precedence tests.

diff -r a78c1e445cfc -r 3d5076f64c24 libinterp/interpfcn/symtab.cc
--- a/libinterp/interpfcn/symtab.cc	sam. févr. 09 07:14:39 2013 -0800
+++ b/libinterp/interpfcn/symtab.cc	mar. janv. 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 (is_superiorto (inf_class, sup_class))
+    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 (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);  
+  // If a has no entry in the precedence table, return false
+  if (p == class_precedence_table.end ())
+    return false;
 
-  class_precedence_table_const_iterator p = class_precedence_table.find (a);
-
-  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
diff -r a78c1e445cfc -r 3d5076f64c24 libinterp/octave-value/ov-class.cc
--- a/libinterp/octave-value/ov-class.cc	sam. févr. 09 07:14:39 2013 -0800
+++ b/libinterp/octave-value/ov-class.cc	mar. janv. 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 a78c1e445cfc -r 3d5076f64c24 test/classes/@CPrecedenceTester1/CPrecedenceTester1.m
--- /dev/null	jeu. janv. 01 00:00:00 1970 +0000
+++ b/test/classes/@CPrecedenceTester1/CPrecedenceTester1.m	mar. janv. 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 a78c1e445cfc -r 3d5076f64c24 test/classes/@CPrecedenceTester1/tattack.m
--- /dev/null	jeu. janv. 01 00:00:00 1970 +0000
+++ b/test/classes/@CPrecedenceTester1/tattack.m	mar. janv. 29 17:55:53 2013 +0100
@@ -0,0 +1,5 @@
+function s = tattack(x, y)
+  
+  s = 'CPrecedenceTester1';
+  
+end
diff -r a78c1e445cfc -r 3d5076f64c24 test/classes/@CPrecedenceTester2/CPrecedenceTester2.m
--- /dev/null	jeu. janv. 01 00:00:00 1970 +0000
+++ b/test/classes/@CPrecedenceTester2/CPrecedenceTester2.m	mar. janv. 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 a78c1e445cfc -r 3d5076f64c24 test/classes/@CPrecedenceTester2/tattack.m
--- /dev/null	jeu. janv. 01 00:00:00 1970 +0000
+++ b/test/classes/@CPrecedenceTester2/tattack.m	mar. janv. 29 17:55:53 2013 +0100
@@ -0,0 +1,5 @@
+function s = tattack(x, y)
+  
+  s = 'CPrecedenceTester2';
+  
+end
diff -r a78c1e445cfc -r 3d5076f64c24 test/classes/@CPrecedenceTester3/CPrecedenceTester3.m
--- /dev/null	jeu. janv. 01 00:00:00 1970 +0000
+++ b/test/classes/@CPrecedenceTester3/CPrecedenceTester3.m	mar. janv. 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 a78c1e445cfc -r 3d5076f64c24 test/classes/@CPrecedenceTester3/tattack.m
--- /dev/null	jeu. janv. 01 00:00:00 1970 +0000
+++ b/test/classes/@CPrecedenceTester3/tattack.m	mar. janv. 29 17:55:53 2013 +0100
@@ -0,0 +1,5 @@
+function s = tattack(x, y)
+  
+  s = 'CPrecedenceTester3';
+  
+end
diff -r a78c1e445cfc -r 3d5076f64c24 test/classes/@Snork/tattack.m
--- /dev/null	jeu. janv. 01 00:00:00 1970 +0000
+++ b/test/classes/@Snork/tattack.m	mar. janv. 29 17:55:53 2013 +0100
@@ -0,0 +1,5 @@
+function s = tattack(x, y)
+  
+  s = 'Snork';
+  
+end
diff -r a78c1e445cfc -r 3d5076f64c24 test/classes/test_classes.m
--- a/test/classes/test_classes.m	sam. févr. 09 07:14:39 2013 -0800
+++ b/test/classes/test_classes.m	mar. janv. 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);
