diff liboctave/util/lo-array-gripes.cc @ 20574:dd6345fd8a97

use exceptions for better invalid index error reporting (bug #45957) * lo-array-gripes.h, lo-array-gripes.cc (index_exception): New base class for indexing errors. (invalid_index, out_of_range): New classes. (gripe_index_out_of_range): New overloaded function. (gripe_invalid_index): New overloaded functions. Delete version with no arguments. (gripe_invalid_assignment_size, gripe_assignment_dimension_mismatch): Delete. Change uses of gripe functions as needed. * Cell.cc (Cell::index, Cell::assign, Cell::delete_elements): Use exceptions to collect error info about and handle indexing errors. * data.cc (Fnth_element, do_accumarray_sum, F__accumarray_sum__, do_accumarray_minmax, do_accumarray_minmax_fun, F__accumdim_sum__): Likewise. * oct-map.cc (octave_map::index, octave_map::assign, octave_map::delete_elements): Likewise. * sparse.cc (Fsparse): Likewise. * sub2ind.cc (Fsub2ind, Find2sub): Likewise. New tests. * utils.cc (dims_to_numel): Likewise. * ov-base-diag.cc (octave_base_diag<DMT, MT>::do_index_op, octave_base_diag<DMT, MT>::subsasgn): Likewise. * ov-base-mat.cc (octave_base_matrix<MT>::subsref, octave_base_matrix<MT>::assign): Likewise. * ov-base-sparse.cc (octave_base_sparse<T>::do_index_op, octave_base_sparse<T>::assign, octave_base_sparse<MT>::delete_elements): Likewise. * ov-classdef.cc (cdef_object_array::subsref, cdef_object_array::subsasgn): Likewise. * ov-java.cc (make_java_index): Likewise. * ov-perm.cc (octave_perm_matrix::do_index_op): Likewise. * ov-range.cc (octave_range::do_index_op): Likewise. * ov-re-diag.cc (octave_diag_matrix::do_index_op): Likewise. * ov-str-mat.cc (octave_char_matrix_str::do_index_op_internal): Likewise. * pt-assign.cc (tree_simple_assignment::rvalue1): Likewise. * pt-idx.cc (tree_index_expression::rvalue, tree_index_expression::lvalue): Likewise. * Array-util.cc (sub2ind): Likewise. * toplev.cc (main_loop): Also catch unhandled index_exception exceptions. * ov-base.cc (octave_base_value::index_vector): Improve error message. * ov-re-sparse.cc (octave_sparse_matrix::index_vector): Likewise. * ov-complex.cc (complex_index): New class. (gripe_complex_index): New function. (octave_complex::index_vector): Use it. * pt-id.h, pt-id.cc (tree_identifier::is_variable, tree_black_hole::is_variable): Now const. * pt-idx.cc (final_index_error): New static function. (tree_index_expression::rvalue, tree_index_expression::lvalue): Use it. * index.tst: New tests.
author Lachlan Andrew <lachlanbis@gmail.com>
date Fri, 02 Oct 2015 15:07:37 -0400
parents 4197fc428c7d
children e54ecb33727e
line wrap: on
line diff
--- a/liboctave/util/lo-array-gripes.cc	Fri Oct 02 12:25:39 2015 -0400
+++ b/liboctave/util/lo-array-gripes.cc	Fri Oct 02 15:07:37 2015 -0400
@@ -25,6 +25,7 @@
 #include <config.h>
 #endif
 
+#include <string.h>
 #include "lo-array-gripes.h"
 #include "lo-error.h"
 
@@ -90,34 +91,6 @@
 }
 
 void
-gripe_index_out_of_range (int nd, int dim, octave_idx_type idx,
-                          octave_idx_type ext)
-{
-  const char *err_id = error_id_index_out_of_bounds;
-
-  switch (nd)
-    {
-    case 1:
-      (*current_liboctave_error_with_id_handler)
-        (err_id, "A(I): index out of bounds; value %d out of bound %d",
-         idx, ext);
-      break;
-
-    case 2:
-      (*current_liboctave_error_with_id_handler)
-        (err_id, "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_with_id_handler)
-        (err_id, "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)
 {
@@ -128,25 +101,234 @@
      is1d ? "I" : "..,I,..", idx, ext);
 }
 
-void
-gripe_invalid_index (void)
+
+
+// Common procedures of base class index_exception, thrown whenever an
+// object is indexed incorrectly, such as by an index that is out of
+// range, negative, fractional, complex, or of a non-numeric type.
+
+const char *
+index_exception::err (void) throw ()
 {
-  const char *err_id = error_id_invalid_index;
+  msg = access () + "; " + explain ();
+  return msg.c_str ();
+}
+
+// Show what was illegally accessed, e.g.,  "A(-1,_)", "A(0+1i)", "A(_,3)"
+// Show how many indices come before/after the offending one,
+// e.g., (<error>), (<error>,_), or (_,<error>,...[x5]...)
+
+std::string
+index_exception:: access (void) const
+{
+  // FIXME: don't use a fixed size buffer!
+
+  int buf_len = 300;
+
+  char output [buf_len];
+  char pre [buf_len];
+  char post [buf_len];
+
+  // dim == 0 if position not yet given, or
+  // <static_cast unsigned int>(-1) if explicitly shown to be unknown
+  // both are caught by this condition
 
-  (*current_liboctave_error_with_id_handler)
-#ifdef USE_64_BIT_IDX_T
-    (err_id, "subscript indices must be either positive integers less than 2^63 or logicals");
-#else
-    (err_id, "subscript indices must be either positive integers less than 2^31 or logicals");
-#endif
+  if (static_cast <unsigned int> (dim-1) > 100000)
+    {
+      // No parentheses are given if the dimension is not known.
+      pre[0] = post[0] = '\0';
+    }
+  else
+    {
+      if (dim < 5)
+        {
+          pre[0] = '(';
+          octave_idx_type i;
+
+          for (i = 1; i < dim; i++)
+            {
+              pre[2*i-1] = '_';
+              pre[2*i]   = ',';
+            }
+
+          pre[2*i-1] = '\0';    // i == min (1, dim)
+        }
+      else
+        {
+          sprintf (pre, "(...[x%d]...", dim-1);
+        }
+
+      if (static_cast <unsigned int> (nd-dim) < 5)
+        {
+          for (octave_idx_type i = 0; i < nd-dim; i++)
+            {
+              post[2*i]   = ',';
+              post[2*i+1] = '_';
+            }
+
+          if (nd >= dim)
+            {
+              post[2*(nd-dim)] = ')';
+              post[2*(nd-dim)+1] = '\0';
+            }
+        }
+      else
+        sprintf (post, "...[x%d]...)", nd-dim);
+    }
+
+  const char *v;
+
+  if (var[0] == '\0' || var == "<unknown>")
+    v = "index ";
+  else
+    v = var.c_str ();
+
+  snprintf (output, buf_len, "%s%s%s%s", v, pre, idx(), post);
+
+  return output;
 }
 
-// FIXME: the following is a common error message to resize,
-// regardless of whether it's called from assign or elsewhere.  It
-// seems OK to me, but eventually the gripe can be specialized.
-// Anyway, propagating various error messages into procedure is, IMHO,
-// a nonsense.  If anything, we should change error handling here (and
-// throughout liboctave) to allow custom handling of errors
+class invalid_index : public index_exception
+{
+public:
+
+  invalid_index (const char *value, octave_idx_type ndim,
+                 octave_idx_type dimen)
+    : index_exception (value, ndim, dimen)
+  { }
+
+  const char* explain (void) const
+  {
+#ifdef USE_64_BIT_IDX_T
+    return "subscripts must be either integers 1 to (2^63)-1 or logicals";
+#else
+    return "subscripts must be either integers 1 to (2^31)-1 or logicals";
+#endif
+  }
+
+  // ID of error to throw
+  const char* id (void) const
+  {
+    return error_id_invalid_index;
+  }
+};
+
+// Complain of an index that is: negative, fractional, or too big.
+
+void
+gripe_invalid_index (const char *idx, octave_idx_type nd,
+                     octave_idx_type dim, const char * /* var */)
+{
+    invalid_index e (idx, nd, dim);
+
+    throw e;
+}
+
+void
+gripe_invalid_index (octave_idx_type n, octave_idx_type nd,
+                     octave_idx_type dim, const char *var)
+{
+  // FIXME: don't use a fixed size buffer!
+  char buf [100];
+
+  sprintf (buf, "%d", n+1);
+
+  gripe_invalid_index (buf, nd, dim, var);
+}
+
+void
+gripe_invalid_index (double n, octave_idx_type nd, octave_idx_type dim,
+                     const char *var)
+{
+  // FIXME: don't use a fixed size buffer!
+  char buf [100];
+
+  sprintf (buf, "%g", n+1);
+
+  gripe_invalid_index (buf, nd, dim, var);
+}
+
+
+// Gripe and exception for read access beyond the bounds of an array.
+
+class out_of_range : public index_exception
+{
+public:
+
+  out_of_range (const char *value, octave_idx_type nd_in,octave_idx_type dim_in)
+        : index_exception (value, nd_in, dim_in), extent(0)
+    { }
+
+  const char* explain (void) const
+  {
+    static std::string expl;    // should probably be member variable, but
+                                // then explain() can't be const.
+
+    if (nd >= size.length ())   // if not an index slice
+      {
+        if (var != "")
+          expl = "but " + var + " has size ";
+        else
+          expl = "but object has size ";
+
+        expl = expl + size.str ('x');
+      }
+    else
+      {
+        // FIXME: don't use a fixed size buffer!
+        char buf [100];
+        sprintf (buf, "%d", extent);
+        expl = "out of bound " + std::string (buf);
+      }
+
+    return expl.c_str ();
+  }
+
+  // ID of error to throw.
+  const char* id (void) const
+  {
+    return (error_id_index_out_of_bounds);
+  }
+
+  void set_size (const dim_vector& size_in) { size = size_in; }
+
+  void set_extent (octave_idx_type ext) { extent = ext; }
+
+private:
+
+  dim_vector  size;         // dimension of object being accessed
+
+  octave_idx_type extent;   // length of dimension being accessed
+};
+
+// Complain of an index that is out of range, but we don't know matrix size
+void
+gripe_index_out_of_range (int nd, int dim, octave_idx_type idx,
+                          octave_idx_type ext)
+{
+    char buf [100];
+    sprintf (buf, "%d", idx);
+    out_of_range e (buf, nd, dim);
+
+    e.set_extent (ext);
+    dim_vector d (1,1,1,1,1,1,1);   // make  explain()  give extent not size
+    e.set_size (d);
+    throw e;
+}
+
+// Complain of an index that is out of range
+void
+gripe_index_out_of_range (int nd, int dim, octave_idx_type idx,
+                          octave_idx_type ext, const dim_vector& d)
+{
+    char buf [100];
+    sprintf (buf, "%d", idx);
+    out_of_range e (buf, nd, dim);
+
+    e.set_extent (ext);
+    e.set_size (d);
+    throw e;
+}
 
 void
 gripe_invalid_resize (void)
@@ -157,20 +339,6 @@
 }
 
 void
-gripe_invalid_assignment_size (void)
-{
-  (*current_liboctave_error_handler)
-    ("A(I) = X: X must have the same size as I");
-}
-
-void
-gripe_assignment_dimension_mismatch (void)
-{
-  (*current_liboctave_error_handler)
-    ("A(I,J,...) = X: dimensions mismatch");
-}
-
-void
 gripe_singular_matrix (double rcond)
 {
   if (rcond == 0.0)
@@ -186,3 +354,5 @@
          "matrix singular to machine precision, rcond = %g", rcond);
     }
 }
+
+/* Tests in test/index.tst */