Mercurial > octave
changeset 21990:efce657ceb86
Provide a warning when an array is used in an if/while/until (bug #43098).
* errwarn.{cc,h} (warn_array_as_logical,
reset_first_array_as_logical_since_keyboard): New functions
* error.cc (initialize_default_warning_state):
Disable Octave:array-as-logical by default
* warning_ids.m: Add new warning Octave:array-as-logical.
* input.cc (interactive_input):
Reset first_array_as_logical_since_keyboard.
* ov-base-mat.cc (octave_base_matrix<MT>::is_true):
Call warn_array_as_logical if > 1x1.
* ov-base-diag.cc (octave_base_diag<DMT, MT>::is_true):
Call warn_array_as_logical if > 1x1. Optimise calculation.
* ov-perm.cc (octave_perm_matrix::do_index_op):
Call warn_array_as_logical if > 1x1. Optimise calculation.
* ov-range.cc (octave_range::is_true):
Call warn_array_as_logical if > 1x1. Optimise calculation.
* ov-base-sparse (octave_base_sparse<T>::is_true)
Call warn_array_as_logical if > 1x1. Check for NaN/NA.
* Sparse.cc (Sparse<T>::SparseRep::any_element_is_nan): New function.
* Sparse.h (Sparse<T>::SparseRep::any_element_is_nan,
Sparse<T>::any_element_is_nan): New functions.
* Sparse-b.cc (Sparse<bool>::SparseRep::any_element_is_nan):
New function, specialization of the above.
* if.tst: New built-in self tests.
| author | Lachlan Andrew <lachlanbis@gmail.com> |
|---|---|
| date | Wed, 29 Jun 2016 08:50:09 -0700 |
| parents | 6bce4d23af6b |
| children | 80659e58609f |
| files | libinterp/corefcn/error.cc libinterp/corefcn/errwarn.cc libinterp/corefcn/errwarn.h libinterp/octave-value/ov-base-diag.cc libinterp/octave-value/ov-base-mat.cc libinterp/octave-value/ov-base-sparse.cc libinterp/octave-value/ov-perm.cc libinterp/octave-value/ov-range.cc libinterp/octave-value/ov.cc liboctave/array/Sparse-b.cc liboctave/array/Sparse.cc liboctave/array/Sparse.h scripts/help/warning_ids.m test/if.tst |
| diffstat | 14 files changed, 153 insertions(+), 9 deletions(-) [+] |
line wrap: on
line diff
--- a/libinterp/corefcn/error.cc Tue Jun 28 16:42:15 2016 -0400 +++ b/libinterp/corefcn/error.cc Wed Jun 29 08:50:09 2016 -0700 @@ -1780,6 +1780,7 @@ // Most people will want to have the following disabled. + disable_warning ("Octave:array-as-logical"); disable_warning ("Octave:array-to-scalar"); disable_warning ("Octave:array-to-vector"); disable_warning ("Octave:imag-to-real");
--- a/libinterp/corefcn/errwarn.cc Tue Jun 28 16:42:15 2016 -0400 +++ b/libinterp/corefcn/errwarn.cc Wed Jun 29 08:50:09 2016 -0700 @@ -273,6 +273,23 @@ } void +warn_array_as_logical (const dim_vector& dv) +{ + warning_with_id ("Octave:array-as-logical", + "Using an object of size %s as " + "a boolean value implies all().", + dv.str ().c_str ()); +} + +/* +%!warning <boolean value implies all> +%! warning ("on", "Octave:array-as-logical"); +%! if ([1 1 0]) +%! assert (false); +%! endif +*/ + +void warn_complex_cmp (void) { warning_with_id ("Octave:language-extension",
--- a/libinterp/corefcn/errwarn.h Tue Jun 28 16:42:15 2016 -0400 +++ b/libinterp/corefcn/errwarn.h Wed Jun 29 08:50:09 2016 -0700 @@ -145,6 +145,9 @@ err_wrong_type_arg_for_unary_op (const octave_value& op); OCTINTERP_API extern void +warn_array_as_logical (const dim_vector& dv); + +OCTINTERP_API extern void warn_complex_cmp (void); OCTINTERP_API extern void
--- a/libinterp/octave-value/ov-base-diag.cc Tue Jun 28 16:42:15 2016 -0400 +++ b/libinterp/octave-value/ov-base-diag.cc Wed Jun 29 08:50:09 2016 -0700 @@ -273,11 +273,20 @@ return retval; } +// Return true if this matrix has all true elements (non-zero, not NA/NaN). template <typename DMT, typename MT> bool octave_base_diag<DMT, MT>::is_true (void) const { - return to_dense ().is_true (); + if (dims ().numel () > 1) + { + warn_array_as_logical (dims ()); + // Throw error if any NaN or NA by calling is_true(). + octave_value (matrix.extract_diag ()).is_true (); + return false; // > 1x1 diagonal always has zeros + } + else + return to_dense ().is_true (); // 0x0 or 1x1, handle NaN etc. } // FIXME: This should be achieveable using ::real
--- a/libinterp/octave-value/ov-base-mat.cc Tue Jun 28 16:42:15 2016 -0400 +++ b/libinterp/octave-value/ov-base-mat.cc Wed Jun 29 08:50:09 2016 -0700 @@ -30,6 +30,7 @@ #include "Array-util.h" #include "Cell.h" +#include "errwarn.h" #include "ovl.h" #include "oct-map.h" #include "ov-base.h" @@ -401,6 +402,7 @@ return retval; } +// Return true if this matrix has all true elements (non-zero, not NA/NaN). template <typename MT> bool octave_base_matrix<MT>::is_true (void) const @@ -416,6 +418,9 @@ if (t1.any_element_is_nan ()) err_nan_to_logical_conversion (); + if (nel > 1) + warn_array_as_logical (dv); + boolNDArray t2 = t1.all (); retval = t2(0);
--- a/libinterp/octave-value/ov-base-sparse.cc Tue Jun 28 16:42:15 2016 -0400 +++ b/libinterp/octave-value/ov-base-sparse.cc Wed Jun 29 08:50:09 2016 -0700 @@ -300,13 +300,22 @@ octave_idx_type nel = dv.numel (); octave_idx_type nz = nnz (); - if (nz == nel && nel > 0) + if (nel > 0) { T t1 (matrix.reshape (dim_vector (nel, 1))); - SparseBoolMatrix t2 = t1.all (); + if (t1.any_element_is_nan ()) + err_nan_to_logical_conversion (); + + if (nel > 1) + warn_array_as_logical (dv); - retval = t2(0); + if (nz == nel) + { + SparseBoolMatrix t2 = t1.all (); + + retval = t2(0); + } } return retval;
--- a/libinterp/octave-value/ov-perm.cc Tue Jun 28 16:42:15 2016 -0400 +++ b/libinterp/octave-value/ov-perm.cc Wed Jun 29 08:50:09 2016 -0700 @@ -131,10 +131,18 @@ return retval; } +// Return true if this matrix has all true elements (non-zero, not NaN/NA). +// A permutation cannot have NaN/NA. bool octave_perm_matrix::is_true (void) const { - return to_dense ().is_true (); + if (dims ().numel () > 1) + { + warn_array_as_logical (dims ()); + return false; // > 1x1 permutation always has zeros, and no NaN. + } + else + return dims ().numel (); // 1x1 is [1] == true, 0x0 == false. } double
--- a/libinterp/octave-value/ov-range.cc Tue Jun 28 16:42:15 2016 -0400 +++ b/libinterp/octave-value/ov-range.cc Wed Jun 29 08:50:09 2016 -0700 @@ -264,6 +264,8 @@ return mat.diag (m, n); } +// Return true if this range has all true elements (non-zero, not NaN/NA). +// A range cannot have NaN/NA. bool octave_range::is_true (void) const { @@ -271,10 +273,30 @@ if (! range.is_empty ()) { - // FIXME: this is a potential waste of memory. - Matrix m ((range.matrix_value ().all ()).all ()); + if (dims ().numel () > 1) + warn_array_as_logical (dims ()); + + Range r = range_value (); + double base = r.base (); + double limit = r.limit (); - retval = (m.rows () == 1 && m.columns () == 1 && m (0, 0) != 0.0); + // Can't be zero if we start and finish on the same size of 0 + if (((base > 0 && limit > 0) || (base < 0 && limit < 0)) && numel () > 0) + retval = true; + else + { + /* + // This tells us whether one element is 0, if arithmetic is exact. + double steps_to_zero = base / r.inc (); + + retval = (steps_to_zero != floor (steps_to_zero)); + */ + + // FIXME: this is a waste of memory. + Matrix m ((range.matrix_value ().all ()).all ()); + + retval = ! m.is_empty () && m(0, 0) != 0.0; + } } return retval;
--- a/libinterp/octave-value/ov.cc Tue Jun 28 16:42:15 2016 -0400 +++ b/libinterp/octave-value/ov.cc Wed Jun 29 08:50:09 2016 -0700 @@ -1411,7 +1411,17 @@ // Empty array also means a match. if (tmp.is_defined ()) - retval = tmp.is_true () || tmp.is_empty (); + { + if (tmp.is_empty ()) + retval = true; + else + { + // Reshape into a vector and call all() explicitly, + // to avoid Octave:array-as-logical warning. + tmp = tmp.reshape (dim_vector (tmp.numel (), 1)); + retval = tmp.all ().is_true (); + } + } } return retval;
--- a/liboctave/array/Sparse-b.cc Tue Jun 28 16:42:15 2016 -0400 +++ b/liboctave/array/Sparse-b.cc Wed Jun 29 08:50:09 2016 -0700 @@ -30,6 +30,13 @@ #include "Sparse.h" #include "Sparse.cc" +template <> +bool +Sparse<bool>::SparseRep::any_element_is_nan (void) const +{ + return false; +} + INSTANTIATE_SPARSE (bool, OCTAVE_API); #if 0
--- a/liboctave/array/Sparse.cc Tue Jun 28 16:42:15 2016 -0400 +++ b/liboctave/array/Sparse.cc Wed Jun 29 08:50:09 2016 -0700 @@ -193,6 +193,19 @@ } template <typename T> +bool +Sparse<T>::SparseRep::any_element_is_nan (void) const +{ + octave_idx_type nz = nnz (); + + for (octave_idx_type i = 0; i < nz; i++) + if (octave::math::isnan (d[i])) + return true; + + return false; +} + +template <typename T> Sparse<T>::Sparse (octave_idx_type nr, octave_idx_type nc, T val) : rep (0), dimensions (dim_vector (nr, nc)) {
--- a/liboctave/array/Sparse.h Tue Jun 28 16:42:15 2016 -0400 +++ b/liboctave/array/Sparse.h Wed Jun 29 08:50:09 2016 -0700 @@ -136,6 +136,8 @@ bool indices_ok (void) const; + bool any_element_is_nan (void) const; + private: // No assignment! @@ -692,6 +694,9 @@ { return map<U, U (&) (const T&)> (fcn); } bool indices_ok (void) const { return rep->indices_ok (); } + + bool any_element_is_nan (void) const + { return rep->any_element_is_nan (); } }; template <typename T>
--- a/scripts/help/warning_ids.m Tue Jun 28 16:42:15 2016 -0400 +++ b/scripts/help/warning_ids.m Wed Jun 29 08:50:09 2016 -0700 @@ -23,6 +23,12 @@ ## @item Octave:abbreviated-property-match ## By default, the @code{Octave:abbreviated-property-match} warning is enabled. ## +## @item Octave:array-as-logical +## If the @code{Octave:array-to-scalar} warning is enabled, +## Octave will warn when an array of size greater than 1x1 is used +## as a truth value in an if, while or until statement. +## By default, the @code{Octave:array-as-logical} warning is disabled. +## ## @item Octave:array-to-scalar ## If the @code{Octave:array-to-scalar} warning is enabled, Octave will ## warn when an implicit conversion from an array to a scalar value is
--- a/test/if.tst Tue Jun 28 16:42:15 2016 -0400 +++ b/test/if.tst Wed Jun 29 08:50:09 2016 -0700 @@ -99,3 +99,32 @@ %! endif %! assert (x, 13); +## test "is_true" of different data types +%!error diag (NaN) || 0; +%!test +%! d1 = diag ([]) || 0; +%! d2 = diag (1) || 0; +%! d3 = diag ([1 2]) || 0; +%! assert ([d1 d2 d3], [false true false]); + +%!error sparse (NaN) || 0; +%!error sparse ([1 1 ; 1 NaN]) || 0; +%!test +%! s1 = sparse ([]) || 0; +%! s2 = sparse (1) || 0; +%! s3 = sparse ([1 0 ; 0 2]) || 0; +%! s4 = sparse ([1 1 ; 1 1]) || 0; +%! assert ([s1 s2 s3 s4], [false true false true]); + +%!test +%! r1 = (1:10) || 0; +%! r2 = (-10:-1) || 0; +%! r3 = (-1:1) || 0; +%! assert ([r1 r2 r3], [true true false]); + +%!test +%! c1 = [2i 4i] || 0; +%! c2 = [22 4i] || 0; +%! c3 = i || 0; +%! c4 = complex(0) || 0; +%! assert ([c1 c2 c3 c4], [true true true false]);