You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stdcxx.apache.org by Travis Vitek <Tr...@roguewave.com> on 2008/03/05 19:58:05 UTC

RE: [PATCH] punct.cpp

 

>Travis Vitek wrote:
>
>>Martin Sebor wrote:
>>
>>I don't have the time to do a careful review of the patch WRT
>>the LWG issue at the moment but I note there's a comment above
>>the code that's being modified that references the issue, so
>>it looks like the current code already tries to handle it. If
>>it does so successfully or not I can't say without spending
>>more time on it.
>
>Here is a quote from the resolution...
>
>  For conversion from a floating-point type, str.precision()
>  is specified in the conversion specification.
>
>The following is taken from the comments following that...
>
>  The floatfield determines whether numbers are formatted as
>  if with %f, %e, or %g. If the fixed bit is set, it's %f, if
>  scientific it's %e, and if both bits are set, or neither,
>  it's %g.
>
>  Turning to the C standard, a precision of 0 is meaningful
>  for %f and %e. For %g, precision 0 is taken to be the same
>  as precision 1.
>
>Previously, if the precision was 0 and floatfield was not
>fixed or scientific [i.e. %g] we wouldn't apply precision
>format string at all. According to this, we should apply
>precision 0 and assume that the printf() family correctly
>handles this case, or force precision to 1.
>

I've tested the patch on AIX, and it fixes failure of regression test
22.locale.num.put.stdcxx-2. Unfortunately it causes an additional 22
assertions to fail in 22.locale.num.put.

Travis

RE: [PATCH] punct.cpp

Posted by Travis Vitek <Tr...@roguewave.com>.
Let me try that again, this time with code that compiles and in a more
friendly tone :)

>Martin Sebor wrote:
>
>Yes, although it's not immediately obvious to me what the problem
>is, The first assertion above corresponds to this line:
>
>     TEST (T,  1.1, 0, 0, 0, ' ', "", "%g");
>
>I don't see where the test ends up formatting 1.1 as 1.1 using the
>"%g" directive (on AIX, printf("%g", 1.1) produces 1 as expected.
>

Uh, not where I tested [AIX 5.3, XL C/C++ V9.0].

  $ cat t.cpp
  #include <stdio.h>
  #include <stdlib.h>

  int main (int argc, char* argv [])
  {
      const float f = 1 < argc ? atof (argv [1]) : 1.1;

      printf ("%g\n", f);
      printf ("%.0g\n", f); // should be same as %.1g
      printf ("%.1g\n", f);
      printf ("%.2g\n", f);
      printf ("%.3g\n", f);
      printf ("%.4g\n", f);
      printf ("%.5g\n", f);
      printf ("%.6g\n", f);
      printf ("%.7g\n", f);
      printf ("%.8g\n", f);
      printf ("%.9g\n", f);
      return 0;
  }

  $ xlC t.cpp && a.out 1.1
  1.1
  1
  1
  1.1
  1.1
  1.1
  1.1
  1.1
  1.1
  1.1
  1.10000002

Travis
  

>Martin
> 

>-----Original Message-----
>From: Travis Vitek [mailto:Travis.Vitek@roguewave.com] 
>Sent: Wednesday, March 05, 2008 5:11 PM
>To: dev@stdcxx.apache.org
>Subject: RE: [PATCH] punct.cpp
>
> 
>
>>Martin Sebor wrote:
>>
>>Yes, although it's not immediately obvious to me what the problem
>>is, The first assertion above corresponds to this line:
>>
>>     TEST (T,  1.1, 0, 0, 0, ' ', "", "%g");
>>
>>I don't see where the test ends up formatting 1.1 as 1.1 using the
>>"%g" directive (on AIX, printf("%g", 1.1) produces 1 as expected.
>>
>
>Uh, it most certainly does not.
>
>  $ cat t.cpp
>  #include <stdio.h>
>
>  int main (int argc, char* argv [])
>  {
>      const float f = 1 < argc ? atof (argv [1]) : 1.1;
>
>      printf ("%g\n", f);
>      printf ("%.0g\n", f); // should be same as %.1g
>      printf ("%.1g\n", f);
>      printf ("%.2g\n", f);
>      printf ("%.3g\n", f);
>      printf ("%.4g\n", f);
>      printf ("%.5g\n", f);
>      printf ("%.6g\n", f);
>      printf ("%.7g\n", f);
>      printf ("%.8g\n", f);
>      printf ("%.9g\n", f);
>      return 0;
>  }
>
>  $ xlC t.cpp && a.out 1.1
>  1.1
>  1
>  1
>  1.1
>  1.1
>  1.1
>  1.1
>  1.1
>  1.1
>  1.1
>  1.10000002
>
>Travis
>  
>
>>Martin
>>
>

RE: [PATCH] punct.cpp

Posted by Farid Zaripov <Fa...@epam.com>.
> -----Original Message-----
> From: Martin Sebor [mailto:sebor@roguewave.com] 
> Sent: Friday, March 14, 2008 10:08 PM
> To: dev@stdcxx.apache.org
> Subject: RE: [PATCH] punct.cpp
> 
> > You're right, I was looking at the output of %.g. Looks 
> like we need 
> > to replace the "%g" with "%.0g" here. Let me take care of that.
> > 
> > 
> 
> Actually, I don't think the patch has been applied yet. 
> Farid, please remember to update the test as Travis suggests 
> before committing the patch.

  The test updated thus http://svn.apache.org/viewvc?rev=637392&view=rev

  The patch applied in this commit:
http://svn.apache.org/viewvc?rev=637393&view=rev

Farid.

RE: [PATCH] punct.cpp

Posted by Martin Sebor <se...@roguewave.com>.

Martin Sebor wrote:
> 
> 
> You're right, I was looking at the output of %.g. Looks like we need
> to replace the "%g" with "%.0g" here. Let me take care of that.
> 
> 

Actually, I don't think the patch has been applied yet. Farid, please
remember to update the test as Travis suggests before committing the
patch.

Martin
-- 
View this message in context: http://www.nabble.com/-PATCH--punct.cpp-tp15716394p16053620.html
Sent from the stdcxx-dev mailing list archive at Nabble.com.


RE: [PATCH] punct.cpp

Posted by Martin Sebor <se...@roguewave.com>.

Travis Vitek-4 wrote:
> 
>  
> 
>>Martin Sebor wrote:
>>
>>Yes, although it's not immediately obvious to me what the problem
>>is, The first assertion above corresponds to this line:
>>
>>     TEST (T,  1.1, 0, 0, 0, ' ', "", "%g");
>>
>>I don't see where the test ends up formatting 1.1 as 1.1 using the
>>"%g" directive (on AIX, printf("%g", 1.1) produces 1 as expected.
>>
> 
> Uh, it most certainly does not.
> 
> 

You're right, I was looking at the output of %.g. Looks like we need
to replace the "%g" with "%.0g" here. Let me take care of that.



> 
>   $ cat t.cpp
>   #include <stdio.h>
> 
>   int main (int argc, char* argv [])
>   {
>       const float f = 1 < argc ? atof (argv [1]) : 1.1;
> 
>       printf ("%g\n", f);
>       printf ("%.0g\n", f); // should be same as %.1g
>       printf ("%.1g\n", f);
>       printf ("%.2g\n", f);
>       printf ("%.3g\n", f);
>       printf ("%.4g\n", f);
>       printf ("%.5g\n", f);
>       printf ("%.6g\n", f);
>       printf ("%.7g\n", f);
>       printf ("%.8g\n", f);
>       printf ("%.9g\n", f);
>       return 0;
>   }
> 
>   $ xlC t.cpp && a.out 1.1
>   1.1
>   1
>   1
>   1.1
>   1.1
>   1.1
>   1.1
>   1.1
>   1.1
>   1.1
>   1.10000002
> 
> Travis
>   
> 
>>Martin
>>
> 
> 

-- 
View this message in context: http://www.nabble.com/-PATCH--punct.cpp-tp15716394p16010982.html
Sent from the stdcxx-dev mailing list archive at Nabble.com.


RE: [PATCH] punct.cpp

Posted by Travis Vitek <Tr...@roguewave.com>.
 

>Martin Sebor wrote:
>
>Yes, although it's not immediately obvious to me what the problem
>is, The first assertion above corresponds to this line:
>
>     TEST (T,  1.1, 0, 0, 0, ' ', "", "%g");
>
>I don't see where the test ends up formatting 1.1 as 1.1 using the
>"%g" directive (on AIX, printf("%g", 1.1) produces 1 as expected.
>

Uh, it most certainly does not.

  $ cat t.cpp
  #include <stdio.h>

  int main (int argc, char* argv [])
  {
      const float f = 1 < argc ? atof (argv [1]) : 1.1;

      printf ("%g\n", f);
      printf ("%.0g\n", f); // should be same as %.1g
      printf ("%.1g\n", f);
      printf ("%.2g\n", f);
      printf ("%.3g\n", f);
      printf ("%.4g\n", f);
      printf ("%.5g\n", f);
      printf ("%.6g\n", f);
      printf ("%.7g\n", f);
      printf ("%.8g\n", f);
      printf ("%.9g\n", f);
      return 0;
  }

  $ xlC t.cpp && a.out 1.1
  1.1
  1
  1
  1.1
  1.1
  1.1
  1.1
  1.1
  1.1
  1.1
  1.10000002

Travis
  

>Martin
>

Re: [PATCH] punct.cpp

Posted by Martin Sebor <se...@roguewave.com>.
Travis Vitek wrote:
>  
> 
>> Travis Vitek wrote:
>>
>>> Travis Vitek wrote:
>>>
>>>> Martin Sebor wrote:
>>>>
>>>> I don't have the time to do a careful review of the patch WRT
>>>> the LWG issue at the moment but I note there's a comment above
>>>> the code that's being modified that references the issue, so
>>>> it looks like the current code already tries to handle it. If
>>>> it does so successfully or not I can't say without spending
>>>> more time on it.
>>> Here is a quote from the resolution...
>>>
>>>  For conversion from a floating-point type, str.precision()
>>>  is specified in the conversion specification.
>>>
>>> The following is taken from the comments following that...
>>>
>>>  The floatfield determines whether numbers are formatted as
>>>  if with %f, %e, or %g. If the fixed bit is set, it's %f, if
>>>  scientific it's %e, and if both bits are set, or neither,
>>>  it's %g.
>>>
>>>  Turning to the C standard, a precision of 0 is meaningful
>>>  for %f and %e. For %g, precision 0 is taken to be the same
>>>  as precision 1.
>>>
>>> Previously, if the precision was 0 and floatfield was not
>>> fixed or scientific [i.e. %g] we wouldn't apply precision
>>> format string at all. According to this, we should apply
>>> precision 0 and assume that the printf() family correctly
>>> handles this case, or force precision to 1.
>>>
>> I've tested the patch on AIX, and it fixes failure of regression test
>> 22.locale.num.put.stdcxx-2. Unfortunately it causes an additional 22
>> assertions to fail in 22.locale.num.put.
>>
>> Travis
>>
> 
> I took the time to look at this, and I think 22.locale.num.put.cpp is
> wrong given the resolution of LWG231. Here are a few of the failing
> assertions from 22.locale.num.put...
> 

Here's the full set I get for num_put<char> on AIX 5.3:

$   ./22.locale.num.put  --no-wchar -q --csv \
   | cut -d, -f5- | sed "s/num_put<char>:://"
  1729, "line 377: put (..., double = 1.1) wrote ""1"", expected ""1.1"""
  1730, "line 377: put (..., double = 1.1) wrote ""1"", expected ""1.1"""
  1731, "line 377: put (..., double = -1.1) wrote ""-1"", expected ""-1.1"""
  1732, "line 377: put (..., double = -1.1) wrote ""-1"", expected ""-1.1"""
  1817, "line 377: put (..., long double = 2.1) wrote ""2"", expected 
""2.1"""
  1818, "line 377: put (..., long double = -3.2) wrote ""-3"", expected 
""-3.2"""
  1819, "line 377: put (..., long double = -4.3) wrote ""-4"", expected 
""-4.3"""
  1829, "line 377: put (..., long double = 255) wrote ""3e+02"", 
expected ""255"""
  1830, "line 377: put (..., long double = 255) wrote ""3e+02"", 
expected ""255"""
  1831, "line 377: put (..., long double = 127) wrote ""1e+02"", 
expected ""127"""
  1832, "line 377: put (..., long double = 32767) wrote ""3e+04"", 
expected ""32767"""
  1833, "line 377: put (..., long double = 65535) wrote ""7e+04"", 
expected ""65535"""
# +-----------------------+----------+----------+----------+
# | DIAGNOSTIC            |  ACTIVE  |   TOTAL  | INACTIVE |
# +-----------------------+----------+----------+----------+
# | (S1) INFO             |       49 |       49 |       0% |
# | (S2) NOTE             |        2 |        2 |       0% |
# | (S5) WARNING          |        1 |        1 |       0% |
# | (S7) ASSERTION        |       12 |      852 |      98% |
# +-----------------------+----------+----------+----------+

> Notice that the precision field value is 0 in the above cases. The
> resolution for LWG231 says that for floating point types,
> str.precision() is always used in the conversion specification. So each
> of the above tests would need to take this into account.

Yes, although it's not immediately obvious to me what the problem
is, The first assertion above corresponds to this line:

     TEST (T,  1.1, 0, 0, 0, ' ', "", "%g");

I don't see where the test ends up formatting 1.1 as 1.1 using the
"%g" directive (on AIX, printf("%g", 1.1) produces 1 as expected.

Martin

> 
> We are currently consistent with several other implementations. I don't
> believe that either of them implement the LWG231 resolution. The GNU
> implementation does implement the new behavior.
> 
> Travis
> 
> 
> 


RE: [PATCH] punct.cpp

Posted by Travis Vitek <Tr...@roguewave.com>.
 

>Travis Vitek wrote:
>
>>Travis Vitek wrote:
>>
>>>Martin Sebor wrote:
>>>
>>>I don't have the time to do a careful review of the patch WRT
>>>the LWG issue at the moment but I note there's a comment above
>>>the code that's being modified that references the issue, so
>>>it looks like the current code already tries to handle it. If
>>>it does so successfully or not I can't say without spending
>>>more time on it.
>>
>>Here is a quote from the resolution...
>>
>>  For conversion from a floating-point type, str.precision()
>>  is specified in the conversion specification.
>>
>>The following is taken from the comments following that...
>>
>>  The floatfield determines whether numbers are formatted as
>>  if with %f, %e, or %g. If the fixed bit is set, it's %f, if
>>  scientific it's %e, and if both bits are set, or neither,
>>  it's %g.
>>
>>  Turning to the C standard, a precision of 0 is meaningful
>>  for %f and %e. For %g, precision 0 is taken to be the same
>>  as precision 1.
>>
>>Previously, if the precision was 0 and floatfield was not
>>fixed or scientific [i.e. %g] we wouldn't apply precision
>>format string at all. According to this, we should apply
>>precision 0 and assume that the printf() family correctly
>>handles this case, or force precision to 1.
>>
>
>I've tested the patch on AIX, and it fixes failure of regression test
>22.locale.num.put.stdcxx-2. Unfortunately it causes an additional 22
>assertions to fail in 22.locale.num.put.
>
>Travis
>

I took the time to look at this, and I think 22.locale.num.put.cpp is
wrong given the resolution of LWG231. Here are a few of the failing
assertions from 22.locale.num.put...

    //        +-- value to format
    //        |     +-- fmtflags
    //        |     |  +-- precision
    //        |     |  |  +-- field width
    //        |     |  |  |   +-- fill character
    //        |     |  |  |   |   +-- grouping
    //        |     |  |  |   |   |   +-- expected output
    //        |     |  |  |   |   |   |
    //        V     V  V  V   V   V   V

    TEST (T,  1.1,  0, 0, 0, ' ', "", "%g");
    TEST (T,  1.1,  0, 0, 0, ' ', "", "1.1");
    TEST (T, -1.1,  0, 0, 0, ' ', "", "%g");
    TEST (T, -1.1,  0, 0, 0, ' ', "", "-1.1");

    TEST (T,  0.0L, 0, 0, 0, ' ', "", "%Lg");
    TEST (T,  1.0L, 0, 0, 0, ' ', "", "%Lg");
    TEST (T,  2.1L, 0, 0, 0, ' ', "", "%Lg");
    TEST (T, -3.2L, 0, 0, 0, ' ', "", "%Lg");
    TEST (T, -4.3L, 0, 0, 0, ' ', "", "%Lg");
    // there are more...

Notice that the precision field value is 0 in the above cases. The
resolution for LWG231 says that for floating point types,
str.precision() is always used in the conversion specification. So each
of the above tests would need to take this into account.

We are currently consistent with several other implementations. I don't
believe that either of them implement the LWG231 resolution. The GNU
implementation does implement the new behavior.

Travis