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

[Paul Pluzhnikov <ppluzhnikov at google dot com>]

On Fri, Sep 13, 2013 at 3:02 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:

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

Replaced with __throw_out_of_range_fmt.

> - Please consistently use __builtin_alloca everywhere, alloca isn't a
> standard function.

Done.

> - I would rather call the file itself snprintf_lite.cc, in order not to fool
> somebody that it actually implements the whole snprintf.

Done.

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

How so? The caller could do this:

  __snprintf_lite("aaa: %zu, 8, 12345678);

By the time we get to __concat_size_t, we only have 3 characters left.

> Then, if it returns -1 something is going *very badly* wrong, shouldn't we
> __builtin_abort() or something similar?

I've re-worked this part -- if this ever happens, throw a logic_error with
a message to file a bug.

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

Re-worked.

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

Done.

I also expanded snprintf_lite to handle '%s', as that comes handy inside
string _M_check.

Thanks,

P.S. In the process of updating callers of __throw_out_of_range, I've
discovered that two of them had already used __builtin_sprintf.

P.P.S. Sorry this patch grew ... I can split it into parts if that's easier
to review.


-- 
Paul Pluzhnikov


libstdc++-v3/ChangeLog:

	* include/bits/functexcept.h (__throw_out_of_range_fmt): New.
	* src/c++11/functexcept.cc (__throw_out_of_range_fmt): New.
	* src/c++11/snprintf_lite.cc: New.
	* src/c++11/Makefile.am: Add snprintf_lite.cc.
	* src/c++11/Makefile.in: Regenerate.
	* config/abi/pre/gnu.ver: Add _ZSt24__throw_out_of_range_fmtPKcz.
	* include/std/array (at): Use __throw_out_of_range_fmt.
	* include/debug/array (at): Likewise.
	* include/profile/array (at): Likewise.
	* include/std/bitset (_M_check_initial_position, _M_check): New.
	(bitset::bitset): Use _M_check_initial_position.
	(set, reset, flip, test): Use _M_check.
	* include/ext/vstring.h (_M_check, at): Use __throw_out_of_range_fmt.
	* include/bits/stl_vector.h (_M_range_check): Likewise.
	* include/bits/stl_bvector.h (_M_range_check): Likewise.
	* include/bits/stl_deque.h (_M_range_check): Likewise.
	* include/bits/basic_string.h (_M_check, at): Likewise.
	* testsuite/23_containers/vector/requirements/dr438/assign_neg.cc: Adjust.
	* testsuite/23_containers/vector/requirements/dr438/insert_neg.cc: Likewise.
	* testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc:
Likewise.
	* testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc:
Likewise.
	* testsuite/23_containers/deque/requirements/dr438/assign_neg.cc: Likewise.
	* testsuite/23_containers/deque/requirements/dr438/insert_neg.cc: Likewise.
	* testsuite/23_containers/deque/requirements/dr438/constructor_1_neg.cc:
Likewise.
	* testsuite/23_containers/deque/requirements/dr438/constructor_2_neg.cc:
Likewise.
	* testsuite/23_containers/array/tuple_interface/tuple_element_neg.cc: Likewise.
	* testsuite/23_containers/array/tuple_interface/get_neg.cc: Likewise.
	* testsuite/util/exception/safety.h (generate): Use __throw_out_of_range_fmt.

Attachment: gcc-20130918.txt
Description: Text document