stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: 22.locale.money.get.cpp asserts
Date Wed, 26 Sep 2007 05:05:29 GMT
Farid Zaripov wrote:
>   I have checked the re-asserting lines of the test and compared the
> results with
> DinkumwareSTL and STLport.
> 
>   The line 752:
> 
>     // all spaces extracted since currency symbol (last) is mandatory
>     // verify that the facet doesn't extract too many optional spaces
>     // leaving none for the final required currency symbol
>     TEST (T,  103.0, "103 ", 4, showbase, eofbit, 0, "\3\4\0\2", " ");

I remember this being a problem when I first wrote and tested
the facet. It might be a simple bug or there might be an issue
in the spec. I don't remember anymore so it will take some
studying of the standard to see which it is. It might also
be helpful to look at the strpmon function (wherever it's
implemented) for any precedent or potential guidance

FWIW, I think it might improve the readability if we replaced
the numeric values in the pattern with the symbols used in the
set_pattern() function in the 22.locale.money.put test (i.e.,
"@', ' ', '$', '-' (or '+'), and '1' (or '#') for none, space,
symbol, sign, and value, respectively. The pattern above would
become "+#@$" -- seems a little easier to read than "\3\4\0\2".

> 
>   The line 798:
> 
>     const charT minus_space[][3] = { { '-', ' ', '\0' }, { '\0' } };
>     PunctData<charT>::negative_sign_ [intl] = minus_space [0];
>     PunctData<charT>::negative_sign_ [locl] = minus_space [1];
> 
>     // { sign, value, none, symbol }
>     // negative_sign is "- ", (note the single trailing space)
>     // verify that the extraction suceeds, i.e., that the
> money_base::none
>     // specifier that's last in the pattern doesn't confuse the facet
> into
>     // extracting all the optional whitespace, leaving none to complete
>     // the negative_sign
>     TEST (T, -109.1, "-109  ", 6, 0, eofbit, 0, "\3\4\0\2", "");
> 
>   The problem is in that all spaces are eaten when 'none' pattern
> element was parsed,
> but the ending currency character in first test and trailing
> negative_sign character
> in second test are requires for presence of the space character.
> 
>   The both test lines above are failed on DinkumwareSTL and STLport too.

They could be buggy as well. Some of these are pretty corner cases.

> The questions:
> - may the negative_sign have the trailing part with only space
> characters?
> - may the currency symbol contain only spaces?

There are no restrictions on either the signs or the currency symbol,
but that doesn't mean there must be a way to unambiguously parse them
in certain kinds of input (ideally, there would be, but it's possible
there is an ambiguity given some formats).

Incidentally, when considering this, it's important to look not just
at the money_get facet but also at the "grammar" in [locale.moneypunct],
p1 - p4. There are also a few new issues against the monetary facets
on the committee's issue list that may be of interest:
http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-active.html

> 
> 
> 
>   The another lines with rw_asserts() (851 - 853):
> 
>     PunctData<charT>::positive_sign_ [intl] = 0;
>     PunctData<charT>::positive_sign_ [locl] = 0;
>     PunctData<charT>::negative_sign_ [intl] = 0;
>     PunctData<charT>::negative_sign_ [locl] = 0;
> 
>     [...]
> 
>     TEST (T,  117.0, "117$",   4, showbase, eofbit,  0, "\4\0\2\3",
> "$");
>     TEST (T,  118.0, "118$",   4, showbase, eofbit,  0, "\4\0\2\3",
> "$");
>     TEST (T,  119.0, "119 $",  5, showbase, eofbit,  0, "\4\1\2\3",
> "$");
> 
>   Here the sign is the last position in pattern, and positive sign and
> negative sign both are empty.
> I've prepared the patch, fixing the problem:

Just so I'm clear, this fixes the last problem (i.e., the last three
assertions), right?

It looks okay to me. There are some locale tests where the .get test
exercise the xxx_put facet and/or vice versa. It's probably not the
best design but while they're there it's good to run both the .get
and .put tests when making changes. If you have run both kinds of
tests I see no reason not to commit the patch.

Martin

> 
> 
> Index: _money_get.cc
> ===================================================================
> --- _money_get.cc	(revision 579104)
> +++ _money_get.cc	(working copy)
> @@ -165,7 +165,10 @@
>          case /* '\3' */ money_base::sign: {
>  
>              if (__it == __end) {
> -                __ebits |= _RW::__rw_failbit;
> +                if (__ps.size () && __ns.size ())
> +                    __ebits |= _RW::__rw_failbit;
> +                else
> +                    __sign = __ps.empty () - 1;
>                  break;
>              }
>  
> 
> Farid.
> 


Mime
View raw message