[patch] Make vector::at() assertion message more useful
Oleg Endo
oleg.endo@t-online.de
Sun Aug 18 11:12:00 GMT 2013
On Sun, 2013-08-18 at 11:48 +0100, Chris Jefferson wrote:
> On 18/08/13 11:42, Oleg Endo wrote:
> > On Sun, 2013-08-18 at 11:38 +0100, Chris Jefferson wrote:
> >> On 18/08/13 08:09, Václav Zeman wrote:
> >>> On 08/17/2013 05:10 PM, Gabriel Dos Reis wrote:
> >>>> On Sat, Aug 17, 2013 at 5:41 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> >>>>> Hi,
> >>>>>
> >>>>> Paul Pluzhnikov <ppluzhnikov@google.com> ha scritto: __throw_out_of_range(__N("vector::_M_range_check"));
> >>>>>> + {
> >>>>>> + char __s[256];
> >>>>>> + __builtin_snprintf(__s, sizeof(__s),
> >>>>>> + __N("vector::_M_range_check: %zu >= %zu"),
> >>>>>> + __n, this->size());
> >>>>>> + __throw_out_of_range(__s);
> >>>>>> + }
> >>>>> The idea makes sense, but while we are at it I think the message could be more clear, say what the two numbers are. Also, I don't think we can unconditionally call snprintf, it's C99 and supported targets lack it. Maybe with some libiberty magic? I don't think such magic automagically triggers when __builtin_snprintf is expanded, or does it? Please investigate that.
> >>>> I agree that we can't embark a significant IO library as part of a purely
> >>>> data structure components.
> >>>>
> >>>> Furthermore, __builtin_snprintf doesn't always expand to a
> >>>> compiler magic -- it can be a normal function call.
> >>>>
> >>>> This would be a regression, not an improvement.
> >>> What about adding a simple template function that converts integer into
> >>> std::string using division by 10? You could then concatenate the message
> >>> from std::string fragments, avoiding IO library but having to pull in
> >>> std::string instead. Would that be acceptable?
> >> Rather than making up new functions, why not use a std::ostringstream?
> > Because that would pull in the IO library parts, which is undesired in
> > this case.
>
> There seems to be two seperate issues here (unless I misunderstand).
>
> 1) The problem with bringing in snprintf is that it is not supported on
> all targets. Therefore we have to worry about conditional support.
>
> 2) ostringstream is supported on all platforms, but bringing it in
> includes a largish header and a large chunk of code, although only in
> debug mode.
>
>
> Obviously if there are platforms that do not support ostringstream, we
> should not use it as we get back to the same conditional support issues
> of snprintf. However, I don't see any significant disadvantages to using
> it in debug mode (which already, by necessity, produces larger and much
> slower code).
It would be nice if the exception message was the same regardless of the
debug / non-debug configuration. Using ostringstream seems to be adding
unnecessary clunker. If this is about code re-use, one way could be:
- have some internal simple integer formatting helper functions that
format into a char buffer, i.e. non standard "itoa".
- use those for implementing C++11 std::to_string
(not sure whether std::to_string is really required to actually use
sprintf ...)
- use either std::to_string to format the exception message or just the
internal integer formatting functions directly.
Cheers,
Oleg
More information about the Libstdc++
mailing list