changeset 29567:345c42c067cb

eliminate error flag and err_rep in idx_vector class * idx-vector.h, idx-vector.cc (idx_base_rep::err): Delete data member and all uses. (idx_vector::err_rep, idx_vector::chkerr): Delete function and all uses. (err_index_out_of_range): Tag with OCTAVE_NORETURN attribute. (convert_index): Eliminate conv_error argument. Change all uses. (idx_vector::operator bool): Always return true.
author John W. Eaton <jwe@octave.org>
date Wed, 28 Apr 2021 10:23:41 -0400
parents 14124f73703c
children a7cbd0e54e7a
files liboctave/array/idx-vector.cc liboctave/array/idx-vector.h
diffstat 2 files changed, 39 insertions(+), 91 deletions(-) [+]
line wrap: on
line diff
--- a/liboctave/array/idx-vector.cc	Sun Apr 25 17:36:12 2021 +0200
+++ b/liboctave/array/idx-vector.cc	Wed Apr 28 10:23:41 2021 -0400
@@ -42,14 +42,13 @@
 #include "lo-error.h"
 #include "lo-mappers.h"
 
-OCTAVE_NORETURN static
-void
+OCTAVE_NORETURN static void
 err_invalid_range (void)
 {
   (*current_liboctave_error_handler) ("invalid range used as index");
 }
 
-static void
+OCTAVE_NORETURN static void
 err_index_out_of_range (void)
 {
   (*current_liboctave_error_handler)
@@ -63,14 +62,6 @@
   return &ivr;
 }
 
-idx_vector::idx_vector_rep *
-idx_vector::err_rep (void)
-{
-  static idx_vector_rep ivr;
-  ivr.err = true;
-  return &ivr;
-}
-
 Array<octave_idx_type>
 idx_vector::idx_base_rep::as_array (void)
 {
@@ -85,12 +76,8 @@
   : idx_base_rep ()
 {
   if (c != ':')
-    {
-      (*current_liboctave_error_handler)
-        ("internal error: invalid character converted to idx_vector; must be ':'");
-      // FIXME: this is unreachable now.
-      err = true;
-    }
+    (*current_liboctave_error_handler)
+      ("internal error: invalid character converted to idx_vector; must be ':'");
 }
 
 octave_idx_type
@@ -225,10 +212,9 @@
 }
 
 inline octave_idx_type
-convert_index (octave_idx_type i, bool& conv_error,
-               octave_idx_type& ext)
+convert_index (octave_idx_type i, octave_idx_type& ext)
 {
-  if (i <= 0 && ! conv_error)
+  if (i <= 0)
     octave::err_invalid_index (i-1);
 
   if (ext < i)
@@ -238,30 +224,29 @@
 }
 
 inline octave_idx_type
-convert_index (double x, bool& conv_error, octave_idx_type& ext)
+convert_index (double x, octave_idx_type& ext)
 {
   octave_idx_type i = static_cast<octave_idx_type> (x);
 
   if (static_cast<double> (i) != x)
     octave::err_invalid_index (x-1);
 
-  return convert_index (i, conv_error, ext);
+  return convert_index (i, ext);
 }
 
 inline octave_idx_type
-convert_index (float x, bool& conv_error, octave_idx_type& ext)
+convert_index (float x, octave_idx_type& ext)
 {
-  return convert_index (static_cast<double> (x), conv_error, ext);
+  return convert_index (static_cast<double> (x), ext);
 }
 
 template <typename T>
 inline octave_idx_type
-convert_index (octave_int<T> x, bool& conv_error,
-               octave_idx_type& ext)
+convert_index (octave_int<T> x, octave_idx_type& ext)
 {
   octave_idx_type i = octave_int<octave_idx_type> (x).value ();
 
-  return convert_index (i, conv_error, ext);
+  return convert_index (i, ext);
 }
 
 template <typename T>
@@ -270,7 +255,7 @@
 {
   octave_idx_type dummy = 0;
 
-  data = convert_index (x, err, dummy);
+  data = convert_index (x, dummy);
 }
 
 idx_vector::idx_scalar_rep::idx_scalar_rep (octave_idx_type i)
@@ -325,7 +310,7 @@
       std::unique_ptr<octave_idx_type []> d (new octave_idx_type [len]);
 
       for (octave_idx_type i = 0; i < len; i++)
-        d[i] = convert_index (nda.xelem (i), err, ext);
+        d[i] = convert_index (nda.xelem (i), ext);
 
       data = d.release ();
     }
@@ -344,10 +329,7 @@
         {
           octave_idx_type k = inda.xelem (i);
           if (k < 0)
-            {
-              if (! err)
-                octave::err_invalid_index (k);
-            }
+            octave::err_invalid_index (k);
           else if (k > max)
             max = k;
         }
@@ -1285,13 +1267,8 @@
 idx_vector::freeze (octave_idx_type z_len, const char *, bool resize_ok)
 {
   if (! resize_ok && extent (z_len) > z_len)
-    {
-      (*current_liboctave_error_handler)
-        ("invalid matrix index = %" OCTAVE_IDX_TYPE_FORMAT, extent (z_len));
-      // FIXME: Should we call this before calling error_handler?
-      rep->err = true;
-      chkerr ();
-    }
+    (*current_liboctave_error_handler)
+      ("invalid matrix index = %" OCTAVE_IDX_TYPE_FORMAT, extent (z_len));
 
   return length (z_len);
 }
--- a/liboctave/array/idx-vector.h	Sun Apr 25 17:36:12 2021 +0200
+++ b/liboctave/array/idx-vector.h	Wed Apr 28 10:23:41 2021 -0400
@@ -81,7 +81,7 @@
   {
   public:
 
-    idx_base_rep (void) : count (1), err (false) { }
+    idx_base_rep (void) : count (1) { }
 
     // No copying!
 
@@ -123,8 +123,6 @@
     virtual Array<octave_idx_type> as_array (void);
 
     octave::refcount<octave_idx_type> count;
-
-    bool err;
   };
 
   // The magic colon index.
@@ -235,8 +233,7 @@
 
     idx_scalar_rep (void) = delete;
 
-    idx_scalar_rep (octave_idx_type i, direct)
-      : idx_base_rep (), data (i) { }
+    idx_scalar_rep (octave_idx_type i, direct) : idx_base_rep (), data (i) { }
 
     // No copying!
 
@@ -290,8 +287,7 @@
   public:
 
     idx_vector_rep (void)
-      : data (nullptr), len (0), ext (0), aowner (nullptr), orig_dims ()
-    { }
+      : data (nullptr), len (0), ext (0), aowner (nullptr), orig_dims () { }
 
     // Direct constructor.
     idx_vector_rep (octave_idx_type *_data, octave_idx_type _len,
@@ -450,41 +446,22 @@
   // constructor).
   static idx_vector_rep *nil_rep (void);
 
-  // The shared empty vector representation with the error flag set.
-  static idx_vector_rep *err_rep (void);
-
-  // If there was an error in constructing the rep, replace it with
-  // empty vector for safety.
-  void chkerr (void)
-  {
-    if (rep->err)
-      {
-        if (--rep->count == 0)
-          delete rep;
-        rep = err_rep ();
-        rep->count++;
-      }
-  }
-
 public:
 
   // Fast empty constructor.
   idx_vector (void) : rep (nil_rep ()) { rep->count++; }
 
   // Zero-based constructors (for use from C++).
-  idx_vector (octave_idx_type i) : rep (new idx_scalar_rep (i))
-  { chkerr (); }
+  idx_vector (octave_idx_type i) : rep (new idx_scalar_rep (i)) { }
 
 #if OCTAVE_SIZEOF_F77_INT_TYPE != OCTAVE_SIZEOF_IDX_TYPE
   idx_vector (octave_f77_int_type i)
-    : rep (new idx_scalar_rep (static_cast<octave_idx_type> (i)))
-  { chkerr (); }
+    : rep (new idx_scalar_rep (static_cast<octave_idx_type> (i))) { }
 #endif
 
   idx_vector (octave_idx_type start, octave_idx_type limit,
               octave_idx_type step = 1)
-    : rep (new idx_range_rep (start, limit, step))
-  { chkerr (); }
+    : rep (new idx_range_rep (start, limit, step)) { }
 
   static idx_vector
   make_range (octave_idx_type start, octave_idx_type step,
@@ -494,50 +471,43 @@
   }
 
   idx_vector (const Array<octave_idx_type>& inda)
-    : rep (new idx_vector_rep (inda))
-  { chkerr (); }
+    : rep (new idx_vector_rep (inda)) { }
 
   // Directly pass extent, no checking.
   idx_vector (const Array<octave_idx_type>& inda, octave_idx_type ext)
-    : rep (new idx_vector_rep (inda, ext, DIRECT))
-  { }
+    : rep (new idx_vector_rep (inda, ext, DIRECT)) { }
 
   // Colon is best constructed by simply copying (or referencing) this member.
   static const idx_vector colon;
 
   // or passing ':' here
-  idx_vector (char c) : rep (new idx_colon_rep (c)) { chkerr (); }
+  idx_vector (char c) : rep (new idx_colon_rep (c)) { }
 
   // Conversion constructors (used by interpreter).
 
   template <typename T>
-  idx_vector (octave_int<T> x) : rep (new idx_scalar_rep (x)) { chkerr (); }
+  idx_vector (octave_int<T> x) : rep (new idx_scalar_rep (x)) { }
 
-  idx_vector (double x) : rep (new idx_scalar_rep (x)) { chkerr (); }
+  idx_vector (double x) : rep (new idx_scalar_rep (x)) { }
 
-  idx_vector (float x) : rep (new idx_scalar_rep (x)) { chkerr (); }
+  idx_vector (float x) : rep (new idx_scalar_rep (x)) { }
 
   // A scalar bool does not necessarily map to scalar index.
-  idx_vector (bool x) : rep (new idx_mask_rep (x)) { chkerr (); }
+  idx_vector (bool x) : rep (new idx_mask_rep (x)) { }
 
   template <typename T>
-  idx_vector (const Array<octave_int<T>>& nda) : rep (new idx_vector_rep (nda))
-  { chkerr (); }
+  idx_vector (const Array<octave_int<T>>& nda)
+    : rep (new idx_vector_rep (nda)) { }
 
-  idx_vector (const Array<double>& nda) : rep (new idx_vector_rep (nda))
-  { chkerr (); }
+  idx_vector (const Array<double>& nda) : rep (new idx_vector_rep (nda)) { }
 
-  idx_vector (const Array<float>& nda) : rep (new idx_vector_rep (nda))
-  { chkerr (); }
+  idx_vector (const Array<float>& nda) : rep (new idx_vector_rep (nda)) { }
 
   idx_vector (const Array<bool>& nda);
 
-  idx_vector (const octave::range<double>& r)
-    : rep (new idx_range_rep (r))
-  { chkerr (); }
+  idx_vector (const octave::range<double>& r) : rep (new idx_range_rep (r)) { }
 
-  idx_vector (const Sparse<bool>& nda) : rep (new idx_vector_rep (nda))
-  { chkerr (); }
+  idx_vector (const Sparse<bool>& nda) : rep (new idx_vector_rep (nda)) { }
 
   idx_vector (const idx_vector& a) : rep (a.rep) { rep->count++; }
 
@@ -577,8 +547,9 @@
   octave_idx_type operator () (octave_idx_type n) const
   { return rep->xelem (n); }
 
-  operator bool (void) const
-  { return ! rep->err; }
+  // FIXME: idx_vector objects are either created successfully or an
+  // error is thrown, so this method no longer makes sense.
+  operator bool (void) const { return true; }
 
   bool is_colon (void) const
   { return rep->idx_class () == class_colon; }