Re: [boost] Re: move_ptr

From: Peter Dimov (pdimov_at_[hidden])
Date: 2004-01-22 08:06:43

Howard Hinnant wrote:
> On Jan 21, 2004, at 3:38 PM, Peter Dimov wrote:
>
>> Not giving up easily. :-)
>>
>> T & operator[] ( size_t i ) const;
>>
>> Returns: *(get() + i)
>>
>> No opportunity to assert since you don't know the size of the block.
>>
>> T & operator[] ( ptrdiff_t i ) const;
>>
>> Requires: i >= 0
>> Returns: *(get() + i)
>>
>> Since i < 0 is undefined behavior, you (the implementor) now have the
>> choice
>> of assert()ing or defining it so that negative indices wrap around as
>> in the
>> size_t case (in case ptrdiff_t can't hold all size_t values and large
>> arrays
>> are more important than catching errors).
>
> You just about convinced me.  You were this <holds up fingers> close!
> :-)
>
> I like vendor choice (naturally), so your argument really appealed to
> me.  I would have to choose to support large arrays on 16 and 32 bit
> systems.  Buffers of that size are rare, but no longer unheard of.  I
> think I could safely choose assert on 64 bit systems though.
>
> So I cranked up my integer conversion warnings, played with a sample
> large buffer, and with the ptrdiff_t/size_t switch on move_ptr:
>
> size_t buf_size = size_t(2)*1024*1024*1024 + size_t(1)*1024*1024;
> Metrowerks::move_ptr<char[]> p1(new char [buf_size]);
> for (size_t i = 0; i < buf_size; ++i)
>      p1[i] = char(0);
>
> This compiles and runs fine with move_ptr[] using ptrdiff_t or size_t.
> However, it does give an irritating warning when move_ptr[] uses
> ptrdiff_t:
>
> Warning : implicit arithmetic conversion from 'unsigned long' to 'int'
> HelloWorld.cp line 19     p1[i] = char(0);
>
> To silence the warning a naive user might change the type of the index
> to ptrdiff_t:
>
> ptrdiff_t buf_size = ptrdiff_t(2)*1024*1024*1024 +
> ptrdiff_t(1)*1024*1024;
> Metrowerks::move_ptr<char[]> p1(new char [buf_size]);
> for (ptrdiff_t i = 0; i < buf_size; ++i)
>      p1[i] = char(0);
>
> This now compiles perfectly for [](ptrdiff_t), but of course you get a
> warning for [](size_t):
>
> Warning : implicit arithmetic conversion from 'int' to 'unsigned long'
> HelloWorld.cp line 34     p1[i] = char(0);
>
> Despite the fact that it compiles perfectly (with [](ptrdiff_t)) it
> now
> runs incorrectly.  The body of the for loop short circuits (of course)
> on the first test.
>
> So if I want to support large arrays (and I really, really should), my
> choices are:
>
> move_ptr[](ptrdiff_t) : Requires clients to use a cast to silence
> conversion warnings, or risk incorrect indexing logic when buffer size
> grows beyond INT_MAX.
>
> move_ptr[](size_t) :  Works for large arrays with no warnings.
>
> Guess which one I favor. ;-)

We're deep into "theorycraft" now. :-)

FWIW, I really don't think that someone that allocated a 2+Gb array will use
operator[] on it; pointer arithmetic looks much more likely, and we're back
to ptrdiff_t. (Well at least that's how I deal with large arrays.)

Looks like operator[]( unspecified-integral-type ) beats everything.