This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On 27/03/14 12:52 +0100, Rüdiger Sonderfeld wrote:
* 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.

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>

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.

I have signed the FSF papers.

Excellent, that's the hardest part of contributing.

+2014-03-27  Rüdiger Sonderfeld  <ruediger@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.

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.

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"

+
+  /// Calls base.setf(ios_base::fixed|
ios_base::scientific,ios_base::floatfield)

Does this line exceed 80 characters?

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


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

+//#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!


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]