Mercurial > octave

changeset 25926:373fe1608f7c

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Emit an error when utility matrices (zeros, ones, rand, etc.) are given a fractional dimension (bug #54781). * data.cc (fill_matrix): Replace calls to "xidx_type_value" with "idx_type_value (true)" which enables input checking. * data.cc (Fones): Add new BIST tests for negative dimensions, fractional dimensions, unknown conversion type. * rand.cc (do_rand): Replace call to "xidx_type_value" with "idx_type_value (true)" which enables input checking. * rand.cc (Frand): Add new BIST tests for negative dimensions and fractional dimensions. * diag-perm.tst: Fix incorrect BIST test using a fractional dimension.
author Rik <rik@octave.org>
date Thu, 11 Oct 2018 14:04:47 -0700
parents e9c24b5e8673
children d6581134daaa
files libinterp/corefcn/data.cc libinterp/corefcn/rand.cc test/diag-perm.tst
diffstat 3 files changed, 26 insertions(+), 21 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/corefcn/data.cc	Thu Oct 11 18:46:05 2018 +0200
+++ b/libinterp/corefcn/data.cc	Thu Oct 11 14:04:47 2018 -0700
@@ -3931,8 +3931,7 @@
         dims.resize (nargin);
 
         for (int i = 0; i < nargin; i++)
-          dims(i) = (args(i).isempty ()
-                     ? 0 : args(i).xidx_type_value ("%s: dimension arguments must be scalar integers", fcn));
+          dims(i) = (args(i).isempty () ? 0 : args(i).idx_type_value (true));
       }
       break;
     }
@@ -3941,9 +3940,8 @@
 
   octave::check_dimensions (dims, fcn);
 
-  // FIXME: perhaps this should be made extensible by
-  // using the class name to lookup a function to call to create
-  // the new value.
+  // FIXME: Perhaps this should be made extensible by using the class name
+  //        to lookup a function to call to create the new value.
 
   // Note that automatic narrowing will handle conversion from
   // NDArray to scalar.
@@ -4041,8 +4039,7 @@
         dims.resize (nargin);
 
         for (int i = 0; i < nargin; i++)
-          dims(i) = (args(i).isempty ()
-                     ? 0 : args(i).xidx_type_value ("%s: dimension arguments must be scalar integers", fcn));
+          dims(i) = (args(i).isempty () ? 0 : args(i).idx_type_value (true));
       }
       break;
     }
@@ -4105,8 +4102,7 @@
         dims.resize (nargin);
 
         for (int i = 0; i < nargin; i++)
-          dims(i) = (args(i).isempty ()
-                     ? 0 : args(i).xidx_type_value ("%s: dimension arguments must be scalar integers", fcn));
+          dims(i) = (args(i).isempty () ? 0 : args(i).idx_type_value (true));
       }
       break;
     }
@@ -4170,8 +4166,7 @@
         dims.resize (nargin);
 
         for (int i = 0; i < nargin; i++)
-          dims(i) = (args(i).isempty ()
-                     ? 0 : args(i).xidx_type_value ("%s: dimension arguments must be scalar integers", fcn));
+          dims(i) = (args(i).isempty () ? 0 : args(i).idx_type_value (true));
       }
       break;
     }
@@ -4225,8 +4220,7 @@
         dims.resize (nargin);
 
         for (int i = 0; i < nargin; i++)
-          dims(i) = (args(i).isempty ()
-                     ? 0 : args(i).xidx_type_value ("%s: dimension arguments must be scalar integers", fcn));
+          dims(i) = (args(i).isempty () ? 0 : args(i).idx_type_value (true));
       }
       break;
     }
@@ -4292,6 +4286,8 @@
 %!assert (ones (2, 3, "int8"), int8 ([1, 1, 1; 1, 1, 1]))
 %!assert (ones (3, 2, "int8"), int8 ([1, 1; 1, 1; 1, 1]))
 %!assert (size (ones (3, 4, 5, "int8")), [3, 4, 5])
+
+%!assert (size (ones (1, -2, 2)), [1, 0, 2])
 */
 
 /*
@@ -4309,6 +4305,11 @@
 %!   fail ([func2str(func) " ([])"]);
 %!   fail ([func2str(func) " (zeros (0, 0, 1))"]);
 %! endfor
+
+## Test input validation
+%!error <invalid data type specified> zeros (1, 1, "foobar")
+%!error <conversion of 1.1 .*failed> zeros (1, 1.1, 2)
+
 */
 
 DEFUN (zeros, args, ,
@@ -4927,9 +4928,8 @@
 {
   octave_value retval;
 
-  // FIXME: perhaps this should be made extensible by using
-  // the class name to lookup a function to call to create the new
-  // value.
+  // FIXME: Perhaps this should be made extensible by using the class name
+  //        to lookup a function to call to create the new value.
 
   switch (dt)
     {
--- a/libinterp/corefcn/rand.cc	Thu Oct 11 18:46:05 2018 +0200
+++ b/libinterp/corefcn/rand.cc	Thu Oct 11 14:04:47 2018 -0700
@@ -286,10 +286,7 @@
 
             for (int i = 0; i < nargin; i++)
               {
-                octave_idx_type elt =
-                  args(idx+i).xidx_type_value (
-                    "%s: dimension must be a scalar or array of integers",
-                    fcn);
+                octave_idx_type elt = args(idx+i).idx_type_value (true);
 
                 // Negative dimensions treated as zero for Matlab compatibility
                 dims(i) = (elt >= 0 ? elt : 0);
@@ -538,6 +535,14 @@
 %!assert (__rand_sample__ (NaN), __rand_sample__ (0))
 */
 
+/*
+## Check that negative dimensions are treated as zero for Matlab compatibility
+%!assert (size (rand (1, -1, 2)), [1, 0, 2])
+
+## Test input validation
+%!error <conversion of 1.1 to.* failed> rand (1, 1.1)
+*/
+
 static std::string current_distribution = octave::rand::distribution ();
 
 DEFUN (randn, args, ,
--- a/test/diag-perm.tst	Thu Oct 11 18:46:05 2018 +0200
+++ b/test/diag-perm.tst	Thu Oct 11 14:04:47 2018 -0700
@@ -77,7 +77,7 @@
 %! P1 = eye (1) (:, [1]);
 %! A1 = 1;
 %! P = eye (n) (:, randperm (n));
-%! A = rand (n-3, n, .5);
+%! A = rand (n-3, n);
 %! assert (typeinfo (A * P1), "matrix");
 %! assert (full (A * P1), full (A) * P1);
 %! assert (typeinfo (P1 * A), "matrix");