Re: [patch] Make vector::at() assertion message more useful (try #2)

[Paolo Carlini <paolo dot carlini at oracle dot com>]

Hi,

On 09/13/2013 02:01 AM, Paul Pluzhnikov wrote:
> On Wed, Sep 4, 2013 at 9:55 PM, Daniel Krügler
> <daniel.kruegler@gmail.com> wrote:
>
> > > Did you mean "pessimises code size", or something else?
> > Yes.
> Daniel's idea proved a good one, and I now have a patch that I am
> happy with, and that will be easy to extend to string::at(), and other
> __throw_... functions.
>
> I've added the new snprintf.cc to c++11/ rather than c++98/ as Paolo
> suggested, because the only current caller is in c++11/functexcept.cc
Patch looks pretty good to me. Thanks for persisting. Some details:
- The game with the variadic and the non-variadic __throw_out_of_range 
makes me a little nervous. Let's just name the new one differently, like 
__throw_out_of_range_var.
- Please consistently use __builtin_alloca everywhere, alloca isn't a 
standard function.
- I would rather call the file itself snprintf_lite.cc, in order not to 
fool somebody that it actually implements the whole snprintf.
- I'm a bit confused about __concat_size_t returning -1. Since it only 
formats integers, I think we can be *sure* that the buffer is big 
enough. Then, if it returns -1 something is going *very badly* wrong, 
shouldn't we __builtin_abort() or something similar?
- In terms of buffer sizes, this comment:

    // enough for expanding up to 5 size_t's in the format.

and then the actual code in __snprintf_lite makes me a little nervous. 
Agreed, we are not going to overflow the buffer, but truncating with no 
diagnostic whatsoever seems rather gross. We can probably sort out this 
later, new ideas welcome, anyway.
- While we are at it, shouldn't we use the new facility at least in 
array, vector<bool> and deque too? For consistency over the containers.

Thanks,
Paolo.