# HG changeset patch
# User Julien Bect <julien.bect@centralesupelec.fr>
# Date 1455110962 -3600
#      Wed Feb 10 14:29:22 2016 +0100
# Branch stable
# Node ID e32ca498de8de5af02f72c6ab4fd4739d097b0fc
# Parent  44f7664689f2ce10d5444768a743a6a3961e3497
Respect user-defined size() (bug #43925, bug #44498)

* data.cc (Frows): Call size() insteas of rows().
* data.cc (Fcolumns): Call size() insteas of columns().
* data.cc (Fisscalar): Call size() instead of numel().
* data.cc (Fisrow): Call size() instead of dims().
* data.cc (Fiscolumn): Call size() instead of dims().
* data.cc (Fisvector): Call size() instead of dims().
* data.cc (Fismatrix): Call size() instead of dims().
* data.cc (Fissquare): Call size() instead of dims().
* test/classes/classes.tst: Update unit tests.
* test/classes/@SizeTester/SizeTester.m: Make it usable in Matlab as well.
* test/classes/@SizeTester/numel.m: Idem.
* test/classes/@SizeTester/size.m: Idem.

diff -r 44f7664689f2 -r e32ca498de8d libinterp/corefcn/data.cc
--- a/libinterp/corefcn/data.cc	Fri May 06 17:02:31 2016 -0400
+++ b/libinterp/corefcn/data.cc	Wed Feb 10 14:29:22 2016 +0100
@@ -2935,7 +2935,11 @@
   octave_value retval;
 
   if (args.length () == 1)
-    retval = args(0).rows ();
+    {
+      // Use size() instead of rows() to allow user overloading.
+      // Create of copy of args(0) since size() isn't const
+      retval = (octave_value (args(0)).size ())(0);
+    }
   else
     print_usage ();
 
@@ -2952,7 +2956,11 @@
   octave_value retval;
 
   if (args.length () == 1)
-    retval = args(0).columns ();
+    {
+      // Use size() instead of columns() to allow user overloading.
+      // Create of copy of args(0) since size() isn't const
+      retval = (octave_value (args(0)).size ())(1);
+    }
   else
     print_usage ();
 
@@ -3717,7 +3725,14 @@
   octave_value retval;
 
   if (args.length () == 1)
-    retval = args(0).numel () == 1;
+    {
+      // Use size() instead of dims() to allow user overloading. *Not*
+      // to be replaced by numel(), since numel() might be overloaded in
+      // such a way that numel(x) differs from prod(size(x)).
+      // Create of copy of args(0) since size() isn't const
+      Matrix sz = octave_value (args(0)).size ();
+      retval = (sz.numel () == 2) && (sz(0) == 1) && (sz(1) == 1);
+    }
   else
     print_usage ();
 
@@ -3757,8 +3772,14 @@
 
   if (args.length () == 1)
     {
-      dim_vector sz = args(0).dims ();
-      retval = sz.length () == 2 && (sz(0) == 1 || sz(1) == 1);
+      // Use size() instead of dims() to allow user overloading.
+      // Create of copy of args(0) since size() isn't const
+      Matrix sz = octave_value (args(0)).size ();
+      // sz must be of the form [1 n] or [n 1],
+      // with n a non-negative integer
+      retval = (sz.numel () == 2)
+        && ((sz(0) == 1 && sz(1) >= 0 && xisinteger (sz(1)))       // row
+            || (sz(1) == 1 && sz(0) >= 0 && xisinteger (sz(0))));  // or column
     }
   else
     print_usage ();
@@ -3797,8 +3818,13 @@
 
   if (args.length () == 1)
     {
-      dim_vector sz = args(0).dims ();
-      retval = sz.length () == 2 && sz(0) == 1;
+      // Use size() instead of dims() to allow user overloading.
+      // Create of copy of args(0) since size() isn't const
+      Matrix sz = octave_value (args(0)).size ();
+      // sz must be of the form [1 n],
+      // with n a non-negative integer
+      retval = (sz.numel () == 2) && (sz(0) == 1)
+        && (sz(1) >= 0) && (xisinteger (sz(1)));
     }
   else
     print_usage ();
@@ -3847,8 +3873,13 @@
 
   if (args.length () == 1)
     {
-      dim_vector sz = args(0).dims ();
-      retval = sz.length () == 2 && sz(1) == 1;
+      // Use size() instead of dims() to allow user overloading.
+      // Create of copy of args(0) since size() isn't const
+      Matrix sz = octave_value (args(0)).size ();
+      // sz must be of the form [n 1],
+      // with n a non-negative integer
+      retval = (sz.numel () == 2) && (sz(1) == 1)
+        && (sz(0) >= 0) && (xisinteger (sz(0)));
     }
   else
     print_usage ();
@@ -3896,8 +3927,14 @@
 
   if (args.length () == 1)
     {
-      dim_vector sz = args(0).dims ();
-      retval = (sz.length () == 2) && (sz(0) >= 0) && (sz(1) >= 0);
+      // Use size() instead of dims() to allow user overloading.
+      // Create of copy of args(0) since size() isn't const
+      Matrix sz = octave_value (args(0)).size ();
+      // sz must be of the form [m n],
+      // with m and n non-negative integers
+      retval = (sz.numel () == 2)
+        && (sz(0) >= 0) && (xisinteger (sz(0)))
+        && (sz(1) >= 0) && (xisinteger (sz(1)));
     }
   else
     print_usage ();
@@ -3944,8 +3981,13 @@
 
   if (args.length () == 1)
     {
-      dim_vector sz = args(0).dims ();
-      retval = sz.length () == 2 && sz(0) == sz(1);
+      // Use size() instead of dims() to allow user overloading.
+      // Create of copy of args(0) since size() isn't const
+      Matrix sz = octave_value (args(0)).size ();
+      // sz must be of the form [n n],
+      // with n a non-negative integer
+      retval = (sz.numel () == 2) && (sz(0) == sz(1))
+        && (sz(0) >= 0) && (xisinteger (sz(0)));
     }
   else
     print_usage ();
diff -r 44f7664689f2 -r e32ca498de8d test/classes/@SizeTester/SizeTester.m
--- a/test/classes/@SizeTester/SizeTester.m	Fri May 06 17:02:31 2016 -0400
+++ b/test/classes/@SizeTester/SizeTester.m	Wed Feb 10 14:29:22 2016 +0100
@@ -1,6 +1,6 @@
 function x = SizeTester (desired_size)
 
-  x = struct ("desired_size", desired_size);
-  x = class (x, "SizeTester");
+  x = struct ('desired_size', desired_size);
+  x = class (x, 'SizeTester');
 
-endfunction
+end
diff -r 44f7664689f2 -r e32ca498de8d test/classes/@SizeTester/numel.m
--- a/test/classes/@SizeTester/numel.m	Fri May 06 17:02:31 2016 -0400
+++ b/test/classes/@SizeTester/numel.m	Wed Feb 10 14:29:22 2016 +0100
@@ -2,4 +2,4 @@
 
   n = prod (this.desired_size);
 
-endfunction
+end
diff -r 44f7664689f2 -r e32ca498de8d test/classes/@SizeTester/size.m
--- a/test/classes/@SizeTester/size.m	Fri May 06 17:02:31 2016 -0400
+++ b/test/classes/@SizeTester/size.m	Wed Feb 10 14:29:22 2016 +0100
@@ -2,4 +2,4 @@
 
   s = this.desired_size;
 
-endfunction
+end
diff -r 44f7664689f2 -r e32ca498de8d test/classes/classes.tst
--- a/test/classes/classes.tst	Fri May 06 17:02:31 2016 -0400
+++ b/test/classes/classes.tst	Wed Feb 10 14:29:22 2016 +0100
@@ -1,4 +1,4 @@
-## Copyright (C) 2013-2015 Julien Bect
+## Copyright (C) 2013-2016 Julien Bect
 ## Copyright (C) 2009-2015 Robert T. Short
 ##
 ## This file is part of Octave.
@@ -20,12 +20,10 @@
 %% Test script for legacy OOP.
 %% Requires the path to contain the test classes.
 %%
-%% Note: This script and all classes are also intended to run
-%%       in MATLAB to test compatibility.  Don't break that!
-%%
-%%       Note(JBect, 2013/01/27) : in order to ease to process of testing
-%%       Matlab compatibility, the syntax assert(observed, expected) should
-%%       be avoided. I use assert(isequal(observed, expected) instead.
+%% Note: This script and all classes are also intended to run in MATLAB to
+%%       test compatibility.  Don't break that if possible.  In particular,
+%%       use assert (isequal (observed, expected)) instead of
+%%       assert (observed, expected).
 %%
 %% To Do:  This script tests to ensure that things done correctly work
 %%         corrrectly.  It should also check that things done incorrectly
@@ -310,15 +308,15 @@
 
 %% Test overloaded vertcat() for the Snork class
 %% See bug #38128 (http://savannah.gnu.org/bugs/?38128)
-%!test   s = [s1; s2];  assert (isa (s, 'Snork') && isequal (s.gick, [x1; x2]));
-%!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]));
+%!test  s = [s1; s2];  assert (isa (s, 'Snork') && isequal (s.gick, [x1; x2]));
+%!test  s = [s1; x2];  assert (isa (s, 'Snork') && isequal (s.gick, [x1; x2]));
+%!test  s = [x1; s2];  assert (isa (s, 'Snork') && isequal (s.gick, [x1; x2]));
 
 %% Test overloaded horzcat() for the Snork class
 %% See bug #38128 (http://savannah.gnu.org/bugs/?38128)
-%!test   s = [s1 s2];  assert (isa (s, 'Snork') && isequal (s.gick, [x1 x2]));
-%!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]));
+%!test  s = [s1 s2];  assert (isa (s, 'Snork') && isequal (s.gick, [x1 x2]));
+%!test  s = [s1 x2];  assert (isa (s, 'Snork') && isequal (s.gick, [x1 x2]));
+%!test  s = [x1 s2];  assert (isa (s, 'Snork') && isequal (s.gick, [x1 x2]));
 
 %% Test with the Blork class, where neither vertcat() nor horzcat() is overloaded
 %!shared x1, x2, x3
@@ -361,49 +359,72 @@
 %% Trying to change to CPrecendenceTester3 > Snork
 %!error D = CPrecedenceTester3 (1);
 
-##############################################
-## Testing overridden size+numel method     ##
-## (builtin size method and similar methods ##
-## use the size of the struct container)    ##
-##############################################
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+%% Testing overridden size+numel method     %%
+%% (builtin size method and similar methods %%
+%% use the size of the struct container)    %%
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+
+% Non-regression tests for bugs #44334 and #45118 concerning isequal
+%!shared st
+%!test st = SizeTester ([1 1]);
+%!assert (isequal (st, st))                    % This is for bug #44334
+%!test x = struct ('a', st);  isequal (x, x);  % This is for bug #45118
+
+% Some explanations about the following tests:
+% 1/ for Matlab compatiblity,
+%     * isscalar and isvector must respect the value returned by the overriden
+%       size function for Matlab compatibility (see bugs #43925 and #44498).
+%     * numel doesn't respect size in general, unless it is also overriden to do
+%       so which is the case in the @SizeTester class used here); the rationale
+%       is that numel is related to the size of the comma-separated list
+%       obtained when using dot-indexing.
+%     * ndims doesn't respect size either, although Mathworks's documentation
+%       suggests that it should; this can be considered as a bug in Matlab, but
+%       this is also how Octave has behaved until now (no regression here).
+%       See http://octave.1599824.n4.nabble.com/Matlab-compatibility-or-not-Matlab-compatibility-tp4674770.html
+% 2/ for consistency
+%     * Octave-specific functions such as rows, columns and issquare
+%       also respect the value returned by the overriden size function .
 
 %!shared st
 %!test st = SizeTester ([1 1]);
-%! assert (isequal (size (st), [1 1]))
-%! assert (isequal (numel (st), 1))
+%!assert (isequal (size (st), [1 1]))
+%!assert (isequal (numel (st), 1))
 %!assert (isequal (ndims (st), 2))
 %!assert (isequal (rows (st), 1))
 %!assert (isequal (columns (st), 1))
-%!assert (isequal (st, st))
 %!assert (isscalar (st))
 %!assert (isvector (st))
+%!assert (issquare (st))
+
 
 %!test st = SizeTester ([1 2]);
-%! assert (isequal (size (st), [1 2]))
-%! assert (isequal (numel (st), 2))
+%!assert (isequal (size (st), [1 2]))
+%!assert (isequal (numel (st), 2))
 %!assert (isequal (ndims (st), 2))
 %!assert (isequal (rows (st), 1))
-%!xtest assert (isequal (columns (st), 2))
-%!assert (isequal (st, st))                # bug #44334
-%!xtest assert (not (isscalar (st)))       # bug #44498
+%!assert (isequal (columns (st), 2))
+%!assert (~ (isscalar (st)))
 %!assert (isvector (st))
+%!assert (~ issquare (st))
 
 %!test st = SizeTester ([2 3]);
-%! assert (isequal (size (st), [2 3]))
-%! assert (isequal (numel (st), 6))
+%!assert (isequal (size (st), [2 3]))
+%!assert (isequal (numel (st), 6))
 %!assert (isequal (ndims (st), 2))
-%!xtest assert (isequal (rows (st), 2))
-%!xtest assert (isequal (columns (st), 3))
-%!assert (isequal (st, st))                # bug #44334
-%!xtest assert (not (isscalar (st)))       # bug #44498
-%!xtest assert (not (isvector (st)))       # bug #44498
+%!assert (isequal (rows (st), 2))
+%!assert (isequal (columns (st), 3))
+%!assert (~ (isscalar (st)))
+%!assert (~ (isvector (st)))
+%!assert (~ issquare (st))
 
 %!test st = SizeTester ([2 3 4]);
-%! assert (isequal (size (st), [2 3 4]))
-%! assert (isequal (numel (st), 24))
-%!xtest assert (isequal (ndims (st), 3))
-%!xtest assert (isequal (rows (st), 2))
-%!xtest assert (isequal (columns (st), 3))
-%!assert (isequal (st, st))                # bug #44334
-%!xtest assert (not (isscalar (st)))       # bug #44498
-%!xtest assert (not (isvector (st)))       # bug #44498
+%!assert (isequal (size (st), [2 3 4]))
+%!assert (isequal (numel (st), 24))
+%!assert (isequal (ndims (st), 2))   % 2 instead of 3, for Matlab compatibility
+%!assert (isequal (rows (st), 2))
+%!assert (isequal (columns (st), 3))
+%!assert (~ (isscalar (st)))
+%!assert (~ (isvector (st)))
+%!assert (~ issquare (st))
