diff libinterp/octave-value/ov-base-mat.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 b2100e1659ac
children 1a0a433c8263
line wrap: on
line diff
--- a/libinterp/octave-value/ov-base-mat.cc	Fri Oct 02 12:25:39 2015 -0400
+++ b/libinterp/octave-value/ov-base-mat.cc	Fri Oct 02 15:07:37 2015 -0400
@@ -36,7 +36,7 @@
 #include "ov-base-mat.h"
 #include "ov-base-scalar.h"
 #include "pr-output.h"
-
+ 
 template <class MT>
 octave_value
 octave_base_matrix<MT>::subsref (const std::string& type,
@@ -140,72 +140,88 @@
   int nd = matrix.ndims ();
   const MT& cmatrix = matrix;
 
-  switch (n_idx)
-    {
-    case 0:
-      retval = matrix;
-      break;
+  // If we catch an indexing error in index_vector, we flag an error in
+  // index k.  Ensure it is the right value befor each idx_vector call.
+  // Same variable as used in the for loop in the default case.
+
+  octave_idx_type k = 0;
 
-    case 1:
-      {
-        idx_vector i = idx (0).index_vector ();
+  try
+    {
+      switch (n_idx)
+        {
+        case 0:
+          retval = matrix;
+          break;
 
-        if (! error_state)
+        case 1:
           {
-            // optimize single scalar index.
-            if (! resize_ok && i.is_scalar ())
-              retval = cmatrix.checkelem (i(0));
-            else
-              retval = MT (matrix.index (i, resize_ok));
+            idx_vector i = idx (0).index_vector ();
+
+            if (! error_state)
+              {
+                // optimize single scalar index.
+                if (! resize_ok && i.is_scalar ())
+                  retval = cmatrix.checkelem (i(0));
+                else
+                  retval = MT (matrix.index (i, resize_ok));
+              }
           }
-      }
-      break;
+          break;
 
-    case 2:
-      {
-        idx_vector i = idx (0).index_vector ();
-
-        if (! error_state)
+        case 2:
           {
-            idx_vector j = idx (1).index_vector ();
+            idx_vector i = idx (0).index_vector ();
 
             if (! error_state)
               {
-                // optimize two scalar indices.
-                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));
+                k=1;
+                idx_vector j = idx (1).index_vector ();
+
+                if (! error_state)
+                  {
+                    // optimize two scalar indices.
+                    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));
+                  }
               }
           }
-      }
-      break;
+          break;
 
-    default:
-      {
-        Array<idx_vector> idx_vec (dim_vector (n_idx, 1));
-        bool scalar_opt = n_idx == nd && ! resize_ok;
-        const dim_vector dv = matrix.dims ();
+        default:
+          {
+            Array<idx_vector> idx_vec (dim_vector (n_idx, 1));
+            bool scalar_opt = n_idx == nd && ! resize_ok;
+            const dim_vector dv = matrix.dims ();
 
-        for (octave_idx_type i = 0; i < n_idx; i++)
-          {
-            idx_vec(i) = idx(i).index_vector ();
+            for (k = 0; k < n_idx; k++)
+              {
+                idx_vec(k) = idx(k).index_vector ();
+
+                if (error_state)
+                  break;
 
-            if (error_state)
-              break;
-
-            scalar_opt = (scalar_opt && idx_vec(i).is_scalar ());
-          }
+                scalar_opt = (scalar_opt && idx_vec(k).is_scalar ());
+              }
 
-        if (! error_state)
-          {
-            if (scalar_opt)
-              retval = cmatrix.checkelem (conv_to_int_array (idx_vec));
-            else
-              retval = MT (matrix.index (idx_vec, resize_ok));
+            if (! error_state)
+              {
+                if (scalar_opt)
+                  retval = cmatrix.checkelem (conv_to_int_array (idx_vec));
+                else
+                  retval = MT (matrix.index (idx_vec, resize_ok));
+              }
           }
-      }
-      break;
+          break;
+        }
+    }
+  catch (index_exception& e)
+    {
+      // Rethrow to allow more info to be reported later.
+      e.set_pos_if_unset (n_idx, k+1);
+      throw;
     }
 
   return retval;
@@ -217,51 +233,65 @@
 {
   octave_idx_type n_idx = idx.length ();
 
-  switch (n_idx)
-    {
-    case 0:
-      panic_impossible ();
-      break;
+  // If we catch an indexing error in index_vector, we flag an error in
+  // index k.  Ensure it is the right value befor each idx_vector call.
+  // Same variable as used in the for loop in the default case.
 
-    case 1:
-      {
-        idx_vector i = idx (0).index_vector ();
+  octave_idx_type k = 0;
 
-        if (! error_state)
-          matrix.assign (i, rhs);
-      }
-      break;
+  try
+    {
+      switch (n_idx)
+        {
+          case 0:
+            panic_impossible ();
+            break;
 
-    case 2:
-      {
-        idx_vector i = idx (0).index_vector ();
+          case 1:
+            {
+                  idx_vector i = idx (0).index_vector ();
 
-        if (! error_state)
-          {
-            idx_vector j = idx (1).index_vector ();
+                  if (! error_state)
+                    matrix.assign (i, rhs);
+            }
+            break;
+
+          case 2:
+            {
+              idx_vector i = idx (0).index_vector ();
 
-            if (! error_state)
-              matrix.assign (i, j, rhs);
-          }
-      }
-      break;
+              if (! error_state)
+                {
+                  k = 1;
+                  idx_vector j = idx (1).index_vector ();
 
-    default:
-      {
-        Array<idx_vector> idx_vec (dim_vector (n_idx, 1));
+                  if (! error_state)
+                    matrix.assign (i, j, rhs);
+                }
+            }
+            break;
+
+          default:
+            {
+              Array<idx_vector> idx_vec (dim_vector (n_idx, 1));
 
-        for (octave_idx_type i = 0; i < n_idx; i++)
-          {
-            idx_vec(i) = idx(i).index_vector ();
+              for (k = 0; k < n_idx; k++)
+                {
+                  idx_vec(k) = idx(k).index_vector ();
+
+                  if (error_state)
+                    break;
+                }
 
-            if (error_state)
-              break;
-          }
-
-        if (! error_state)
-          matrix.assign (idx_vec, rhs);
-      }
-      break;
+              if (! error_state)
+                matrix.assign (idx_vec, rhs);
+            }
+            break;
+        }
+    }
+  catch (index_exception& e)
+    {
+      gripe_invalid_index (e.idx(), n_idx, k+1);
     }
 
   // Clear cache.
@@ -288,86 +318,100 @@
 
   MT mrhs (dim_vector (1, 1), rhs);
 
-  switch (n_idx)
-    {
-    case 0:
-      panic_impossible ();
-      break;
+  // If we catch an indexing error in index_vector, we flag an error in
+  // index k.  Ensure it is the right value befor each idx_vector call.
+  // Same variable as used in the for loop in the default case.
+
+  octave_idx_type k = 0;
 
-    case 1:
-      {
-        idx_vector i = idx (0).index_vector ();
+  try
+    {
+      switch (n_idx)
+        {
+        case 0:
+          panic_impossible ();
+          break;
 
-        if (! error_state)
+        case 1:
           {
-            // optimize single scalar index.
-            if (i.is_scalar () && i(0) < matrix.numel ())
-              matrix(i(0)) = rhs;
-            else
-              matrix.assign (i, mrhs);
+            idx_vector i = idx (0).index_vector ();
+
+            if (! error_state)
+              {
+                // optimize single scalar index.
+                if (i.is_scalar () && i(0) < matrix.numel ())
+                  matrix(i(0)) = rhs;
+                else
+                  matrix.assign (i, mrhs);
+              }
           }
-      }
-      break;
+          break;
 
-    case 2:
-      {
-        idx_vector i = idx (0).index_vector ();
-
-        if (! error_state)
+        case 2:
           {
-            idx_vector j = idx (1).index_vector ();
+            idx_vector i = idx (0).index_vector ();
 
             if (! error_state)
               {
-                // optimize two scalar indices.
-                if (i.is_scalar () && j.is_scalar () && nd == 2
-                    && i(0) < matrix.rows () && j(0) < matrix.columns ())
-                  matrix(i(0), j(0)) = rhs;
-                else
-                  matrix.assign (i, j, mrhs);
+                k = 1;
+                idx_vector j = idx (1).index_vector ();
+
+                if (! error_state)
+                  {
+                    // optimize two scalar indices.
+                    if (i.is_scalar () && j.is_scalar () && nd == 2
+                        && i(0) < matrix.rows () && j(0) < matrix.columns ())
+                      matrix(i(0), j(0)) = rhs;
+                    else
+                      matrix.assign (i, j, mrhs);
+                  }
               }
           }
-      }
-      break;
+          break;
 
-    default:
-      {
-        Array<idx_vector> idx_vec (dim_vector (n_idx, 1));
-        bool scalar_opt = n_idx == nd;
-        const dim_vector dv = matrix.dims ().redim (n_idx);
+        default:
+          {
+            Array<idx_vector> idx_vec (dim_vector (n_idx, 1));
+            bool scalar_opt = n_idx == nd;
+            const dim_vector dv = matrix.dims ().redim (n_idx);
 
-        for (octave_idx_type i = 0; i < n_idx; i++)
-          {
-            idx_vec(i) = idx(i).index_vector ();
+            for (k = 0; k < n_idx; k++)
+              {
+                idx_vec(k) = idx(k).index_vector ();
 
-            if (error_state)
-              break;
+                if (error_state)
+                  break;
 
-            scalar_opt = (scalar_opt && idx_vec(i).is_scalar ()
-                          && idx_vec(i)(0) < dv(i));
-          }
+                scalar_opt = (scalar_opt && idx_vec(k).is_scalar ()
+                              && idx_vec(k)(0) < dv(k));
+              }
 
-        if (! error_state)
-          {
-            if (scalar_opt)
+            if (! error_state)
               {
-                // optimize all scalar indices. Don't construct an index array,
-                // but rather calc a scalar index directly.
-                octave_idx_type k = 1;
-                octave_idx_type j = 0;
-                for (octave_idx_type i = 0; i < n_idx; i++)
+                if (scalar_opt)
                   {
-                    j += idx_vec(i)(0) * k;
-                    k *= dv(i);
+                    // optimize all scalar indices. Don't construct
+                    // an index array, but rather calc a scalar index directly.
+                    octave_idx_type n = 1;
+                    octave_idx_type j = 0;
+                    for (octave_idx_type i = 0; i < n_idx; i++)
+                      {
+                        j += idx_vec(i)(0) * n;
+                        n *= dv (i);
+                      }
+                    matrix(j) = rhs;
                   }
-                matrix(j) = rhs;
+                else
+                  matrix.assign (idx_vec, mrhs);
               }
-            else
-              matrix.assign (idx_vec, mrhs);
           }
-      }
-      break;
+          break;
+        }
     }
+  catch (const index_exception& e)
+    {
+      gripe_invalid_index (e.idx(), n_idx, k+1);
+     }
 
   // Clear cache.
   clear_cached_info ();