changeset 32059:bade9602c5a1

Validate M and N dimension inputs to sparse(). * sparse.cc (Fsparse): Use xidx_type_value() to extract an octave_idx_type for M and N inputs or throw an error. Remove call to get_dimensions(). Remove FIXME note about unreachable code. Update error messages to be specific about what is wrong with M or N inputs. * mk-sparse-tst.sh: Remove FIXME notes. Update BIST tests for M and N inputs.
author Rik <rik@octave.org>
date Wed, 26 Apr 2023 14:40:37 -0700
parents e242124f1240
children 1203a2d81a42
files libinterp/corefcn/sparse.cc test/mk-sparse-tst.sh
diffstat 2 files changed, 15 insertions(+), 20 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/corefcn/sparse.cc	Wed Apr 26 10:09:09 2023 -0700
+++ b/libinterp/corefcn/sparse.cc	Wed Apr 26 14:40:37 2023 -0700
@@ -199,15 +199,11 @@
     }
   else if (nargin == 2)
     {
-      octave_idx_type m = 0;
-      octave_idx_type n = 0;
-
-      get_dimensions (args(0), args(1), "sparse", m, n);
+      octave_idx_type m = args(0).xidx_type_value ("sparse: M must be a non-negative integer");
+      octave_idx_type n = args(1).xidx_type_value ("sparse: N must be a non-negative integer");
 
-      // FIXME: this code is never active because get_dimensions()
-      //        replaces negative dimensions with 0.
       if (m < 0 || n < 0)
-        error ("sparse: dimensions must be non-negative");
+        error ("sparse: dimensions M and N must be non-negative");
 
       retval = SparseMatrix (m, n);
     }
@@ -237,15 +233,14 @@
 
       if (nargin == 5)
         {
-          get_dimensions (args(3), args(4), "sparse", m, n);
+          m = args(3).xidx_type_value ("sparse: M must be a non-negative integer");
+          n = args(4).xidx_type_value ("sparse: N must be a non-negative integer");
 
-          // FIXME: this code is never active because get_dimensions()
-          //        replaces negative dimensions with 0.
           if (m < 0 || n < 0)
-            error ("sparse: dimensions must be non-negative");
+            error ("sparse: dimensions M and N must be non-negative");
         }
 
-      int argidx = 0;    // index we're checking when index_vector throws
+      int argidx = 0;    // index being checked when index_vector throws
       try
         {
           idx_vector i = args(0).index_vector ();
--- a/test/mk-sparse-tst.sh	Wed Apr 26 10:09:09 2023 -0700
+++ b/test/mk-sparse-tst.sh	Wed Apr 26 14:40:37 2023 -0700
@@ -182,19 +182,19 @@
 %! s = sparse (single ([1 2]));
 %!error <wrong type argument 'uint8 matrix'>
 %! s = sparse (uint8 ([1 2]));
-%% FIXME: negative dimensions are allowed and replaced with 0.
-%%        If this is fixed, re-instate these tests.
-%!#error <dimensions must be non-negative> sparse (-1, 2)
-%!#error <dimensions must be non-negative> sparse (1, -2)
+%!error <M must be a non-negative integer> sparse ({1}, 2)
+%!error <N must be a non-negative integer> sparse (1, {2})
+%!error <dimensions M and N must be non-negative> sparse (-1, 2)
+%!error <dimensions M and N must be non-negative> sparse (1, -2)
 %!error <dimension mismatch> sparse (1,[2,3],[1,2,3])
 %!error <invalid option: foobar> sparse ([1,1],[1,1],[1,2],"foobar")
 %% negative subscripts are disallowed
 %!error <subscripts must be> sparse ([1,3],[1,-4],[3,5],2,2)
 %!error <subscripts must be> sparse ([1,3],[1,-4],[3,5i],2,2)
-%% FIXME: negative dimensions are allowed and replaced with 0.
-%%        If this is fixed, re-instate these tests.
-%!#error <dimensions must be non-negative> sparse ([1,1],[1,1],[1,2], -1, 2)
-%!#error <dimensions must be non-negative> sparse ([1,1],[1,1],[1,2], 1, -2)
+%!error <M must be a non-negative integer> sparse ([1,1],[1,1],[1,2], {1}, 2)
+%!error <N must be a non-negative integer> sparse ([1,1],[1,1],[1,2], 1, {2})
+%!error <dimensions M and N must be non-negative> sparse ([1,1],[1,1],[1,2], -1, 2)
+%!error <dimensions M and N must be non-negative> sparse ([1,1],[1,1],[1,2], 1, -2)
 %!warning <input array cast to double>
 %! warning ("on", "Octave:sparse:double-conversion", "local");
 %! s = sparse ([1,1],[1,1], single ([1,2]), 2, 2);