changeset 28519:d4563c5d4060

handle all dispatching for colon operator in do_colon_op function * ov.cc (do_colon_op): Handle all dispatching here, but using lookup based on argument vector passed to feval instead of determining dispatch type separately. * pt-colon.cc (tree_colon_expression::evaluate): Simply evaluate arguments and pass them to do_colon_op. * test/colon-op: New test directory. * test/module.mk: Update.
author John W. Eaton <jwe@octave.org>
date Mon, 29 Jun 2020 23:45:19 -0400
parents b8ab8b58547d
children 23a33db2bdb3
files libinterp/octave-value/ov.cc libinterp/parse-tree/pt-colon.cc test/colon-op/@legacy_colon_op/colon.m test/colon-op/@legacy_colon_op/legacy_colon_op.m test/colon-op/colon-op.tst test/colon-op/colon_op.m test/colon-op/module.mk test/module.mk
diffstat 8 files changed, 146 insertions(+), 130 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/octave-value/ov.cc	Tue Jun 30 15:52:25 2020 -0400
+++ b/libinterp/octave-value/ov.cc	Mon Jun 29 23:45:19 2020 -0400
@@ -2506,103 +2506,95 @@
 
   if (base.isobject () || increment.isobject () || limit.isobject ())
     {
-      std::string dispatch_type;
-
-      if (base.isobject ())
-        dispatch_type = base.class_name ();
-      else if (increment.is_defined () && increment.isobject ())
-        dispatch_type = increment.class_name ();
-      else
-        dispatch_type = limit.class_name ();
-
-      octave::symbol_table& symtab = octave::__get_symbol_table__ ("do_colon_op");
-
-      octave_value meth = symtab.find_method ("colon", dispatch_type);
-
-      if (! meth.is_defined ())
-        error ("colon method not defined for %s class", dispatch_type.c_str ());
-
-      octave_value_list args;
+      octave_value_list tmp1;
 
       if (increment.is_defined ())
         {
-          args(2) = limit;
-          args(1) = increment;
+          tmp1(2) = limit;
+          tmp1(1) = increment;
+          tmp1(0) = base;
         }
       else
-        args(1) = limit;
-
-      args(0) = base;
-
-      octave_value_list tmp = octave::feval (meth.function_value (), args, 1);
-
-      if (tmp.length () > 0)
-        retval = tmp(0);
+        {
+          tmp1(1) = limit;
+          tmp1(0) = base;
+        }
+
+      octave::interpreter& interp = octave::__get_interpreter__ ("do_colon_op");
+
+      octave::symbol_table& symtab = interp.get_symbol_table ();
+
+      octave_value fcn = symtab.find_function ("colon", tmp1);
+
+      if (fcn.is_defined ())
+        {
+          octave_value_list tmp2 = interp.feval (fcn, tmp1, 1);
+
+          return tmp2 (0);
+        }
     }
+
+  bool result_is_str = (base.is_string () && limit.is_string ());
+  bool dq_str = (base.is_dq_string () || limit.is_dq_string ());
+
+  if (base.numel () > 1 || limit.numel () > 1
+      || (increment.is_defined () && increment.numel () > 1))
+    warning_with_id ("Octave:colon-nonscalar-argument",
+                     "colon arguments should be scalars");
+
+  if (base.iscomplex () || limit.iscomplex ()
+      || (increment.is_defined () && increment.iscomplex ()))
+    warning_with_id ("Octave:colon-complex-argument",
+                     "imaginary part of complex colon arguments is ignored");
+
+  Matrix m_base, m_limit, m_increment;
+
+  try
+    {
+      m_base = base.matrix_value (true);
+    }
+  catch (octave::execution_exception& e)
+    {
+      error (e, "invalid base value in colon expression");
+    }
+
+  try
+    {
+      m_limit = limit.matrix_value (true);
+    }
+  catch (octave::execution_exception& e)
+    {
+      error (e, "invalid limit value in colon expression");
+    }
+
+  try
+    {
+      m_increment = (increment.is_defined ()
+                     ? increment.matrix_value (true)
+                     : Matrix (1, 1, 1.0));
+    }
+  catch (octave::execution_exception& e)
+    {
+      error (e, "invalid increment value in colon expression");
+    }
+
+  bool base_empty = m_base.isempty ();
+  bool limit_empty = m_limit.isempty ();
+  bool increment_empty = m_increment.isempty ();
+
+  if (base_empty || limit_empty || increment_empty)
+    retval = Range ();
   else
     {
-      bool result_is_str = (base.is_string () && limit.is_string ());
-      bool dq_str = (base.is_dq_string () || limit.is_dq_string ());
-
-      if (base.numel () > 1 || limit.numel () > 1
-          || (increment.is_defined () && increment.numel () > 1))
-        warning_with_id ("Octave:colon-nonscalar-argument",
-                         "colon arguments should be scalars");
-
-      if (base.iscomplex () || limit.iscomplex ()
-          || (increment.is_defined () && increment.iscomplex ()))
-        warning_with_id ("Octave:colon-complex-argument",
-                         "imaginary part of complex colon arguments is ignored");
-
-      Matrix m_base, m_limit, m_increment;
-
-      try
-        {
-          m_base = base.matrix_value (true);
-        }
-      catch (octave::execution_exception& e)
-        {
-          error (e, "invalid base value in colon expression");
-        }
-
-      try
-        {
-          m_limit = limit.matrix_value (true);
-        }
-      catch (octave::execution_exception& e)
-        {
-          error (e, "invalid limit value in colon expression");
-        }
-
-      try
-        {
-          m_increment = (increment.is_defined ()
-                         ? increment.matrix_value (true)
-                         : Matrix (1, 1, 1.0));
-        }
-      catch (octave::execution_exception& e)
-        {
-          error (e, "invalid increment value in colon expression");
-        }
-
-      bool base_empty = m_base.isempty ();
-      bool limit_empty = m_limit.isempty ();
-      bool increment_empty = m_increment.isempty ();
-
-      if (base_empty || limit_empty || increment_empty)
-        retval = Range ();
-      else
-        {
-          Range r (m_base(0), m_limit(0), m_increment(0));
-
-          // For compatibility with Matlab, don't allow the range used in
-          // a FOR loop expression to be converted to a Matrix.
-
-          retval = octave_value (r, is_for_cmd_expr);
-
-          if (result_is_str)
-            retval = (retval.convert_to_str (false, true, dq_str ? '"' : '\''));
-        }
+      Range r (m_base(0), m_limit(0), m_increment(0));
+
+      // For compatibility with Matlab, don't allow the range used in
+      // a FOR loop expression to be converted to a Matrix.
+
+      retval = octave_value (r, is_for_cmd_expr);
+
+      if (result_is_str)
+        retval = (retval.convert_to_str (false, true, dq_str ? '"' : '\''));
     }
 
   return retval;
--- a/libinterp/parse-tree/pt-colon.cc	Tue Jun 30 15:52:25 2020 -0400
+++ b/libinterp/parse-tree/pt-colon.cc	Mon Jun 29 23:45:19 2020 -0400
@@ -57,52 +57,22 @@
     if (! m_base || ! m_limit)
       return val;
 
-    octave_value ov_base = m_base->evaluate (tw);
-
-    octave_value ov_limit = m_limit->evaluate (tw);
-
-    if (ov_base.isobject () || ov_limit.isobject ())
-      {
-        octave_value_list tmp1;
-
-        if (m_increment)
-          {
-            octave_value ov_increment = m_increment->evaluate (tw);
+    octave_value ov_base;
+    octave_value ov_increment;
+    octave_value ov_limit;
 
-            tmp1(2) = ov_limit;
-            tmp1(1) = ov_increment;
-            tmp1(0) = ov_base;
-          }
-        else
-          {
-            tmp1(1) = ov_limit;
-            tmp1(0) = ov_base;
-          }
-
-        interpreter& interp = tw.get_interpreter ();
-
-        symbol_table& symtab = interp.get_symbol_table ();
-
-        octave_value fcn = symtab.find_function ("colon", tmp1);
-
-        if (! fcn.is_defined ())
-          error ("can not find overloaded colon function");
-
-        octave_value_list tmp2 = feval (fcn, tmp1, 1);
-
-        val = tmp2 (0);
+    if (m_increment)
+      {
+        ov_base = m_base->evaluate (tw);
+        ov_increment = m_increment->evaluate (tw);
+        ov_limit = m_limit->evaluate (tw);
       }
     else
       {
-        octave_value ov_increment = 1.0;
-
-        if (m_increment)
-          ov_increment = m_increment->evaluate (tw);
-
-        val = do_colon_op (ov_base, ov_increment, ov_limit,
-                           is_for_cmd_expr ());
+        ov_base = m_base->evaluate (tw);
+        ov_limit = m_limit->evaluate (tw);
       }
 
-    return val;
+    return do_colon_op (ov_base, ov_increment, ov_limit, is_for_cmd_expr ());
   }
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/colon-op/@legacy_colon_op/colon.m	Mon Jun 29 23:45:19 2020 -0400
@@ -0,0 +1,7 @@
+function r = colon (a, b, c)
+  if (nargin == 2)
+    r = sprintf ("%s:%s", class (a), class (b));
+  else
+    r = sprintf ("%s:%s:%s", class (a), class (b), class (c));
+  end
+end
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/colon-op/@legacy_colon_op/legacy_colon_op.m	Mon Jun 29 23:45:19 2020 -0400
@@ -0,0 +1,3 @@
+function obj = legacy_colon_op ()
+  obj = class (struct (), "legacy_colon_op");
+end
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/colon-op/colon-op.tst	Mon Jun 29 23:45:19 2020 -0400
@@ -0,0 +1,25 @@
+%!test
+%! x = colon_op ();
+%! assert (x:2:3, "colon_op:double:double");
+%! assert (1:x:3, "double:colon_op:double");
+%! assert (1:2:x, "double:double:colon_op");
+%! assert (x:x:3, "colon_op:colon_op:double");
+%! assert (1:x:x, "double:colon_op:colon_op");
+%! assert (x:2:x, "colon_op:double:colon_op");
+%! assert (x:x:x, "colon_op:colon_op:colon_op");
+%! assert (x:2, "colon_op:double");
+%! assert (1:x, "double:colon_op");
+%! assert (x:x, "colon_op:colon_op");
+
+%!test
+%! x = legacy_colon_op ();
+%! assert (x:2:3, "legacy_colon_op:double:double");
+%! assert (1:x:3, "double:legacy_colon_op:double");
+%! assert (1:2:x, "double:double:legacy_colon_op");
+%! assert (x:x:3, "legacy_colon_op:legacy_colon_op:double");
+%! assert (1:x:x, "double:legacy_colon_op:legacy_colon_op");
+%! assert (x:2:x, "legacy_colon_op:double:legacy_colon_op");
+%! assert (x:x:x, "legacy_colon_op:legacy_colon_op:legacy_colon_op");
+%! assert (x:2, "legacy_colon_op:double");
+%! assert (1:x, "double:legacy_colon_op");
+%! assert (x:x, "legacy_colon_op:legacy_colon_op");
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/colon-op/colon_op.m	Mon Jun 29 23:45:19 2020 -0400
@@ -0,0 +1,11 @@
+classdef colon_op
+  methods
+    function r = colon (a, b, c)
+      if (nargin == 2)
+        r = sprintf ("%s:%s", class (a), class (b));
+      else
+        r = sprintf ("%s:%s:%s", class (a), class (b), class (c));
+      end
+    end
+  end
+end
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/colon-op/module.mk	Mon Jun 29 23:45:19 2020 -0400
@@ -0,0 +1,7 @@
+colon_op_TEST_FILES = \
+  %reldir%/@legacy_colon_op/colon.m \
+  %reldir%/@legacy_colon_op/legacy_colon_op.m \
+  %reldir%/colon-op.tst \
+  %reldir%/colon_op.m
+
+TEST_FILES += $(colon_op_TEST_FILES)
--- a/test/module.mk	Tue Jun 30 15:52:25 2020 -0400
+++ b/test/module.mk	Mon Jun 29 23:45:19 2020 -0400
@@ -87,6 +87,7 @@
 include %reldir%/classdef/module.mk
 include %reldir%/classdef-multiple-inheritance/module.mk
 include %reldir%/classes/module.mk
+include %reldir%/colon-op/module.mk
 include %reldir%/ctor-vs-method/module.mk
 include %reldir%/fcn-handle/module.mk
 include %reldir%/local-functions/module.mk