On 13/12/17 18:42 +0000, Jonathan Wakely wrote:
The bug here is that we called putback even if the initial __is >> __ch
extraction failed and set eofbit, and putback clears the eofbit. I
found a number of other problems though, such as not even trying to
call putback after failing to find the ',' and ')' characters.

I decided to rewrite the function following the proposed resolution
for https://wg21.link/lwg2714 which is a much more precise
specification for much more desirable semantics.

        PR libstdc++/59568
        * include/std/complex (operator>>): Implement proposed resolution to
        LWG 2714. Use putback if and only if a character has been successfully
        extracted but isn't a delimiter. Use ctype::widen and traits::eq when
        testing if extracted characters match delimiters.
        * testsuite/26_numerics/complex/dr2714.cc: New test.

Tested powerpc64le-linux, committed to trunk.

For the release branches I'm considering just fixing the bug that
clears eofbit, and not the whole rewrite of the function.

Actually there's another bug in the original function, which is that
it unconditionally sets "__x = __re_x;" even if extracting a value
into __re_x failed.

Here's what I plan to commit for the branches.

Tested x86_64-linux.

commit 907f186cd5d2e98f8ddf031b46b2cb0ae520b0d7
Author: Jonathan Wakely <jwak...@redhat.com>
Date:   Wed Dec 13 20:21:26 2017 +0000

    PR libstdc++/59568 don't use putback or update value when extraction fails
    
            PR libstdc++/59568
            * include/std/complex (operator>>): Only use putback if a character
            was successfully extracted and only set the value if a number was
            successfully extracted.
            * testsuite/26_numerics/complex/inserters_extractors/char/59568.cc:
            New test.

diff --git a/libstdc++-v3/include/std/complex b/libstdc++-v3/include/std/complex
index 6342c98e88a..22107cb2264 100644
--- a/libstdc++-v3/include/std/complex
+++ b/libstdc++-v3/include/std/complex
@@ -493,7 +493,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     operator>>(basic_istream<_CharT, _Traits>& __is, complex<_Tp>& __x)
     {
       _Tp __re_x, __im_x;
-      _CharT __ch;
+      _CharT __ch = _CharT();
       __is >> __ch;
       if (__ch == '(')
 	{
@@ -511,11 +511,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  else
 	    __is.setstate(ios_base::failbit);
 	}
-      else
+      else if (__is)
 	{
 	  __is.putback(__ch);
-	  __is >> __re_x;
-	  __x = __re_x;
+	  if (__is >> __re_x)
+	    __x = __re_x;
+	  else
+	    __is.setstate(ios_base::failbit);
 	}
       return __is;
     }
diff --git a/libstdc++-v3/testsuite/26_numerics/complex/inserters_extractors/char/59568.cc b/libstdc++-v3/testsuite/26_numerics/complex/inserters_extractors/char/59568.cc
new file mode 100644
index 00000000000..2bbdb6abae4
--- /dev/null
+++ b/libstdc++-v3/testsuite/26_numerics/complex/inserters_extractors/char/59568.cc
@@ -0,0 +1,166 @@
+// Copyright (C) 2017 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/>.
+
+// { dg-options "-std=gnu++98" }
+
+#include <complex>
+#include <sstream>
+#include <complex>
+#include <testsuite_hooks.h>
+
+void
+test01()
+{
+  std::istringstream in(" 1 (2) ( 2.0 , 0.5 ) ");
+  std::complex<double> c1, c2, c3;
+  in >> c1 >> c2 >> c3;
+  VERIFY( in.good() );
+  VERIFY( c1.real() == 1 && c1.imag() == 0 );
+  VERIFY( c2.real() == 2 && c2.imag() == 0 );
+  VERIFY( c3.real() == 2 && c3.imag() == 0.5 );
+}
+
+void
+test02()
+{
+  std::istringstream in;
+  std::complex<double> c(-1, -1);
+  const std::complex<double> c0 = c;
+
+  in.str("a");
+  in >> c;
+  VERIFY( in.fail() );
+  in.clear();
+  VERIFY( in.get() == 'a' );
+  VERIFY( c == c0 );
+
+  in.str(" ( ) ");
+  in >> c;
+  VERIFY( in.fail() );
+  in.clear();
+  VERIFY( in.get() == ')' );
+  VERIFY( c == c0 );
+
+  in.str("(,");
+  in >> c;
+  VERIFY( in.fail() );
+  in.clear();
+  VERIFY( in.get() == ',' );
+  VERIFY( c == c0 );
+
+  in.str("(b)");
+  in >> c;
+  VERIFY( in.fail() );
+  in.clear();
+  VERIFY( in.get() == 'b' );
+  VERIFY( c == c0 );
+
+  in.str("( c)");
+  in >> c;
+  VERIFY( in.fail() );
+  in.clear();
+  VERIFY( in.get() == 'c' );
+  VERIFY( c == c0 );
+
+  in.str("(99d");
+  in >> c;
+  VERIFY( in.fail() );
+  in.clear();
+  // VERIFY( in.get() == 'd' );
+  VERIFY( c == c0 );
+
+  in.str("(99 e");
+  in >> c;
+  VERIFY( in.fail() );
+  in.clear();
+  // VERIFY( in.get() == 'e' );
+  VERIFY( c == c0 );
+
+  in.str("(99, f");
+  in >> c;
+  VERIFY( in.fail() );
+  in.clear();
+  VERIFY( in.get() == 'f' );
+  VERIFY( c == c0 );
+
+  in.str("(99, 88g");
+  in >> c;
+  VERIFY( in.fail() );
+  in.clear();
+  // VERIFY( in.get() == 'g' );
+  VERIFY( c == c0 );
+
+  in.str("(99, 88 h");
+  in >> c;
+  VERIFY( in.fail() );
+  in.clear();
+  // VERIFY( in.get() == 'h' );
+  VERIFY( c == c0 );
+
+  in.str("(99, )");
+  in >> c;
+  VERIFY( in.fail() );
+  in.clear();
+  VERIFY( in.get() == ')' );
+  VERIFY( c == c0 );
+}
+
+void
+test03()
+{
+  // PR libstdc++/59568
+  std::istringstream in;
+  std::complex<double> c;
+
+  in.str("");
+  in >> c;
+  VERIFY( in.fail() );
+  VERIFY( in.eof() );
+  in.clear();
+
+  in.str(" ");
+  in >> c;
+  VERIFY( in.fail() );
+  VERIFY( in.eof() );
+  in.clear();
+
+  in.str("(99");
+  in >> c;
+  VERIFY( in.fail() );
+  VERIFY( in.eof() );
+  in.clear();
+
+  in.str("(99,");
+  in >> c;
+  VERIFY( in.fail() );
+  VERIFY( in.eof() );
+  in.clear();
+
+  in.str("(99,99");
+  in >> c;
+  VERIFY( in.fail() );
+  VERIFY( in.eof() );
+  in.clear();
+}
+
+int
+main()
+{
+  test01();
+  test02();
+  test03();
+}

Reply via email to