[v3] Fix libstdc++/50880

[v3] Fix libstdc++/50880

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 197 bytes --]

Hi,

tested x86_64-linux (with _GLIBCXX_USE_C99_COMPLEX_TR1 manually set to 
zero for the affected function), committed to mainline. Will go in 4.6.3 
too.

Thanks,
Paolo.

///////////////////////

[-- Attachment #2: CL_50880 --]
[-- Type: text/plain, Size: 348 bytes --]

2011-10-27  Richard B. Kreckel  <kreckel@ginac.de>
	    Paolo Carlini  <paolo.carlini@oracle.com>

	PR libstdc++/50880
	* include/std/complex (__complex_acosh): Fix for __z.real() < 0.
	* include/tr1/complex (__complex_acosh): Likewise.
	* testsuite/26_numerics/complex/50880.cc: New.
	* testsuite/tr1/8_c_compatibility/complex/50880.cc: Likewise.

[-- Attachment #3: patch_50880 --]
[-- Type: text/plain, Size: 4331 bytes --]

Index: include/std/complex
===================================================================
--- include/std/complex	(revision 180561)
+++ include/std/complex	(working copy)
@@ -1690,6 +1690,8 @@
 			    * (__z.real() + __z.imag()) - _Tp(1.0),
 			    _Tp(2.0) * __z.real() * __z.imag());
       __t = std::sqrt(__t);
+      if (__z.real() < _Tp(-0.0))
+	__t = -__t;
 
       return std::log(__t + __z);
     }
Index: include/tr1/complex
===================================================================
--- include/tr1/complex	(revision 180561)
+++ include/tr1/complex	(working copy)
@@ -189,6 +189,8 @@
 			    * (__z.real() + __z.imag()) - _Tp(1.0),
 			    _Tp(2.0) * __z.real() * __z.imag());
       __t = std::sqrt(__t);
+      if (__z.real() < _Tp(-0.0))
+	__t = -__t;
 
       return std::log(__t + __z);
     }
Index: testsuite/26_numerics/complex/50880.cc
===================================================================
--- testsuite/26_numerics/complex/50880.cc	(revision 0)
+++ testsuite/26_numerics/complex/50880.cc	(revision 0)
@@ -0,0 +1,53 @@
+// { dg-options "-std=gnu++0x" }
+//
+// Copyright (C) 2011 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 <complex>
+#include <testsuite_hooks.h> 
+
+template<typename T>
+  void test01_do()
+  {
+    bool test __attribute__((unused)) = true;
+
+    const std::complex<T> ca(T(-2), T(2));
+    const std::complex<T> cb(T(-2), T(0));
+    const std::complex<T> cc(T(-2), T(-2));
+
+    std::complex<T> cra = std::acosh(ca);
+    std::complex<T> crb = std::acosh(cb);
+    std::complex<T> crc = std::acosh(cc);
+
+    VERIFY( cra.real() > T(0) );
+    VERIFY( crb.real() > T(0) );
+    VERIFY( crc.real() > T(0) );
+  }
+
+// libstdc++/50880
+void test01()
+{
+  test01_do<float>();
+  test01_do<double>();
+  test01_do<long double>();
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
Index: testsuite/tr1/8_c_compatibility/complex/50880.cc
===================================================================
--- testsuite/tr1/8_c_compatibility/complex/50880.cc	(revision 0)
+++ testsuite/tr1/8_c_compatibility/complex/50880.cc	(revision 0)
@@ -0,0 +1,51 @@
+// Copyright (C) 2011 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 <tr1/complex>
+#include <testsuite_hooks.h> 
+
+template<typename T>
+  void test01_do()
+  {
+    bool test __attribute__((unused)) = true;
+
+    const std::complex<T> ca(T(-2), T(2));
+    const std::complex<T> cb(T(-2), T(0));
+    const std::complex<T> cc(T(-2), T(-2));
+
+    std::complex<T> cra = std::tr1::acosh(ca);
+    std::complex<T> crb = std::tr1::acosh(cb);
+    std::complex<T> crc = std::tr1::acosh(cc);
+
+    VERIFY( cra.real() > T(0) );
+    VERIFY( crb.real() > T(0) );
+    VERIFY( crc.real() > T(0) );
+  }
+
+// libstdc++/50880
+void test01()
+{
+  test01_do<float>();
+  test01_do<double>();
+  test01_do<long double>();
+}
+
+int main()
+{
+  test01();
+  return 0;
+}

Re: [v3] Fix libstdc++/50880

[Gabriel Dos Reis]

On Thu, Oct 27, 2011 at 6:02 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
> tested x86_64-linux (with _GLIBCXX_USE_C99_COMPLEX_TR1 manually set to zero
> for the affected function), committed to mainline. Will go in 4.6.3 too.

Hmm, why is the test of the form x < 0.0, and not testing the sign of x?

Re: [v3] Fix libstdc++/50880

[Paolo Carlini]

Hi,

> On Thu, Oct 27, 2011 at 6:02 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>> Hi,
>> 
>> tested x86_64-linux (with _GLIBCXX_USE_C99_COMPLEX_TR1 manually set to zero
>> for the affected function), committed to mainline. Will go in 4.6.3 too.
> 
> Hmm, why is the test of the form x < 0.0, and not testing the sign of x?

I don't know, personally I don't care much about this fallback code. If you want I can do the change.

Paolo

Re: [v3] Fix libstdc++/50880

[Gabriel Dos Reis]

On Thu, Oct 27, 2011 at 8:19 AM, Paolo Carlini <pcarlini@gmail.com> wrote:
> Hi,
>
>> On Thu, Oct 27, 2011 at 6:02 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>>> Hi,
>>>
>>> tested x86_64-linux (with _GLIBCXX_USE_C99_COMPLEX_TR1 manually set to zero
>>> for the affected function), committed to mainline. Will go in 4.6.3 too.
>>
>> Hmm, why is the test of the form x < 0.0, and not testing the sign of x?
>
> I don't know, personally I don't care much about this fallback code. If you want I can do the change.
>

I am surprised by your comment.  You do not care and that is why you
are eager to
commit the patch without checking first with fellow area maintainers?

Re: [v3] Fix libstdc++/50880

[Paolo Carlini]

Hi,

> I am surprised by your comment.  You do not care and that is why you
> are eager to
> commit the patch without checking first with fellow area maintainers?

Yes, probably my comment wan't clear enough: my point was that I cannot spend more time on this issue. I'm convinced, I may be wrong, that the current code is better than it was 6 hours ago, if I disagree, please revert it, do whatever you like, really, I will take no offense.

Paolo

Re: [v3] Fix libstdc++/50880

[Gabriel Dos Reis]

On Thu, Oct 27, 2011 at 10:19 AM, Paolo Carlini <pcarlini@gmail.com> wrote:
> Hi,
>
>> I am surprised by your comment.  You do not care and that is why you
>> are eager to
>> commit the patch without checking first with fellow area maintainers?
>
> Yes, probably my comment wan't clear enough: my point was that I cannot spend more time on this issue. I'm convinced, I may be wrong, that the current code is better than it was 6 hours ago, if I disagree, please revert it, do whatever you like, really, I will take no offense.
>

I fully appreciate you can't spend more time than you have.  That is hardly an
excuse to willy-nilly applying a patch without checking with fellow maintainers
in special area under the pretense that you don't care and if they
don't agree they
just revert the patch.  that way lies madness and chaos.

Re: [v3] Fix libstdc++/50880

[Paolo Carlini]

Hi again,
> 
> Hmm, why is the test of the form x < 0.0, and not testing the sign of x?

Actually, we can as well use the std::abs, no?

Paolo

Re: [v3] Fix libstdc++/50880

[Gabriel Dos Reis]

On Thu, Oct 27, 2011 at 8:21 AM, Paolo Carlini <pcarlini@gmail.com> wrote:
> Hi again,
>>
>> Hmm, why is the test of the form x < 0.0, and not testing the sign of x?
>
> Actually, we can as well use the std::abs, no?
>
> Paolo


The point of using sign is so that signed zero is not
mischaracterized, especially
when cut branch is at issue.