diff mbox

[v2] libstdc++: Add hexfloat/defaultfloat io manipulators.

Message ID 20141006155615.GN4197@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely Oct. 6, 2014, 3:56 p.m. UTC
On 28/03/14 08:56 +0900, Luke Allardyce wrote:
>It looks like the new standard also requires the precision to be
>ignored for hexfloat
>
>>For conversion from a floating-point type, if floatfield != (ios_base::fixed | ios_base:: scientific), str.precision() is specified as precision in the conversion specification. Otherwise, no precision is specified.

I've made this change and adjusted the test so that it doesn't make
any assumptions about the exact format of hexadecimal float output, as
it's unspecified whether you get e.g. 0x1.ep+3 or 0xfp+0 for 15.

As Luke pointed out, this changes the behaviour of some valid C++03
programs, but I think that's the right thing to do in this case. I'll
document that in the release notes.

Tested x86_64-linux, committed to trunk.

Comments

Andreas Schwab Oct. 7, 2014, 7:10 p.m. UTC | #1
Jonathan Wakely <jwakely@redhat.com> writes:

> diff --git a/libstdc++-v3/src/c++98/locale_facets.cc b/libstdc++-v3/src/c++98/locale_facets.cc
> index 3669acb..7ed04e6 100644
> --- a/libstdc++-v3/src/c++98/locale_facets.cc
> +++ b/libstdc++-v3/src/c++98/locale_facets.cc
> @@ -69,19 +69,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>      if (__flags & ios_base::showpoint)
>        *__fptr++ = '#';
>  
> -    // As per DR 231: _always_, not only when 
> -    // __flags & ios_base::fixed || __prec > 0
> -    *__fptr++ = '.';
> -    *__fptr++ = '*';
> +    ios_base::fmtflags __fltfield = __flags & ios_base::floatfield;
> +
> +    if (__fltfield != (ios_base::fixed | ios_base::scientific))
> +      {
> +        // As per DR 231: not only when __flags & ios_base::fixed || __prec > 0
> +        *__fptr++ = '.';
> +        *__fptr++ = '*';
> +      }
>  
>      if (__mod)
>        *__fptr++ = __mod;
> -    ios_base::fmtflags __fltfield = __flags & ios_base::floatfield;
>      // [22.2.2.2.2] Table 58
>      if (__fltfield == ios_base::fixed)
>        *__fptr++ = 'f';
>      else if (__fltfield == ios_base::scientific)
>        *__fptr++ = (__flags & ios_base::uppercase) ? 'E' : 'e';
> +#ifdef _GLIBCXX_USE_C99
> +    else if (__fltfield == (ios_base::fixed | ios_base::scientific))
> +      *__fptr++ = (__flags & ios_base::uppercase) ? 'A' : 'a';
> +#endif

That cannot work.  std::__convert_from_v always passes __prec before
__v, but the format is "%a".

Andreas.
Rainer Orth Oct. 8, 2014, 11:27 a.m. UTC | #2
Jonathan Wakely <jwakely@redhat.com> writes:

> On 28/03/14 08:56 +0900, Luke Allardyce wrote:
>>It looks like the new standard also requires the precision to be
>>ignored for hexfloat
>>
>>>For conversion from a floating-point type, if floatfield !=
>> (ios_base::fixed | ios_base:: scientific), str.precision() is specified
>> as precision in the conversion specification. Otherwise, no precision is
>> specified.
>
> I've made this change and adjusted the test so that it doesn't make
> any assumptions about the exact format of hexadecimal float output, as
> it's unspecified whether you get e.g. 0x1.ep+3 or 0xfp+0 for 15.
>
> As Luke pointed out, this changes the behaviour of some valid C++03
> programs, but I think that's the right thing to do in this case. I'll
> document that in the release notes.
>
> Tested x86_64-linux, committed to trunk.

On Solaris 11 (both SPARC and x86), the test execution FAILs:

Assertion failed: os && std::stod(os.str()) == d, file /vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc, line 51, function test01
FAIL: 27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc execution test

With -DTEST_NUMPUT_VERBOSE, I get

got: 0x1.0000000000000p-1074

	Rainer
Jonathan Wakely Oct. 8, 2014, 11:37 a.m. UTC | #3
On 08/10/14 13:27 +0200, Rainer Orth wrote:
>On Solaris 11 (both SPARC and x86), the test execution FAILs:
>
>Assertion failed: os && std::stod(os.str()) == d, file /vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc, line 51, function test01
>FAIL: 27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc execution test
>
>With -DTEST_NUMPUT_VERBOSE, I get
>
>got: 0x1.0000000000000p-1074

Thanks, I'll look into that too.
Jonathan Wakely Oct. 8, 2014, 1:57 p.m. UTC | #4
On 08/10/14 12:37 +0100, Jonathan Wakely wrote:
>On 08/10/14 13:27 +0200, Rainer Orth wrote:
>>On Solaris 11 (both SPARC and x86), the test execution FAILs:
>>
>>Assertion failed: os && std::stod(os.str()) == d, file /vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc, line 51, function test01
>>FAIL: 27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc execution test
>>
>>With -DTEST_NUMPUT_VERBOSE, I get
>>
>>got: 0x1.0000000000000p-1074
>
>Thanks, I'll look into that too.

I suspect this was the bug Andreas pointed out (which didn't show up
on x86_64-linux)  and so should be fixed now.
Rainer Orth Oct. 8, 2014, 2:21 p.m. UTC | #5
Jonathan Wakely <jwakely@redhat.com> writes:

> On 08/10/14 12:37 +0100, Jonathan Wakely wrote:
>>On 08/10/14 13:27 +0200, Rainer Orth wrote:
>>>On Solaris 11 (both SPARC and x86), the test execution FAILs:
>>>
>>>Assertion failed: os && std::stod(os.str()) == d, file
>>> /vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc,
>>> line 51, function test01
>>>FAIL: 27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc execution test
>>>
>>>With -DTEST_NUMPUT_VERBOSE, I get
>>>
>>>got: 0x1.0000000000000p-1074
>>
>>Thanks, I'll look into that too.
>
> I suspect this was the bug Andreas pointed out (which didn't show up
> on x86_64-linux)  and so should be fixed now.

I'd already tried that patch before you committed it, and unfortunately
it didn't help.

	Rainer
Jonathan Wakely Oct. 8, 2014, 5:44 p.m. UTC | #6
On 08/10/14 16:21 +0200, Rainer Orth wrote:
>I'd already tried that patch before you committed it, and unfortunately
>it didn't help.

Oh dear. Did you do a clean build, or at least
'touch libstdc++-v3/src/*/*.cc' to ensure the library sources get
rebuilt?  Changes to headers do not trigger everything to be rebuilt,
because we don't have proper dependencies in the makefiles.

If it's still happening I'll have to hope I can reproduce it on one of
the targets I have access to and debug it there.
Rainer Orth Oct. 8, 2014, 5:58 p.m. UTC | #7
Jonathan Wakely <jwakely@redhat.com> writes:

> On 08/10/14 16:21 +0200, Rainer Orth wrote:
>>I'd already tried that patch before you committed it, and unfortunately
>>it didn't help.
>
> Oh dear. Did you do a clean build, or at least
> 'touch libstdc++-v3/src/*/*.cc' to ensure the library sources get
> rebuilt?  Changes to headers do not trigger everything to be rebuilt,
> because we don't have proper dependencies in the makefiles.

Not before, because I didn't realize that the .tcc file is effectively a
header.

> If it's still happening I'll have to hope I can reproduce it on one of
> the targets I have access to and debug it there.

I've now done a clean rebuild (make clean; make) and the failure
remains.

There's been talk about getting Solaris into the compile farm, but I
haven't pursued that yet.

	Rainer
diff mbox

Patch

commit 7090c2fca0e3858ea527b66a1eb44fd504f1ba4e
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Sep 22 23:10:54 2014 +0100

    2014-10-06  RĂ¼diger Sonderfeld  <ruediger@c-plusplus.de>
    	    Jonathan Wakely  <jwakely@redhat.com>
    
    	PR libstdc++/59987
    	* doc/xml/manual/status_cxx2011.xml: Remove hexfloat from notes.
    	* doc/html/manual/status.html: Regenerate.
    	* include/bits/ios_base.h (hexfloat): New function.
    	(defaultfloat): New function.
    	* src/c++98/locale_facets.cc (__num_base::_S_format_float): Support
    	hexadecimal floating point format.
    	* testsuite/27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc:
    	New file.

diff --git a/libstdc++-v3/doc/xml/manual/status_cxx2011.xml b/libstdc++-v3/doc/xml/manual/status_cxx2011.xml
index 5b36556..108de36 100644
--- a/libstdc++-v3/doc/xml/manual/status_cxx2011.xml
+++ b/libstdc++-v3/doc/xml/manual/status_cxx2011.xml
@@ -2131,7 +2131,6 @@  particular release.
       <entry>
         Missing <code>io_errc</code> and <code>iostream_category</code>.
         <code>ios_base::failure</code> is not derived from <code>system_error</code>.
-	Missing <code>ios_base::hexfloat</code>.
       </entry>
     </row>
     <row>
diff --git a/libstdc++-v3/include/bits/ios_base.h b/libstdc++-v3/include/bits/ios_base.h
index fb448fd..5e33b81 100644
--- a/libstdc++-v3/include/bits/ios_base.h
+++ b/libstdc++-v3/include/bits/ios_base.h
@@ -984,6 +984,27 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     return __base;
   }
 
+#if __cplusplus >= 201103L
+  // New C++11 floatfield manipulators
+
+  /// Calls
+  /// base.setf(ios_base::fixed|ios_base::scientific, ios_base::floatfield)
+  inline ios_base&
+  hexfloat(ios_base& __base)
+  {
+    __base.setf(ios_base::fixed | ios_base::scientific, ios_base::floatfield);
+    return __base;
+  }
+
+  /// Calls @c base.unsetf(ios_base::floatfield)
+  inline ios_base&
+  defaultfloat(ios_base& __base)
+  {
+    __base.unsetf(ios_base::floatfield);
+    return __base;
+  }
+#endif
+
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace
 
diff --git a/libstdc++-v3/src/c++98/locale_facets.cc b/libstdc++-v3/src/c++98/locale_facets.cc
index 3669acb..7ed04e6 100644
--- a/libstdc++-v3/src/c++98/locale_facets.cc
+++ b/libstdc++-v3/src/c++98/locale_facets.cc
@@ -69,19 +69,26 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     if (__flags & ios_base::showpoint)
       *__fptr++ = '#';
 
-    // As per DR 231: _always_, not only when 
-    // __flags & ios_base::fixed || __prec > 0
-    *__fptr++ = '.';
-    *__fptr++ = '*';
+    ios_base::fmtflags __fltfield = __flags & ios_base::floatfield;
+
+    if (__fltfield != (ios_base::fixed | ios_base::scientific))
+      {
+        // As per DR 231: not only when __flags & ios_base::fixed || __prec > 0
+        *__fptr++ = '.';
+        *__fptr++ = '*';
+      }
 
     if (__mod)
       *__fptr++ = __mod;
-    ios_base::fmtflags __fltfield = __flags & ios_base::floatfield;
     // [22.2.2.2.2] Table 58
     if (__fltfield == ios_base::fixed)
       *__fptr++ = 'f';
     else if (__fltfield == ios_base::scientific)
       *__fptr++ = (__flags & ios_base::uppercase) ? 'E' : 'e';
+#ifdef _GLIBCXX_USE_C99
+    else if (__fltfield == (ios_base::fixed | ios_base::scientific))
+      *__fptr++ = (__flags & ios_base::uppercase) ? 'A' : 'a';
+#endif
     else
       *__fptr++ = (__flags & ios_base::uppercase) ? 'G' : 'g';
     *__fptr = '\0';
diff --git a/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc b/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc
new file mode 100644
index 0000000..485a485
--- /dev/null
+++ b/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc
@@ -0,0 +1,152 @@ 
+// { dg-options "-std=gnu++11" }
+
+// 2014-03-27 RĂ¼diger Sonderfeld
+// test the hexadecimal floating point inserters (facet num_put)
+
+// Copyright (C) 2014 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+#include <iomanip>
+#include <sstream>
+#include <string>
+#include <testsuite_hooks.h>
+
+#ifndef _GLIBCXX_ASSERT
+#  define TEST_NUMPUT_VERBOSE 1
+#endif
+
+#ifdef TEST_NUMPUT_VERBOSE
+#  include <iostream>
+#endif
+
+using namespace std;
+
+void
+test01()
+{
+  ostringstream os;
+  double d = 272.; // 0x1.1p+8;
+#ifdef TEST_NUMPUT_VERBOSE
+  cout << os.precision() << endl;
+#endif
+  os << hexfloat << setprecision(1);
+  os << d;
+#ifdef TEST_NUMPUT_VERBOSE
+  cout << "got: " << os.str() << endl;
+#endif
+  VERIFY( os && std::stod(os.str()) == d );
+  VERIFY( os.str().substr(0, 2) == "0x" );
+  VERIFY( os.str().find('p') != std::string::npos );
+
+  os.str("");
+  os << uppercase << d;
+#ifdef TEST_NUMPUT_VERBOSE
+  cout << "got: " << os.str() << endl;
+#endif
+  VERIFY( os && std::stod(os.str()) == d );
+  VERIFY( os.str().substr(0, 2) == "0X" );
+  VERIFY( os.str().find('P') != std::string::npos );
+
+  os << nouppercase;
+  os.str("");
+  os << defaultfloat << setprecision(6);
+  os << d;
+#ifdef TEST_NUMPUT_VERBOSE
+  cout << "got: " << os.str() << endl;
+#endif
+  VERIFY( os && os.str() == "272" );
+
+  os.str("");
+  d = 15.; //0x1.ep+3;
+  os << hexfloat << setprecision(1);
+  os << d;
+#ifdef TEST_NUMPUT_VERBOSE
+  cout << "got: " << os.str() << endl;
+#endif
+  VERIFY( os && std::stod(os.str()) == d );
+  os.str("");
+  os << uppercase << d;
+#ifdef TEST_NUMPUT_VERBOSE
+  cout << "got: " << os.str() << endl;
+#endif
+  VERIFY( os && std::stod(os.str()) == d );
+  os << nouppercase;
+  os.str("");
+  os << defaultfloat << setprecision(6);
+  os << d;
+#ifdef TEST_NUMPUT_VERBOSE
+  cout << "got: " << os.str() << endl;
+#endif
+  VERIFY( os && os.str() == "15" );
+}
+
+void
+test02()
+{
+  ostringstream os;
+  long double d = 272.L; // 0x1.1p+8L;
+  os << hexfloat << setprecision(1);
+  os << d;
+#ifdef TEST_NUMPUT_VERBOSE
+  cout << "got: " << os.str() << endl;
+#endif
+  VERIFY( os && std::stold(os.str()) == d );
+  os.str("");
+  os << uppercase << d;
+#ifdef TEST_NUMPUT_VERBOSE
+  cout << "got: " << os.str() << endl;
+#endif
+  VERIFY( os && std::stold(os.str()) == d );
+  os << nouppercase;
+  os.str("");
+  os << defaultfloat << setprecision(6);
+  os << d;
+#ifdef TEST_NUMPUT_VERBOSE
+  cout << "got: " << os.str() << endl;
+#endif
+  VERIFY( os && os.str() == "272" );
+
+  os.str("");
+  os << hexfloat << setprecision(1);
+  d = 15.; //0x1.ep+3;
+  os << d;
+#ifdef TEST_NUMPUT_VERBOSE
+  cout << "got: " << os.str() << endl;
+#endif
+  VERIFY( os && std::stold(os.str()) == d );
+  os.str("");
+  os << uppercase << d;
+#ifdef TEST_NUMPUT_VERBOSE
+  cout << "got: " << os.str() << endl;
+#endif
+  VERIFY( os && std::stold(os.str()) == d );
+  os << nouppercase;
+  os.str("");
+  os << defaultfloat << setprecision(6);
+  os << d;
+#ifdef TEST_NUMPUT_VERBOSE
+  cout << "got: " << os.str() << endl;
+#endif
+  VERIFY( os && os.str() == "15" );
+}
+
+int
+main()
+{
+  test01();
+  test02();
+}