Mercurial > hg > octave-lyh
changeset 10366:e5ae13b8b2c2
improve Array indexing error messages
author | Jaroslav Hajek <highegg@gmail.com> |
---|---|
date | Sat, 27 Feb 2010 08:37:34 +0100 |
parents | 532802559f39 |
children | 173e10268080 |
files | liboctave/Array-util.cc liboctave/Array-util.h liboctave/Array.cc liboctave/Array.h liboctave/ChangeLog liboctave/DiagArray2.cc liboctave/Range.cc liboctave/dim-vector.h src/ChangeLog src/ov-base-mat.cc test/test_struct.m |
diffstat | 11 files changed, 198 insertions(+), 188 deletions(-) [+] |
line wrap: on
line diff
--- a/liboctave/Array-util.cc +++ b/liboctave/Array-util.cc @@ -641,8 +641,8 @@ } void -gripe_nonconformant (const char *op, dim_vector& op1_dims, - dim_vector& op2_dims) +gripe_nonconformant (const char *op, const dim_vector& op1_dims, + const dim_vector& op2_dims) { std::string op1_dims_str = op1_dims.str (); std::string op2_dims_str = op2_dims.str (); @@ -651,3 +651,34 @@ ("%s: nonconformant arguments (op1 is %s, op2 is %s)", op, op1_dims_str.c_str (), op2_dims_str.c_str ()); } + +void gripe_index_out_of_range (int nd, int dim, + octave_idx_type idx, octave_idx_type ext) +{ + switch (nd) + { + case 1: + (*current_liboctave_error_handler) + ("A(I): index out of bounds; value %d out of bound %d", + idx, ext); + break; + case 2: + (*current_liboctave_error_handler) + ("A(I,J): %s index out of bounds; value %d out of bound %d", + (dim == 1) ? "row" : "column", idx, ext); + break; + default: + (*current_liboctave_error_handler) + ("A(I,J,...): index to dimension %d out of bounds; value %d out of bound %d", + dim, idx, ext); + break; + } +} + +void gripe_del_index_out_of_range (bool is1d, octave_idx_type idx, + octave_idx_type ext) +{ + (*current_liboctave_error_handler) + ("A(%s) = []: index out of bounds; value %d out of bound %d", + is1d ? "I" : "..,I,..", idx, ext); +}
--- a/liboctave/Array-util.h +++ b/liboctave/Array-util.h @@ -99,7 +99,13 @@ int op2_nr, int op2_nc); -extern void OCTAVE_API gripe_nonconformant (const char *op, dim_vector& op1_dims, - dim_vector& op2_dims); +extern void OCTAVE_API gripe_nonconformant (const char *op, const dim_vector& op1_dims, + const dim_vector& op2_dims); + +extern void OCTAVE_API gripe_index_out_of_range (int nd, int dim, + octave_idx_type iext, octave_idx_type ext); + +extern void OCTAVE_API gripe_del_index_out_of_range (bool is1d, octave_idx_type iext, + octave_idx_type ext); #endif
--- a/liboctave/Array.cc +++ b/liboctave/Array.cc @@ -206,60 +206,102 @@ } template <class T> -T& -Array<T>::range_error (const char *fcn, octave_idx_type n) const +T& +Array<T>::checkelem (octave_idx_type n) { - (*current_liboctave_error_handler) ("%s (%d): range error", fcn, n); - static T foo; - return foo; + if (n < 0 || n >= slice_len) + gripe_index_out_of_range (1, 1, n+1, slice_len); + + return elem (n); } template <class T> -T& -Array<T>::range_error (const char *fcn, octave_idx_type i, octave_idx_type j) const +T& +Array<T>::checkelem (octave_idx_type i, octave_idx_type j) { - (*current_liboctave_error_handler) - ("%s (%d, %d): range error", fcn, i, j); - static T foo; - return foo; + if (i < 0 || i >= dim1 ()) + gripe_index_out_of_range (2, 1, i+1, dim1 ()); + if (j < 0 || j >= dimensions.numel (1)) + gripe_index_out_of_range (2, 2, j+1, dimensions.numel (1)); + + return elem (i, j); +} + +template <class T> +T& +Array<T>::checkelem (octave_idx_type i, octave_idx_type j, octave_idx_type k) +{ + if (i < 0 || i >= dim1 ()) + gripe_index_out_of_range (3, 1, i+1, dim1 ()); + if (j < 0 || j >= dim2 ()) + gripe_index_out_of_range (3, 2, j+1, dim2 ()); + if (k < 0 || k >= dimensions.numel (2)) + gripe_index_out_of_range (3, 3, k+1, dimensions.numel (2)); + + return elem (i, j, k); } template <class T> -T& -Array<T>::range_error (const char *fcn, octave_idx_type i, octave_idx_type j, octave_idx_type k) const +T& +Array<T>::checkelem (const Array<octave_idx_type>& ra_idx) { - (*current_liboctave_error_handler) - ("%s (%d, %d, %d): range error", fcn, i, j, k); - static T foo; - return foo; + int nd = ra_idx.length (); + const dim_vector dv = dimensions.redim (nd); + for (int d = 0; d < nd; d++) + if (ra_idx(d) < 0 || ra_idx(d) >= dv(d)) + gripe_index_out_of_range (nd, d+1, ra_idx(d)+1, dv(d)); + + return elem (ra_idx); +} + +template <class T> +typename Array<T>::crefT +Array<T>::checkelem (octave_idx_type n) const +{ + if (n < 0 || n >= slice_len) + gripe_index_out_of_range (1, 1, n+1, slice_len); + + return elem (n); } template <class T> -T& -Array<T>::range_error (const char *fcn, const Array<octave_idx_type>& ra_idx) const +typename Array<T>::crefT +Array<T>::checkelem (octave_idx_type i, octave_idx_type j) const +{ + if (i < 0 || i >= dim1 ()) + gripe_index_out_of_range (2, 1, i+1, dim1 ()); + if (j < 0 || j >= dimensions.numel (1)) + gripe_index_out_of_range (2, 2, j+1, dimensions.numel (1)); + + return elem (i, j); +} + +template <class T> +typename Array<T>::crefT +Array<T>::checkelem (octave_idx_type i, octave_idx_type j, octave_idx_type k) const { - std::ostringstream buf; - - buf << fcn << " ("; - - octave_idx_type n = ra_idx.length (); - - if (n > 0) - buf << ra_idx(0); - - for (octave_idx_type i = 1; i < n; i++) - buf << ", " << ra_idx(i); - - buf << "): range error"; - - std::string buf_str = buf.str (); - - (*current_liboctave_error_handler) (buf_str.c_str ()); - - static T foo; - return foo; + if (i < 0 || i >= dim1 ()) + gripe_index_out_of_range (3, 1, i+1, dim1 ()); + if (j < 0 || j >= dim2 ()) + gripe_index_out_of_range (3, 2, j+1, dim2 ()); + if (k < 0 || k >= dimensions.numel (2)) + gripe_index_out_of_range (3, 3, k+1, dimensions.numel (2)); + + return elem (i, j, k); } +template <class T> +typename Array<T>::crefT +Array<T>::checkelem (const Array<octave_idx_type>& ra_idx) const +{ + int nd = ra_idx.length (); + const dim_vector dv = dimensions.redim (nd); + for (int d = 0; d < nd; d++) + if (ra_idx(d) < 0 || ra_idx(d) >= dv(d)) + gripe_index_out_of_range (nd, d+1, ra_idx(d)+1, dv(d)); + + return elem (ra_idx); +} template <class T> Array<T> @@ -267,8 +309,8 @@ { octave_idx_type r = dimensions(0); #ifdef BOUNDS_CHECKING - if (k < 0 || k * r >= numel ()) - range_error ("column", k); + if (k < 0 || k > dimensions.numel (1)) + gripe_index_out_of_range (2, 2, k+1, dimensions.numel (1)); #endif return Array<T> (*this, dim_vector (r, 1), k*r, k*r + r); @@ -280,8 +322,8 @@ { octave_idx_type r = dimensions(0), c = dimensions (1), p = r*c; #ifdef BOUNDS_CHECKING - if (k < 0 || k * p >= numel ()) - range_error ("page", k); + if (k < 0 || k > dimensions.numel (2)) + gripe_index_out_of_range (3, 3, k+1, dimensions.numel (2)); #endif return Array<T> (*this, dim_vector (r, c), k*p, k*p + p); @@ -300,8 +342,10 @@ Array<T>::linear_slice (octave_idx_type lo, octave_idx_type up) const { #ifdef BOUNDS_CHECKING - if (lo < 0 || up > numel ()) - range_error ("linear_slice", lo, up); + if (lo < 0) + gripe_index_out_of_range (1, 1, lo+1, numel ()); + if (up > numel ()) + gripe_index_out_of_range (1, 1, up, numel ()); #endif if (up < lo) up = lo; return Array<T> (*this, dim_vector (up - lo, 1), lo, up); @@ -688,12 +732,6 @@ }; -static void gripe_index_out_of_range (void) -{ - (*current_liboctave_error_handler) - ("A(I): Index exceeds matrix dimension."); -} - template <class T> Array<T> Array<T>::index (const idx_vector& i) const @@ -706,12 +744,11 @@ // A(:) produces a shallow copy as a column vector. retval = Array<T> (*this, dim_vector (n, 1)); } - else if (i.extent (n) != n) - { - gripe_index_out_of_range (); - } else { + if (i.extent (n) != n) + gripe_index_out_of_range (1, 1, i.extent (n), n); // throws + // FIXME -- this is the only place where orig_dimensions are used. dim_vector rd = i.orig_dimensions (); octave_idx_type il = i.length (n); @@ -773,12 +810,13 @@ // A(:,:) produces a shallow copy. retval = Array<T> (*this, dv); } - else if (i.extent (r) != r || j.extent (c) != c) - { - gripe_index_out_of_range (); - } else { + if (i.extent (r) != r) + gripe_index_out_of_range (2, 1, i.extent (r), r); // throws + if (j.extent (c) != c) + gripe_index_out_of_range (2, 2, i.extent (c), c); // throws + octave_idx_type n = numel (), il = i.length (r), jl = j.length (c); idx_vector ii (i); @@ -831,24 +869,17 @@ dim_vector dv = dimensions.redim (ial); // Check for out of bounds conditions. - bool all_colons = true, mismatch = false; + bool all_colons = true; for (int i = 0; i < ial; i++) { if (ia(i).extent (dv(i)) != dv(i)) - { - mismatch = true; - break; - } - else - all_colons = all_colons && ia(i).is_colon (); + gripe_index_out_of_range (ial, i+1, ia(i).extent (dv(i)), dv(i)); // throws + + all_colons = all_colons && ia(i).is_colon (); } - if (mismatch) - { - gripe_index_out_of_range (); - } - else if (all_colons) + if (all_colons) { // A(:,:,...,:) produces a shallow copy. dv.chop_trailing_singletons (); @@ -1387,12 +1418,11 @@ { *this = Array<T> (); } - else if (i.extent (n) != n) - { - gripe_index_out_of_range (); - } else if (i.length (n) != 0) { + if (i.extent (n) != n) + gripe_del_index_out_of_range (true, i.extent (n), n); + octave_idx_type l, u; bool col_vec = ndims () == 2 && columns () == 1 && rows () != 1; if (i.is_scalar () && i(0) == n-1) @@ -1435,12 +1465,11 @@ { *this = Array<T> (); } - else if (i.extent (n) != n) - { - gripe_index_out_of_range (); - } else if (i.length (n) != 0) { + if (i.extent (n) != n) + gripe_del_index_out_of_range (false, i.extent (n), n); + octave_idx_type l, u; if (i.is_cont_range (n, l, u))
--- a/liboctave/Array.h +++ b/liboctave/Array.h @@ -331,13 +331,10 @@ void chop_trailing_singletons (void) GCC_ATTR_DEPRECATED { dimensions.chop_trailing_singletons (); } + octave_idx_type compute_index (octave_idx_type i, octave_idx_type j) const; + octave_idx_type compute_index (octave_idx_type i, octave_idx_type j, octave_idx_type k) const; octave_idx_type compute_index (const Array<octave_idx_type>& ra_idx) const; - T& range_error (const char *fcn, octave_idx_type n) const; - T& range_error (const char *fcn, octave_idx_type i, octave_idx_type j) const; - T& range_error (const char *fcn, octave_idx_type i, octave_idx_type j, octave_idx_type k) const; - T& range_error (const char *fcn, const Array<octave_idx_type>& ra_idx) const; - // No checking, even for multiple references, ever. T& xelem (octave_idx_type n) { return slice_data [n]; } @@ -361,42 +358,10 @@ // unnecessarily force a copy, but that is not so easy, and I see no // clean way to do it. - T& checkelem (octave_idx_type n) - { - if (n < 0 || n >= slice_len) - return range_error ("T& Array<T>::checkelem", n); - else - { - make_unique (); - return xelem (n); - } - } - - T& checkelem (octave_idx_type i, octave_idx_type j) - { - if (i < 0 || j < 0 || i >= dim1 () || j >= dim2 ()) - return range_error ("T& Array<T>::checkelem", i, j); - else - return elem (dim1()*j+i); - } - - T& checkelem (octave_idx_type i, octave_idx_type j, octave_idx_type k) - { - if (i < 0 || j < 0 || k < 0 || i >= dim1 () || j >= dim2 () || k >= dim3 ()) - return range_error ("T& Array<T>::checkelem", i, j, k); - else - return elem (i, dim2()*k+j); - } - - T& checkelem (const Array<octave_idx_type>& ra_idx) - { - octave_idx_type i = compute_index (ra_idx); - - if (i < 0) - return range_error ("T& Array<T>::checkelem", ra_idx); - else - return elem (i); - } + T& checkelem (octave_idx_type n); + T& checkelem (octave_idx_type i, octave_idx_type j); + T& checkelem (octave_idx_type i, octave_idx_type j, octave_idx_type k); + T& checkelem (const Array<octave_idx_type>& ra_idx); T& elem (octave_idx_type n) { @@ -423,48 +388,19 @@ T& operator () (const Array<octave_idx_type>& ra_idx) { return elem (ra_idx); } #endif - crefT checkelem (octave_idx_type n) const - { - if (n < 0 || n >= slice_len) - return range_error ("T Array<T>::checkelem", n); - else - return xelem (n); - } - - crefT checkelem (octave_idx_type i, octave_idx_type j) const - { - if (i < 0 || j < 0 || i >= dim1 () || j >= dim2 ()) - return range_error ("T Array<T>::checkelem", i, j); - else - return elem (dim1()*j+i); - } - - crefT checkelem (octave_idx_type i, octave_idx_type j, octave_idx_type k) const - { - if (i < 0 || j < 0 || k < 0 || i >= dim1 () || j >= dim2 () || k >= dim3 ()) - return range_error ("T Array<T>::checkelem", i, j, k); - else - return Array<T>::elem (i, Array<T>::dim1()*k+j); - } - - crefT checkelem (const Array<octave_idx_type>& ra_idx) const - { - octave_idx_type i = compute_index (ra_idx); - - if (i < 0) - return range_error ("T Array<T>::checkelem", ra_idx); - else - return Array<T>::elem (i); - } + crefT checkelem (octave_idx_type n) const; + crefT checkelem (octave_idx_type i, octave_idx_type j) const; + crefT checkelem (octave_idx_type i, octave_idx_type j, octave_idx_type k) const; + crefT checkelem (const Array<octave_idx_type>& ra_idx) const; crefT elem (octave_idx_type n) const { return xelem (n); } - crefT elem (octave_idx_type i, octave_idx_type j) const { return elem (dim1()*j+i); } + crefT elem (octave_idx_type i, octave_idx_type j) const { return xelem (i, j); } - crefT elem (octave_idx_type i, octave_idx_type j, octave_idx_type k) const { return elem (i, dim2()*k+j); } + crefT elem (octave_idx_type i, octave_idx_type j, octave_idx_type k) const { return xelem (i, j, k); } crefT elem (const Array<octave_idx_type>& ra_idx) const - { return Array<T>::elem (compute_index (ra_idx)); } + { return Array<T>::xelem (compute_index (ra_idx)); } #if defined (BOUNDS_CHECKING) crefT operator () (octave_idx_type n) const { return checkelem (n); }
--- a/liboctave/ChangeLog +++ b/liboctave/ChangeLog @@ -1,3 +1,12 @@ +2010-02-27 Jaroslav Hajek <highegg@gmail.com> + + * Array-util.cc (gripe_index_out_of_range): New function. + * Array.cc (Array<T>::range_error): Remove. + (Array<T>::checkelem): Rewrite. + (Array<T>::index, Array<T>::delete_elements): Simplify. + * DiagArray2.cc (DiagArray2::checkelem): Use gripe_index_out_of_range. + * Range.cc (Range::checkelem, Range::index): Ditto. + 2010-02-26 Jaroslav Hajek <highegg@gmail.com> * mx-inlines.cc (OP_DUP_FCN): Remove.
--- a/liboctave/DiagArray2.cc +++ b/liboctave/DiagArray2.cc @@ -85,8 +85,10 @@ T DiagArray2<T>::checkelem (octave_idx_type r, octave_idx_type c) const { - if (r < 0 || c < 0 || r >= dim1 () || c >= dim2 ()) - (*current_liboctave_error_handler) ("range error in DiagArray2"); + if (r < 0 || r >= dim1 ()) + gripe_index_out_of_range (2, 1, r+1, dim1 ()); + if (c < 0 || c >= dim2 ()) + gripe_index_out_of_range (2, 2, c+1, dim2 ()); return elem (r, c); }
--- a/liboctave/Range.cc +++ b/liboctave/Range.cc @@ -35,6 +35,7 @@ #include "lo-mappers.h" #include "lo-math.h" #include "lo-utils.h" +#include "Array-util.h" Range::Range (double b, double i, octave_idx_type n) : rng_base (b), rng_limit (b + n * i), rng_inc (i), @@ -85,7 +86,7 @@ Range::checkelem (octave_idx_type i) const { if (i < 0 || i >= rng_nelem) - (*current_liboctave_error_handler) ("Range::elem (%d): range error", i); + gripe_index_out_of_range (1, 1, i+1, rng_nelem); return rng_base + rng_inc * i; } @@ -110,13 +111,11 @@ { retval = matrix_value ().reshape (dim_vector (rng_nelem, 1)); } - else if (i.extent (n) != n) - { - (*current_liboctave_error_handler) - ("A(I): Index exceeds matrix dimension."); - } else { + if (i.extent (n) != n) + gripe_index_out_of_range (1, 1, i.extent (n), n); // throws + dim_vector rd = i.orig_dimensions (); octave_idx_type il = i.length (n);
--- a/liboctave/dim-vector.h +++ b/liboctave/dim-vector.h @@ -157,14 +157,18 @@ octave_idx_type& elem (int i) { +#ifdef BOUNDS_CHECKING assert (i >= 0 && i < ndims ()); +#endif make_unique (); return rep[i]; } octave_idx_type elem (int i) const { +#ifdef BOUNDS_CHECKING assert (i >= 0 && i < ndims ()); +#endif return rep[i]; }
--- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,8 @@ +2010-02-27 Jaroslav Hajek <highegg@gmail.com> + + * ov-base-mat.cc (do_index_op): Use checkelem for scalar indices. + Simplify. + 2010-02-26 Jaroslav Hajek <highegg@gmail.com> * src/xpow.cc: Update.
--- a/src/ov-base-mat.cc +++ b/src/ov-base-mat.cc @@ -136,6 +136,7 @@ octave_idx_type n_idx = idx.length (); int nd = matrix.ndims (); + const MT& cmatrix = matrix; switch (n_idx) { @@ -150,8 +151,8 @@ if (! error_state) { // optimize single scalar index. - if (i.is_scalar () && i(0) < matrix.numel ()) - retval = const_cast<const MT&> (matrix)(i(0)); + if (! resize_ok && i.is_scalar ()) + retval = cmatrix.checkelem (i(0)); else retval = MT (matrix.index (i, resize_ok)); } @@ -169,9 +170,8 @@ if (! error_state) { // optimize two scalar indices. - if (i.is_scalar () && j.is_scalar () && nd == 2 - && i(0) < matrix.rows () && j(0) < matrix.columns ()) - retval = const_cast<const MT&> (matrix)(i(0), j(0)); + if (! resize_ok && i.is_scalar () && j.is_scalar ()) + retval = cmatrix.checkelem (i(0), j(0)); else retval = MT (matrix.index (i, j, resize_ok)); } @@ -182,7 +182,7 @@ default: { Array<idx_vector> idx_vec (n_idx, 1); - bool scalar_opt = n_idx == nd; + bool scalar_opt = n_idx == nd && ! resize_ok; const dim_vector dv = matrix.dims (); for (octave_idx_type i = 0; i < n_idx; i++) @@ -192,24 +192,13 @@ if (error_state) break; - scalar_opt = (scalar_opt && idx_vec(i).is_scalar () - && idx_vec(i)(0) < dv(0)); + scalar_opt = (scalar_opt && idx_vec(i).is_scalar ()); } if (! error_state) { if (scalar_opt) - { - // optimize all scalar indices. Don't construct an index array, - // but rather calc a scalar index directly. - octave_idx_type k = 1, j = 0; - for (octave_idx_type i = 0; i < n_idx; i++) - { - j += idx_vec(i)(0) * k; - k *= dv (i); - } - retval = const_cast<const MT&> (matrix)(j); - } + retval = cmatrix.checkelem (conv_to_int_array (idx_vec)); else retval = MT (matrix.index (idx_vec, resize_ok)); }