# HG changeset patch # User Jaroslav Hajek # Date 1231946505 -3600 # Node ID a62a982163b6ec855cf6d360a9e106e2ac14d342 # Parent f3a54bae02c8e81cd79a8ceac45065f8af07f7fc clean up Array and DiagArray2 diff --git a/liboctave/Array.cc b/liboctave/Array.cc --- a/liboctave/Array.cc +++ b/liboctave/Array.cc @@ -46,6 +46,26 @@ // all the derived classes. template +void +Array::make_unique (void) +{ + if (rep->count > 1) + { + --rep->count; + rep = new ArrayRep (slice_data, slice_len, true); + slice_data = rep->data; + } + else if (slice_len != rep->len) + { + // Possibly economize here. + ArrayRep *new_rep = new ArrayRep (slice_data, slice_len, true); + delete rep; + rep = new_rep; + slice_data = rep->data; + } +} + +template Array::Array (const Array& a, const dim_vector& dv) : rep (a.rep), dimensions (dv), slice_data (a.slice_data), slice_len (a.slice_len) @@ -82,6 +102,20 @@ } return *this; +} + +template +void +Array::fill (const T& val) +{ + if (rep->count > 1) + { + --rep->count; + rep = new ArrayRep (length (), val); + slice_data = rep->data; + } + else + std::fill (slice_data, slice_data + slice_len, val); } template @@ -131,13 +165,7 @@ } } - // FIXME -- it would be better if we did not have to do - // this, so we could share the data while still having different - // dimension vectors. - - retval.make_unique (); - - retval.dimensions = new_dimensions; + retval = Array (*this, new_dimensions); } return retval; diff --git a/liboctave/Array.h b/liboctave/Array.h --- a/liboctave/Array.h +++ b/liboctave/Array.h @@ -97,49 +97,15 @@ public: - // !!! WARNING !!! -- these should be protected, not public. You - // should not access these methods directly! - - void make_unique (void) - { - if (rep->count > 1) - { - --rep->count; - rep = new ArrayRep (slice_data, slice_len, true); - slice_data = rep->data; - } - else if (slice_len != rep->len) - { - // Possibly economize here. - ArrayRep *new_rep = new ArrayRep (slice_data, slice_len, true); - delete rep; - rep = new_rep; - slice_data = rep->data; - } - } - - void make_unique (const T& val) - { - if (rep->count > 1) - { - --rep->count; - rep = new ArrayRep (slice_len, val); - slice_data = rep->data; - } - else - std::fill (slice_data, slice_data + slice_len, val); - } + void make_unique (void); typedef T element_type; - // !!! WARNING !!! -- these should be protected, not public. You - // should not access these data members directly! +protected: typename Array::ArrayRep *rep; dim_vector dimensions; - -protected: T* slice_data; octave_idx_type slice_len; @@ -220,7 +186,7 @@ template Array (const Array& a) : rep (new typename Array::ArrayRep (coerce (a.data (), a.length ()), a.length ())), - dimensions (a.dimensions) + dimensions (a.dims ()) { slice_data = rep->data; slice_len = rep->len; @@ -260,7 +226,7 @@ Array& operator = (const Array& a); - void fill (const T& val) { make_unique (val); } + void fill (const T& val); octave_idx_type capacity (void) const { return slice_len; } octave_idx_type length (void) const { return capacity (); } diff --git a/liboctave/ChangeLog b/liboctave/ChangeLog --- a/liboctave/ChangeLog +++ b/liboctave/ChangeLog @@ -0,0 +1,14 @@ +2009-01-14 Jaroslav Hajek + + * Array.h (Array::rep, Array::dimensions): Make protected. + * Array.cc (Array::make_unique): Move implementation here. + (Array::fill): Dtto. + * DiagArray2.h (DiagArray2): Reimplement without abusing + Array internals. + (DiagArray2::operator Array2): New method. + * DiagArray2.cc (DiagArray2): Update methods. + * MDiagArray2.h (MDiagArray2::operator Array2): Simplify. + * PermMatrix.h (PermMatrix): Reimplement without abusing + Array internals. + * PermMatrix.cc (PermMatrix): Update methods. + diff --git a/liboctave/DiagArray2.cc b/liboctave/DiagArray2.cc --- a/liboctave/DiagArray2.cc +++ b/liboctave/DiagArray2.cc @@ -37,17 +37,42 @@ #include "lo-error.h" template +const typename DiagArray2::Proxy& +DiagArray2::Proxy::operator = (const T& val) const +{ + if (i == j) + { + if (object) + object->set (val, i); + } + else + (*current_liboctave_error_handler) + ("invalid assignment to off-diagonal in diagonal array"); + + return *this; +} + +template +DiagArray2::Proxy::operator T () const +{ + if (object && i == j) + return object->get (i); + else + { + static T foo; + return foo; + } +} + +template Array DiagArray2::diag (octave_idx_type k) const { Array d; if (k == 0) - { - // The main diagonal is shallow-copied. - d = *this; - d.dimensions = dim_vector (length ()); - } + // The main diagonal is shallow-copied. + d = *this; else if (k > 0 && k < cols ()) d = Array (std::min (cols () - k, rows ()), T ()); else if (k < 0 && -k < rows ()) @@ -64,7 +89,8 @@ DiagArray2::transpose (void) const { DiagArray2 retval (*this); - retval.dimensions = dim_vector (dim2 (), dim1 ()); + retval.d1 = d2; + retval.d2 = d1; return retval; } @@ -91,7 +117,20 @@ (*current_liboctave_error_handler) ("range error in DiagArray2"); return T (); } - return (r == c) ? Array::xelem (r) : T (0); + return elem (r, c); +} + +template +typename DiagArray2::Proxy +DiagArray2::checkelem (octave_idx_type r, octave_idx_type c) +{ + if (r < 0 || c < 0 || r >= dim1 () || c >= dim2 ()) + { + (*current_liboctave_error_handler) ("range error in DiagArray2"); + return Proxy (0, r, c); + } + else + return Proxy (this, r, c); } template @@ -104,37 +143,16 @@ return; } - if (r == dim1 () && c == dim2 ()) - return; - - // FIXME: this is a mess. DiagArray2 really needs a rewrite. - typename Array::ArrayRep *old_rep = Array::rep; - const T *old_data = this->data (); - octave_idx_type old_len = this->length (); - - octave_idx_type new_len = r < c ? r : c; - - Array::rep = new typename Array::ArrayRep (new_len); - Array::slice_data = Array::rep->data; - Array::slice_len = Array::rep->len; - - this->dimensions = dim_vector (r, c); - - if (old_data && old_len > 0) + if (r != dim1 () || c != dim2 ()) { - octave_idx_type min_len = old_len < new_len ? old_len : new_len; - - for (octave_idx_type i = 0; i < min_len; i++) - xelem (i, i) = old_data[i]; + Array::resize (std::min (r, c)); + d1 = r; d2 = c; } - - if (--old_rep->count <= 0) - delete old_rep; } template void -DiagArray2::resize (octave_idx_type r, octave_idx_type c, const T& val) +DiagArray2::resize_fill (octave_idx_type r, octave_idx_type c, const T& val) { if (r < 0 || c < 0) { @@ -142,32 +160,21 @@ return; } - if (r == dim1 () && c == dim2 ()) - return; + if (r != dim1 () || c != dim2 ()) + { + Array::resize_fill (std::min (r, c), val); + d1 = r; d2 = c; + } +} - typename Array::ArrayRep *old_rep = Array::rep; - const T *old_data = this->data (); - octave_idx_type old_len = this->length (); +template +DiagArray2::operator Array2 (void) const +{ + Array2 result (dim1 (), dim2 ()); + for (octave_idx_type i = 0, len = length (); i < len; i++) + result.xelem (i, i) = dgelem (i); - octave_idx_type new_len = r < c ? r : c; - - Array::rep = new typename Array::ArrayRep (new_len); - - this->dimensions = dim_vector (r, c); - - octave_idx_type min_len = old_len < new_len ? old_len : new_len; - - if (old_data && old_len > 0) - { - for (octave_idx_type i = 0; i < min_len; i++) - xelem (i, i) = old_data[i]; - } - - for (octave_idx_type i = min_len; i < new_len; i++) - xelem (i, i) = val; - - if (--old_rep->count <= 0) - delete old_rep; + return result; } /* diff --git a/liboctave/DiagArray2.h b/liboctave/DiagArray2.h --- a/liboctave/DiagArray2.h +++ b/liboctave/DiagArray2.h @@ -3,7 +3,7 @@ Copyright (C) 1996, 1997, 2000, 2002, 2003, 2004, 2005, 2006, 2007 John W. Eaton -Copyright (C) 2008 Jaroslav Hajek +Copyright (C) 2008, 2009 Jaroslav Hajek This file is part of Octave. @@ -30,10 +30,10 @@ #include #include "Array.h" +#include "Array2.h" #include "lo-error.h" // A two-dimensional array with diagonal elements only. -// // Idea and example code for Proxy class and functions from: // // From: address@hidden (James Kanze) @@ -45,13 +45,12 @@ // James Kanze email: address@hidden // GABI Software, Sarl., 8 rue du Faisan, F-67000 Strasbourg, France -// Array is inherited privately because we abuse the dimensions variable -// for true dimensions. Therefore, the inherited Array object is not a valid -// Array object, and should not be publicly accessible. +// Array is inherited privately so that some methods, like index, don't +// produce unexpected results. template class -DiagArray2 : private Array +DiagArray2 : protected Array { private: @@ -66,30 +65,9 @@ Proxy (DiagArray2 *ref, octave_idx_type r, octave_idx_type c) : i (r), j (c), object (ref) { } - const Proxy& operator = (const T& val) const - { - if (i == j) - { - if (object) - object->set (val, i); - } - else - (*current_liboctave_error_handler) - ("invalid assignment to off-diagonal in diagonal array"); + const Proxy& operator = (const T& val) const; - return *this; - } - - operator T () const - { - if (object && i == j) - return object->get (i); - else - { - static T foo; - return foo; - } - } + operator T () const; private: @@ -106,100 +84,92 @@ }; -friend class Proxy; + friend class Proxy; protected: + octave_idx_type d1, d2; - DiagArray2 (T *d, octave_idx_type r, octave_idx_type c) : Array (d, r < c ? r : c) - { Array::dimensions = dim_vector (r, c); } + DiagArray2 (T *d, octave_idx_type r, octave_idx_type c) + : Array (d, std::min (r, c)), d1 (r), d2 (c) { } public: typedef T element_type; - DiagArray2 (void) : Array (dim_vector (0, 0)) { } + DiagArray2 (void) + : Array (), d1 (0), d2 (0) { } - DiagArray2 (octave_idx_type r, octave_idx_type c) : Array (r < c ? r : c) - { this->dimensions = dim_vector (r, c); } + DiagArray2 (octave_idx_type r, octave_idx_type c) + : Array (std::min (r, c)), d1 (r), d2 (c) { } - DiagArray2 (octave_idx_type r, octave_idx_type c, const T& val) : Array (r < c ? r : c) - { - this->dimensions = dim_vector (r, c); + DiagArray2 (octave_idx_type r, octave_idx_type c, const T& val) + : Array (std::min (r, c), val), d1 (r), d2 (c) { } - Array::fill (val); - } + DiagArray2 (const Array& a) + : Array (a), d1 (a.numel ()), d2 (a.numel ()) { } - DiagArray2 (const Array& a) : Array (a) - { this->dimensions = dim_vector (a.length (), a.length ()); } - - DiagArray2 (const DiagArray2& a) : Array (a) - { this->dimensions = a.dims (); } + DiagArray2 (const DiagArray2& a) + : Array (a), d1 (a.d1), d2 (a.d2) { } template - DiagArray2 (const DiagArray2& a) : Array (a.diag ()) - { this->dimensions = a.dims (); } + DiagArray2 (const DiagArray2& a) + : Array (a.diag ()), d1 (a.dim1 ()), d2 (a.dim2 ()) { } ~DiagArray2 (void) { } DiagArray2& operator = (const DiagArray2& a) { if (this != &a) - Array::operator = (a); + { + Array::operator = (a); + d1 = a.d1; + d2 = a.d2; + } return *this; } - - octave_idx_type dim1 (void) const { return Array::dimensions(0); } - octave_idx_type dim2 (void) const { return Array::dimensions(1); } + octave_idx_type dim1 (void) const { return d1; } + octave_idx_type dim2 (void) const { return d2; } octave_idx_type rows (void) const { return dim1 (); } octave_idx_type cols (void) const { return dim2 (); } octave_idx_type columns (void) const { return dim2 (); } + // FIXME: a dangerous ambiguity? octave_idx_type length (void) const { return Array::length (); } octave_idx_type nelem (void) const { return dim1 () * dim2 (); } octave_idx_type numel (void) const { return nelem (); } size_t byte_size (void) const { return length () * sizeof (T); } - dim_vector dims (void) const { return Array::dimensions; } + dim_vector dims (void) const { return dim_vector (d1, d2); } Array diag (octave_idx_type k = 0) const; - Proxy elem (octave_idx_type r, octave_idx_type c) - { - return Proxy (this, r, c); - } - - Proxy checkelem (octave_idx_type r, octave_idx_type c) - { - if (r < 0 || c < 0 || r >= dim1 () || c >= dim2 ()) - { - (*current_liboctave_error_handler) ("range error in DiagArray2"); - return Proxy (0, r, c); - } - else - return Proxy (this, r, c); - } - - Proxy operator () (octave_idx_type r, octave_idx_type c) - { - if (r < 0 || c < 0 || r >= dim1 () || c >= dim2 ()) - { - (*current_liboctave_error_handler) ("range error in DiagArray2"); - return Proxy (0, r, c); - } - else - return Proxy (this, r, c); - } + // Warning: the non-const two-index versions will silently ignore assignments + // to off-diagonal elements. T elem (octave_idx_type r, octave_idx_type c) const { - return (r == c) ? Array::xelem (r) : T (0); + return (r == c) ? Array::elem (r) : T (0); } + T& elem (octave_idx_type r, octave_idx_type c) + { + static T zero (0); + return (r == c) ? Array::elem (r) : zero; + } + + T dgelem (octave_idx_type i) const + { return Array::elem (i); } + + T& dgelem (octave_idx_type i) + { return Array::elem (i); } + T checkelem (octave_idx_type r, octave_idx_type c) const; + Proxy checkelem (octave_idx_type r, octave_idx_type c); + T operator () (octave_idx_type r, octave_idx_type c) const { #if defined (BOUNDS_CHECKING) @@ -209,24 +179,39 @@ #endif } + // FIXME: can this cause problems? +#if defined (BOUNDS_CHECKING) + Proxy operator () (octave_idx_type r, octave_idx_type c) + { + return checkelem (r, c); + } +#else + T& operator () (octave_idx_type r, octave_idx_type c) + { + return elem (r, c); + } +#endif + // No checking. - - T& xelem (octave_idx_type r, octave_idx_type c) - { - static T foo (0); - return (r == c) ? Array::xelem (r) : foo; - } T xelem (octave_idx_type r, octave_idx_type c) const { return (r == c) ? Array::xelem (r) : T (0); } + T& dgxelem (octave_idx_type i) + { return Array::xelem (i); } + + T dgxelem (octave_idx_type i) const + { return Array::xelem (i); } + void resize (octave_idx_type n, octave_idx_type m); - void resize (octave_idx_type n, octave_idx_type m, const T& val); + void resize_fill (octave_idx_type n, octave_idx_type m, const T& val); DiagArray2 transpose (void) const; DiagArray2 hermitian (T (*fcn) (const T&) = 0) const; + + operator Array2 (void) const; const T *data (void) const { return Array::data (); } diff --git a/liboctave/MDiagArray2.h b/liboctave/MDiagArray2.h --- a/liboctave/MDiagArray2.h +++ b/liboctave/MDiagArray2.h @@ -72,17 +72,7 @@ operator MArray2 () const { - octave_idx_type nr = DiagArray2::dim1 (); - octave_idx_type nc = DiagArray2::dim2 (); - - MArray2 retval (nr, nc, T (0)); - - octave_idx_type len = nr < nc ? nr : nc; - - for (octave_idx_type i = 0; i < len; i++) - retval.xelem (i, i) = this->xelem (i, i); - - return retval; + return DiagArray2::operator Array2 (); } octave_idx_type nnz (void) const diff --git a/liboctave/PermMatrix.cc b/liboctave/PermMatrix.cc --- a/liboctave/PermMatrix.cc +++ b/liboctave/PermMatrix.cc @@ -39,7 +39,6 @@ PermMatrix::PermMatrix (const Array& p, bool colp, bool check) : Array (p), _colp(colp) { - this->dimensions = dim_vector (p.length (), p.length ()); if (check) { if (! idx_vector (p).is_permutation (p.length ())) @@ -61,14 +60,12 @@ Array idxa (len); for (octave_idx_type i = 0; i < len; i++) idxa(i) = idx(i); Array::operator = (idxa); - this->dimensions = dim_vector (len, len); } } PermMatrix::PermMatrix (octave_idx_type n) : Array (n), _colp (false) { - this->dimensions = dim_vector (n, n); for (octave_idx_type i = 0; i < n; i++) xelem (i) = i; } diff --git a/liboctave/PermMatrix.h b/liboctave/PermMatrix.h --- a/liboctave/PermMatrix.h +++ b/liboctave/PermMatrix.h @@ -26,15 +26,11 @@ #include "Array.h" #include "mx-defs.h" -// Array is inherited privately because we abuse the dimensions variable -// for true dimensions. Therefore, the inherited Array object is not a valid -// Array object, and should not be publicly accessible. +// Array is inherited privately so that some methods, like index, don't +// produce unexpected results. -class PermMatrix : private Array +class PermMatrix : protected Array { -private: - - octave_idx_type get (octave_idx_type i) const { return Array::xelem (i); } public: @@ -51,36 +47,34 @@ PermMatrix (const idx_vector& idx, bool colp = false, octave_idx_type n = 0); octave_idx_type dim1 (void) const - { return Array::dimensions(0); } + { return Array::length (); } octave_idx_type dim2 (void) const - { return Array::dimensions(1); } + { return Array::length (); } octave_idx_type rows (void) const { return dim1 (); } octave_idx_type cols (void) const { return dim2 (); } octave_idx_type columns (void) const { return dim2 (); } + octave_idx_type perm_length (void) const + { return Array::length (); } octave_idx_type length (void) const - { return Array::length (); } + { return dim1 () * dim2 (); } octave_idx_type nelem (void) const { return dim1 () * dim2 (); } octave_idx_type numel (void) const { return nelem (); } - size_t byte_size (void) const { return length () * sizeof (octave_idx_type); } + size_t byte_size (void) const { return perm_length () * sizeof (octave_idx_type); } - dim_vector dims (void) const { return Array::dimensions; } + dim_vector dims (void) const { return dim_vector (dim1 (), dim2 ()); } Array pvec (void) const - { - Array retval (*this); - retval.dimensions = dim_vector (length ()); - return retval; - } + { return *this; } octave_idx_type elem (octave_idx_type i, octave_idx_type j) const { return (_colp - ? ((get(j) != i) ? 1 : 0) - : ((get(i) != j) ? 1 : 0)); + ? ((Array::elem (j) != i) ? 1 : 0) + : ((Array::elem (i) != j) ? 1 : 0)); } octave_idx_type diff --git a/src/strfns.cc b/src/strfns.cc --- a/src/strfns.cc +++ b/src/strfns.cc @@ -395,7 +395,7 @@ { // Broadcast the string. - boolNDArray output (cell.dimensions, false); + boolNDArray output (cell.dims (), false); std::string s = r == 0 ? std::string () : str[0]; @@ -430,7 +430,7 @@ { // Must match in all dimensions. - boolNDArray output (cell.dimensions, false); + boolNDArray output (cell.dims (), false); if (cell.length () == r) { @@ -471,8 +471,8 @@ r2 = cell2.length (); } - const dim_vector size1 = cell1.dimensions; - const dim_vector size2 = cell2.dimensions; + const dim_vector size1 = cell1.dims (); + const dim_vector size2 = cell2.dims (); boolNDArray output (size1, false); @@ -671,7 +671,7 @@ { // Broadcast the string. - boolNDArray output (cell.dimensions, false); + boolNDArray output (cell.dims (), false); if (c < n) { @@ -724,7 +724,7 @@ { // Must match in all dimensions. - boolNDArray output (cell.dimensions, false); + boolNDArray output (cell.dims (), false); if (cell.numel () == r) { @@ -774,8 +774,8 @@ r2 = cell2.length (); } - const dim_vector size1 = cell1.dimensions; - const dim_vector size2 = cell2.dimensions; + const dim_vector size1 = cell1.dims (); + const dim_vector size2 = cell2.dims (); boolNDArray output (size1, false);