Re: Rethinking octave_idx_type

[Carnë Draug]

On 25 November 2016 at 17:38, John W. Eaton <address@hidden> wrote:
> On 11/25/2016 12:33 PM, Mike Miller wrote:
>
>> Do you have a reason to prefer a signed type rather than an unsigned
>> size_t? Using a size_t would make Array<T> more compatible with STL
>> container definitions.
>
>
> We have a number of places where we use -1 to indicate "not defined". So as
> a first pass, using a signed type will be easier.  Once that works we could
> consider just using size_t and fixing the places where negative values are
> used or possible.
>
> jwe

Using size_t will definitely break the image package since it uses
octave_idx_type type to store offsets in an array which are often
negative.

On 26 November 2016 at 22:27, John W. Eaton <address@hidden> wrote:
> On 11/25/2016 11:35 PM, Rik wrote:
>
>> Other than performance, I think it does make sense to become more
>> compatible with the STL.  I don't think we have to switch to an unsigned
>> type though.  2^63 = 9 x 10^18 elements and if each element is an IEEE
>> double that would be 70 exabytes.  I don't know of any personal PCs that
>> would need the extra bit of addressing from an unsigned type because
>> they actually have 140 exabytes of memory.
>
> Using size_t (which is unsigned) helps compatibility with the STL because
> that's what it uses for sizes and indexing.  But I agree it is not
> absolutely necessary.  We already get away with using a signed type and have
> dealt with most of the places where compilers warn about mixing signed and
> unsigned types.

We get away with the issue but because of it it's a bum to write templates
that handle both STL containers and Octave Array.  So if we are going to
change this, why not go all the way?  We don't have to break anything.

What if instead of changing octave_idx_type, we have Array<T>::size_type
and Array<T>::difference_type?  These would map nicely to the STL names.
We have Array<T>::size_type [1] since Octave 4.2 but I will guess this is
not used outside of core so disruption would be minimal.  We could leave
octave_idx_type alone and deprecate it while recommending to use this new
two typedefs.

Carnë

[1] 
http://hg.savannah.gnu.org/hgweb/octave/file/296d9b0eec8d/liboctave/array/Array.h#l203