[boost] Re: move_ptr

From: Howard Hinnant (hinnant_at_[hidden])
Date: 2004-01-21 17:06:35

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. ;-)

-Howard