Discussion:
[PATCH] libstdc++: Add hexfloat/defaultfloat io manipulators.
Rüdiger Sonderfeld
2014-03-27 11:52:10 UTC
Permalink
* 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.

hexfloat/defaultfloat are new iostream manipulators introduced in C++11.
See the changes in [locale.nm.put] (§22.4.2.2) and
[floatfield.manip] (§27.5.6.4). This patch does not add input support
for hexfloats. The effect of outputting hexadecimal floating points by
setting both fixed and scientific is also added in C++98. I am not sure
how to change this except for adding a C++11 specific implementation of
`__num_base::_S_format_float'. But since the C++11 standard explicitly
says that "C++2003 gives no meaning to the combination of fixed and
scientific," this might be acceptable anyway.

I have signed the FSF papers.

Signed-off-by: Rüdiger Sonderfeld <***@c-plusplus.de>
---
libstdc++-v3/ChangeLog | 9 ++
libstdc++-v3/include/bits/ios_base.h | 20 +++
libstdc++-v3/src/c++98/locale_facets.cc | 2 +
.../inserters_arithmetic/char/hexfloat.cc | 141
+++++++++++++++++++++
4 files changed, 172 insertions(+)
create mode 100644 libstdc++-
v3/testsuite/27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index f6008d1..3345d12 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,12 @@
+2014-03-27 Rüdiger Sonderfeld <***@c-plusplus.de>
+
+ * 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.
+
2014-03-25 Jonathan Wakely <***@redhat.com>

PR libstdc++/60658
diff --git a/libstdc++-v3/include/bits/ios_base.h b/libstdc++-
v3/include/bits/ios_base.h
index ae856de..8f263c1 100644
--- a/libstdc++-v3/include/bits/ios_base.h
+++ b/libstdc++-v3/include/bits/ios_base.h
@@ -969,6 +969,26 @@ namespace std _GLIBCXX_VISIBILITY(default)
return __base;
}

+#if __cplusplus >= 201103L
+ // New floatingfield 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 base.unsetf(ios_base::floatfield)
+ inline ios_base&
+ defaultfloat(ios_base& __base)
+ {
+ __base.unsetf(ios_base::floatfield);
+ return __base;
+ }
+#endif // __cplusplus >= 201103L
+
_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..9455f42 100644
--- a/libstdc++-v3/src/c++98/locale_facets.cc
+++ b/libstdc++-v3/src/c++98/locale_facets.cc
@@ -82,6 +82,8 @@ namespace std _GLIBCXX_VISIBILITY(default)
*__fptr++ = 'f';
else if (__fltfield == ios_base::scientific)
*__fptr++ = (__flags & ios_base::uppercase) ? 'E' : 'e';
+ else if (__fltfield == (ios_base::fixed | ios_base::scientific))
+ *__fptr++ = (__flags & ios_base::uppercase) ? 'A' : 'a';
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..b0f5724
--- /dev/null
+++ b/libstdc++-
v3/testsuite/27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc
@@ -0,0 +1,141 @@
+// { dg-options "-std=gnu++0x" }
+
+// 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 <iostream>
+#include <iomanip>
+#include <sstream>
+#include <limits>
+#include <testsuite_hooks.h>
+
+using namespace std;
+
+//#ifndef _GLIBCXX_ASSERT
+# define TEST_NUMPUT_VERBOSE 1
+//#endif
+
+void
+test01()
+{
+ {
+ ostringstream os;
+ double d = 272.; // 0x1.1p+8;
+ cout << os.precision() << endl;
+ os << hexfloat << setprecision(1);
+ os << d;
+#ifdef TEST_NUMPUT_VERBOSE
+ cout << "got: " << os.str() << endl;
+#endif
+ VERIFY( os && os.str() == "0x1.1p+8" );
+ os.str("");
+ os << uppercase << d;
+#ifdef TEST_NUMPUT_VERBOSE
+ cout << "got: " << os.str() << endl;
+#endif
+ VERIFY( os && os.str() == "0X1.1P+8" );
+ 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 && os.str() == "0x1.ep+3" );
+ os.str("");
+ os << uppercase << d;
+#ifdef TEST_NUMPUT_VERBOSE
+ cout << "got: " << os.str() << endl;
+#endif
+ VERIFY( os && os.str() == "0X1.EP+3" );
+ 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" );
+ }
+
+ {
+ 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 && os.str() == "0x8.8p+5" );
+ os.str("");
+ os << uppercase << d;
+#ifdef TEST_NUMPUT_VERBOSE
+ cout << "got: " << os.str() << endl;
+#endif
+ VERIFY( os && os.str() == "0X8.8P+5" );
+ 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 && os.str() == "0xf.0p+0" );
+ os.str("");
+ os << uppercase << d;
+#ifdef TEST_NUMPUT_VERBOSE
+ cout << "got: " << os.str() << endl;
+#endif
+ VERIFY( os && os.str() == "0XF.0P+0" );
+ 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();
+ return 0;
+}
--
1.9.1
Jonathan Wakely
2014-03-27 13:37:23 UTC
Permalink
Post by Rüdiger Sonderfeld
* include/bits/ios_base.h (hexfloat): New function.
(defaultfloat): New function.
Support hexadecimal floating point format.
New file.
Thanks for the patch. I have a few comments inline below.

With a few tweaks I think this can be committed once the trunk
re-opens for stage 1 <http://gcc.gnu.org/develop.html#stage1>
Post by Rüdiger Sonderfeld
hexfloat/defaultfloat are new iostream manipulators introduced in C++11.
See the changes in [locale.nm.put] (§22.4.2.2) and
[floatfield.manip] (§27.5.6.4). This patch does not add input support
for hexfloats. The effect of outputting hexadecimal floating points by
setting both fixed and scientific is also added in C++98. I am not sure
how to change this except for adding a C++11 specific implementation of
`__num_base::_S_format_float'. But since the C++11 standard explicitly
says that "C++2003 gives no meaning to the combination of fixed and
scientific," this might be acceptable anyway.
Yes, I think that's OK, thanks for explaining it.

We could document that (fixed|scientific) has that effect in c++98
mode. I'll also need to update the C++11 status table in the manual to
note std::hexfloat works for output streams.
Post by Rüdiger Sonderfeld
I have signed the FSF papers.
Excellent, that's the hardest part of contributing.
Post by Rüdiger Sonderfeld
+
+ * include/bits/ios_base.h (hexfloat): New function.
+ (defaultfloat): New function.
+ Support hexadecimal floating point format.
+ New file.
N.B. patches to the ChangeLog rarely apply cleanly (because someone
else may have changed the ChangeLog since the patch was created) so
the convention is to send the ChangeLog entry in the email body, or as
a separate attachment, or by using 'git log -p @{u}..' so the commit
log shows it, rather than as part of the patch.
Post by Rüdiger Sonderfeld
diff --git a/libstdc++-v3/include/bits/ios_base.h b/libstdc++-
v3/include/bits/ios_base.h
index ae856de..8f263c1 100644
--- a/libstdc++-v3/include/bits/ios_base.h
+++ b/libstdc++-v3/include/bits/ios_base.h
@@ -969,6 +969,26 @@ namespace std _GLIBCXX_VISIBILITY(default)
return __base;
}
+#if __cplusplus >= 201103L
+ // New floatingfield manipulators
It's only a comment, but that should be "floatfield"
Post by Rüdiger Sonderfeld
+
+ /// Calls base.setf(ios_base::fixed|
ios_base::scientific,ios_base::floatfield)
Does this line exceed 80 characters?
Post by Rüdiger Sonderfeld
diff --git a/libstdc++-v3/src/c++98/locale_facets.cc b/libstdc++-
v3/src/c++98/locale_facets.cc
index 3669acb..9455f42 100644
--- a/libstdc++-v3/src/c++98/locale_facets.cc
+++ b/libstdc++-v3/src/c++98/locale_facets.cc
@@ -82,6 +82,8 @@ namespace std _GLIBCXX_VISIBILITY(default)
*__fptr++ = 'f';
else if (__fltfield == ios_base::scientific)
*__fptr++ = (__flags & ios_base::uppercase) ? 'E' : 'e';
+ else if (__fltfield == (ios_base::fixed | ios_base::scientific))
+ *__fptr++ = (__flags & ios_base::uppercase) ? 'A' : 'a';
else
*__fptr++ = (__flags & ios_base::uppercase) ? 'G' : 'g';
*__fptr = '\0';
I wonder if we need a config macro to test whether the libc's
vsnprintf supports the A conversion specifier. You could do:

#ifdef _GLIBCXX_USE_C99
else if (__fltfield == (ios_base::fixed | ios_base::scientific))
*__fptr++ = (__flags & ios_base::uppercase) ? 'A' : 'a';
#endif

which I think would be correct for now (I'm planning to overhaul how we
use _GLIBCXX_USE_C99 later this year).
Post by Rüdiger Sonderfeld
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..b0f5724
--- /dev/null
+++ b/libstdc++-
v3/testsuite/27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc
@@ -0,0 +1,141 @@
+// { dg-options "-std=gnu++0x" }
New tests should use -std=gnu++11, the gnu++0x option is deprecated so
will stop working at some point (and we'll have to update the existing
tests using it).
Post by Rüdiger Sonderfeld
+//#ifndef _GLIBCXX_ASSERT
+# define TEST_NUMPUT_VERBOSE 1
+//#endif
This appears to make the test write to std::cout by default, did you
mean to remove that before submitting the patch?

Thanks again for contributing this to the library!
Rüdiger Sonderfeld
2014-03-27 16:00:19 UTC
Permalink
Hello Jonathan,

thanks for your comments.
Post by Jonathan Wakely
N.B. patches to the ChangeLog rarely apply cleanly (because someone
else may have changed the ChangeLog since the patch was created) so
the convention is to send the ChangeLog entry in the email body, or as
log shows it, rather than as part of the patch.
Yes, ChangeLog's can be a bit of a pain. I removed the ChangeLog from the
patch. But FYI there is a ChangeLog merge driver hidden in gnulib, which
can be helpful when dealing with ChangeLog files in git (and potentially
other version control systems)

http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/git-merge-changelog.c
Post by Jonathan Wakely
We could document that (fixed|scientific) has that effect in c++98
mode.
Where should it be documented?

Regards,
Rüdiger

-- 8< ---------------------------------------------------------- >8 --

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

Signed-off-by: Rüdiger Sonderfeld <***@c-plusplus.de>
---
libstdc++-v3/include/bits/ios_base.h | 21 +++
libstdc++-v3/src/c++98/locale_facets.cc | 4 +
.../inserters_arithmetic/char/hexfloat.cc | 141 +++++++++++++++++++++
3 files changed, 166 insertions(+)
create mode 100644 libstdc++-v3/testsuite/27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc

diff --git a/libstdc++-v3/include/bits/ios_base.h b/libstdc++-v3/include/bits/ios_base.h
index ae856de..b7fae43 100644
--- a/libstdc++-v3/include/bits/ios_base.h
+++ b/libstdc++-v3/include/bits/ios_base.h
@@ -969,6 +969,27 @@ namespace std _GLIBCXX_VISIBILITY(default)
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 base.unsetf(ios_base::floatfield).
+ inline ios_base&
+ defaultfloat(ios_base& __base)
+ {
+ __base.unsetf(ios_base::floatfield);
+ return __base;
+ }
+#endif // __cplusplus >= 201103L
+
_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..a21d665 100644
--- a/libstdc++-v3/src/c++98/locale_facets.cc
+++ b/libstdc++-v3/src/c++98/locale_facets.cc
@@ -82,6 +82,10 @@ namespace std _GLIBCXX_VISIBILITY(default)
*__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..55c46ad
--- /dev/null
+++ b/libstdc++-v3/testsuite/27_io/basic_ostream/inserters_arithmetic/char/hexfloat.cc
@@ -0,0 +1,141 @@
+// { 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 <iostream>
+#include <iomanip>
+#include <sstream>
+#include <limits>
+#include <testsuite_hooks.h>
+
+using namespace std;
+
+#ifndef _GLIBCXX_ASSERT
+# define TEST_NUMPUT_VERBOSE 1
+#endif
+
+void
+test01()
+{
+ {
+ ostringstream os;
+ double d = 272.; // 0x1.1p+8;
+ cout << os.precision() << endl;
+ os << hexfloat << setprecision(1);
+ os << d;
+#ifdef TEST_NUMPUT_VERBOSE
+ cout << "got: " << os.str() << endl;
+#endif
+ VERIFY( os && os.str() == "0x1.1p+8" );
+ os.str("");
+ os << uppercase << d;
+#ifdef TEST_NUMPUT_VERBOSE
+ cout << "got: " << os.str() << endl;
+#endif
+ VERIFY( os && os.str() == "0X1.1P+8" );
+ 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 && os.str() == "0x1.ep+3" );
+ os.str("");
+ os << uppercase << d;
+#ifdef TEST_NUMPUT_VERBOSE
+ cout << "got: " << os.str() << endl;
+#endif
+ VERIFY( os && os.str() == "0X1.EP+3" );
+ 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" );
+ }
+
+ {
+ 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 && os.str() == "0x8.8p+5" );
+ os.str("");
+ os << uppercase << d;
+#ifdef TEST_NUMPUT_VERBOSE
+ cout << "got: " << os.str() << endl;
+#endif
+ VERIFY( os && os.str() == "0X8.8P+5" );
+ 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 && os.str() == "0xf.0p+0" );
+ os.str("");
+ os << uppercase << d;
+#ifdef TEST_NUMPUT_VERBOSE
+ cout << "got: " << os.str() << endl;
+#endif
+ VERIFY( os && os.str() == "0XF.0P+0" );
+ 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();
+ return 0;
+}
--
1.9.1
Jonathan Wakely
2014-03-27 16:27:14 UTC
Permalink
Post by Rüdiger Sonderfeld
Hello Jonathan,
thanks for your comments.
Post by Jonathan Wakely
N.B. patches to the ChangeLog rarely apply cleanly (because someone
else may have changed the ChangeLog since the patch was created) so
the convention is to send the ChangeLog entry in the email body, or as
log shows it, rather than as part of the patch.
Yes, ChangeLog's can be a bit of a pain. I removed the ChangeLog from the
patch. But FYI there is a ChangeLog merge driver hidden in gnulib, which
can be helpful when dealing with ChangeLog files in git (and potentially
other version control systems)
http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/git-merge-changelog.c
Yes, I use that myself, and generate patches with the 'git lgp'
command shown at http://gcc.gnu.org/wiki/GitMirror
Post by Rüdiger Sonderfeld
Post by Jonathan Wakely
We could document that (fixed|scientific) has that effect in c++98
mode.
Where should it be documented?
Probably somewhere in doc/xml/manual/io.xml but I'm happy to do that
once the patch is committed if you like.

Thanks for the updated patch, I will try to remember to commit it when
stage 1 starts. If you don't get a mail from me then please ping me as
a reminder.
Luke Allardyce
2014-03-27 23:56:36 UTC
Permalink
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.
So perhaps something like

Index: libstdc++-v3/src/c++98/locale_facets.cc
===================================================================
--- libstdc++-v3/src/c++98/locale_facets.cc (リビジョン 208802)
+++ libstdc++-v3/src/c++98/locale_facets.cc (作業コピー)
@@ -69,19 +69,23 @@
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)) {
+ // Precision used if flags != hexfloat
+ *__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';
+ else if (__fltfield == ios_base::fixed | ios_base::scientific)
+ *__fptr++ = (__flags & ios_base::uppercase) ? 'A' : 'a';
else
*__fptr++ = (__flags & ios_base::uppercase) ? 'G' : 'g';
*__fptr = '\0';


Also the old standard seems to require that ios_base::fixed |
ios_base::scientific (or any other combination) falls through to the
uppercase test; I was trying to use abi_tag for a solution as not only
would two versions of _S_format_float be necessary, but also num_get
due to the pre-instantiated templates for <char> and <wchar>, which
led me to http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60642. It might
just be more trouble than it's worth.
Post by Rüdiger Sonderfeld
Hello Jonathan,
thanks for your comments.
Post by Jonathan Wakely
N.B. patches to the ChangeLog rarely apply cleanly (because someone
else may have changed the ChangeLog since the patch was created) so
the convention is to send the ChangeLog entry in the email body, or as
log shows it, rather than as part of the patch.
Yes, ChangeLog's can be a bit of a pain. I removed the ChangeLog from the
patch. But FYI there is a ChangeLog merge driver hidden in gnulib, which
can be helpful when dealing with ChangeLog files in git (and potentially
other version control systems)
http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/git-merge-changelog.c
Yes, I use that myself, and generate patches with the 'git lgp'
command shown at http://gcc.gnu.org/wiki/GitMirror
Post by Rüdiger Sonderfeld
Post by Jonathan Wakely
We could document that (fixed|scientific) has that effect in c++98
mode.
Where should it be documented?
Probably somewhere in doc/xml/manual/io.xml but I'm happy to do that
once the patch is committed if you like.
Thanks for the updated patch, I will try to remember to commit it when
stage 1 starts. If you don't get a mail from me then please ping me as
a reminder.
Jonathan Wakely
2014-04-15 10:13:58 UTC
Permalink
Post by Luke Allardyce
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.
Thanks for pointing out that difference. We'll need a test for that.
Post by Luke Allardyce
Also the old standard seems to require that ios_base::fixed |
ios_base::scientific (or any other combination) falls through to the
uppercase test; I was trying to use abi_tag for a solution as not only
would two versions of _S_format_float be necessary, but also num_get
due to the pre-instantiated templates for <char> and <wchar>, which
led me to http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60642. It might
just be more trouble than it's worth.
I don't think we need to worry about that, if I understand correctly
the combination of fixed|scientific has unspecified behaviour in
C++03, so we can make our implementation do exactly what it does in
C++11.
Jonathan Wakely
2014-10-06 15:56:15 UTC
Permalink
Post by Luke Allardyce
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.
Andreas Schwab
2014-10-07 19:10:12 UTC
Permalink
Post by Rüdiger Sonderfeld
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.
--
Andreas Schwab, ***@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Jonathan Wakely
2014-10-08 10:07:47 UTC
Permalink
Post by Andreas Schwab
That cannot work. std::__convert_from_v always passes __prec before
__v, but the format is "%a".
Ah yes. I'm testing this fix now.
Jonathan Wakely
2014-10-08 13:55:59 UTC
Permalink
Post by Jonathan Wakely
Post by Andreas Schwab
That cannot work. std::__convert_from_v always passes __prec before
__v, but the format is "%a".
Ah yes. I'm testing this fix now.
Committed.

I assume it was working for me because __convert_from_v("%a", 6, 272.)
happens to pass the double differently from the integer, and vsnprintf
looks for a double in the relevant register and ignores the integer
argument.
Post by Jonathan Wakely
commit 543771e2db1642715854ae4bec81d803ca8e2e59
Date: Wed Oct 8 10:39:27 2014 +0100
* include/bits/locale_facets.tcc (num_put::_M_insert_float): Do not
pass precision when using hexfloat format.
* src/c++98/locale_facets.cc (__num_base::_S_format_float): Always
output precision if C99 hexfloat conversion specifiers not available.
diff --git a/libstdc++-v3/include/bits/locale_facets.tcc b/libstdc++-v3/include/bits/locale_facets.tcc
index cf12a08..88adc0d 100644
--- a/libstdc++-v3/include/bits/locale_facets.tcc
+++ b/libstdc++-v3/include/bits/locale_facets.tcc
@@ -988,20 +988,32 @@ _GLIBCXX_BEGIN_NAMESPACE_LDBL
__num_base::_S_format_float(__io, __fbuf, __mod);
#ifdef _GLIBCXX_USE_C99
+ // Precision is always used except for hexfloat format.
+ const bool __use_prec =
+ (__io.flags() & ios_base::floatfield) != ios_base::floatfield;
+
// First try a buffer perhaps big enough (most probably sufficient
// for non-ios_base::fixed outputs)
int __cs_size = __max_digits * 3;
char* __cs = static_cast<char*>(__builtin_alloca(__cs_size));
- __len = std::__convert_from_v(_S_get_c_locale(), __cs, __cs_size,
- __fbuf, __prec, __v);
+ if (__use_prec)
+ __len = std::__convert_from_v(_S_get_c_locale(), __cs, __cs_size,
+ __fbuf, __prec, __v);
+ else
+ __len = std::__convert_from_v(_S_get_c_locale(), __cs, __cs_size,
+ __fbuf, __v);
// If the buffer was not large enough, try again with the correct size.
if (__len >= __cs_size)
{
__cs_size = __len + 1;
__cs = static_cast<char*>(__builtin_alloca(__cs_size));
- __len = std::__convert_from_v(_S_get_c_locale(), __cs, __cs_size,
- __fbuf, __prec, __v);
+ if (__use_prec)
+ __len = std::__convert_from_v(_S_get_c_locale(), __cs, __cs_size,
+ __fbuf, __prec, __v);
+ else
+ __len = std::__convert_from_v(_S_get_c_locale(), __cs, __cs_size,
+ __fbuf, __v);
}
#else
// Consider the possibility of long ios_base::fixed outputs
diff --git a/libstdc++-v3/src/c++98/locale_facets.cc b/libstdc++-v3/src/c++98/locale_facets.cc
index 7ed04e6..b3ca5dc 100644
--- a/libstdc++-v3/src/c++98/locale_facets.cc
+++ b/libstdc++-v3/src/c++98/locale_facets.cc
@@ -71,7 +71,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
ios_base::fmtflags __fltfield = __flags & ios_base::floatfield;
+#ifdef _GLIBCXX_USE_C99
+ // Precision is always used except for hexfloat format.
if (__fltfield != (ios_base::fixed | ios_base::scientific))
+#endif
{
// As per DR 231: not only when __flags & ios_base::fixed || __prec > 0
*__fptr++ = '.';
Rainer Orth
2014-10-08 11:27:32 UTC
Permalink
Post by Jonathan Wakely
Post by Luke Allardyce
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
--
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
Jonathan Wakely
2014-10-08 11:37:26 UTC
Permalink
Post by Rainer Orth
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
2014-10-08 13:57:12 UTC
Permalink
Post by Jonathan Wakely
Post by Rainer Orth
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
2014-10-08 14:21:55 UTC
Permalink
Post by Jonathan Wakely
Post by Jonathan Wakely
Post by Rainer Orth
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
--
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
Jonathan Wakely
2014-10-08 17:44:16 UTC
Permalink
Post by Rainer Orth
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
2014-10-08 17:58:52 UTC
Permalink
Post by Jonathan Wakely
Post by Rainer Orth
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.
Post by Jonathan Wakely
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
--
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
Loading...