changeset 23346:957d95286f52

avoid indexing error with "end" and classdef objects (bug #50716) * pt-idx.cc (tree_index_expression::rvalue1): Avoid error with expressions like this: obj.meth(var(end)). * test/bug-50716/bug-50716.tst, test/bug-50716/myclass.m: New tests. * test/bug-50716/module.mk: New file. * test/module.mk: Update.
author John W. Eaton <jwe@octave.org>
date Wed, 05 Apr 2017 09:00:40 -0400
parents 0b6810085ed3
children ed25d1f8163c
files libinterp/parse-tree/pt-idx.cc test/bug-50716/bug-50716.tst test/bug-50716/module.mk test/bug-50716/myclass.m test/module.mk
diffstat 5 files changed, 87 insertions(+), 17 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/parse-tree/pt-idx.cc	Wed Apr 05 13:26:45 2017 +0200
+++ b/libinterp/parse-tree/pt-idx.cc	Wed Apr 05 09:00:40 2017 -0400
@@ -360,25 +360,38 @@
           {
             tree_argument_list *al = *p_args;
 
-            // In Matlab, () can only be followed by '.'.  In Octave, we do not
-            // enforce this for rvalue expressions, but we'll split the
-            // evaluation at this point.  This will, hopefully, allow Octave's
-            // looser rules apply smoothly for Matlab overloaded subsref
-            // codes.
-            bool force_split = type[i-1] == '(' && type[i] != '.';
+            // In Matlab, () can only be followed by '.'.  In Octave, we
+            // do not enforce this for rvalue expressions, but we'll
+            // split the evaluation at this point.  This will,
+            // hopefully, allow Octave's looser rules apply smoothly for
+            // Matlab overloaded subsref codes.
+
+            // We might have an expression like
+            //
+            //   x{end}.a(end)
+            //
+            // and we are looking at the argument list that contains the
+            // second (or third, etc.) "end" token, so we must evaluate
+            // everything up to the point of that argument list so we
+            // can pass the appropriate value to the built-in end
+            // function.
 
-            if (force_split || (al && al->has_magic_end ()))
+            // An expression like
+            //
+            //    s.a (f (1:end))
+            //
+            // can mean a lot of different things depending on the types
+            // of s, a, and f.  Let's just say it's complicated and that
+            // the following code is definitely not correct in all
+            // cases.  That it is already so complex makes me think that
+            // there must be a better way.
+             
+            bool split = ((type[i-1] == '(' && type[i] != '.')
+                          || (al && al->has_magic_end ()
+                              && ! tmp.is_classdef_object ()));
+
+            if (split)
               {
-                // (we have force_split, or) we have an expression like
-                //
-                //   x{end}.a(end)
-                //
-                // and we are looking at the argument list that
-                // contains the second (or third, etc.) "end" token,
-                // so we must evaluate everything up to the point of
-                // that argument list so we can pass the appropriate
-                // value to the built-in end function.
-
                 try
                   {
                     octave_value_list tmp_list
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-50716/bug-50716.tst	Wed Apr 05 09:00:40 2017 -0400
@@ -0,0 +1,37 @@
+## Copyright (C) 2017 John W. Eaton
+##
+## This file is part of Octave.
+##
+## Octave is free software; you can redistribute it and/or modify it
+## under the terms of the GNU General Public License as published by
+## the Free Software Foundation; either version 3 of the License, or
+## (at your option) any later version.
+##
+## Octave is distributed in the hope that it will be useful, but
+## WITHOUT ANY WARRANTY; without even the implied warranty of
+## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+## GNU General Public License for more details.
+##
+## You should have received a copy of the GNU General Public License
+## along with Octave; see the file COPYING.  If not, see
+## <http://www.gnu.org/licenses/>.
+
+%!test
+%! obj = myclass ();
+%! str = "Octave";
+%! val = obj.methodA (str(1:end));
+%! assert (val, str);
+
+%!test <50716>
+%! obj = myclass ();
+%! foo = {obj};
+%! str = "Octave";
+%! val = foo{1}.methodA (str(1:end));
+%! assert (val, str);
+
+%!test <50716>
+%! obj = myclass ();
+%! foo = {obj};
+%! str = "Octave";
+%! val = foo{end}.methodA (str(1:end));
+%! assert (val, str);
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-50716/module.mk	Wed Apr 05 09:00:40 2017 -0400
@@ -0,0 +1,6 @@
+
+bug_50716_TEST_FILES = \
+  test/bug-50716/myclass.m
+  test/bug-50716/bug-50716.tst
+
+TEST_FILES += $(bug_50716_TEST_FILES)
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-50716/myclass.m	Wed Apr 05 09:00:40 2017 -0400
@@ -0,0 +1,13 @@
+classdef myclass < handle
+
+  methods
+    function obj = myclass
+    endfunction
+
+    function r = methodA (obj, val)
+      r = val;
+    endfunction
+
+  endmethods
+
+endclassdef
--- a/test/module.mk	Wed Apr 05 13:26:45 2017 +0200
+++ b/test/module.mk	Wed Apr 05 09:00:40 2017 -0400
@@ -52,6 +52,7 @@
 include test/bug-46660/module.mk
 include test/bug-50014/module.mk
 include test/bug-50035/module.mk
+include test/bug-50716/module.mk
 include test/class-concat/module.mk
 include test/classdef/module.mk
 include test/classdef-multiple-inheritance/module.mk