diff mbox

Preserve ::is{inf,nan}{f,l} prototypes even for C++11 and later

Message ID 20160201162152.GH3017@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Feb. 1, 2016, 4:21 p.m. UTC
The recent changes disable not just ::is{inf,nan} prototypes that are
incompatible with C++11 and later and that are defined in <cmath> or
libstdc++ <math.h> wrapper, but also the ::is{inf,nan}{f,l} prototypes,
that are not incompatible with C++11.  This patch adds them back.

---
 math/bits/mathcalls.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Jonathan Wakely Feb. 1, 2016, 4:34 p.m. UTC | #1
On 01/02/16 17:21 +0100, Jakub Jelinek wrote:
>The recent changes disable not just ::is{inf,nan} prototypes that are
>incompatible with C++11 and later and that are defined in <cmath> or
>libstdc++ <math.h> wrapper, but also the ::is{inf,nan}{f,l} prototypes,
>that are not incompatible with C++11.  This patch adds them back.

N.B. I also posted Jakub's patch at
https://sourceware.org/ml/libc-alpha/2016-02/msg00020.html with some
more context. The patch is the same though, so you can ignroe my
duplicate.


>---
> math/bits/mathcalls.h | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/math/bits/mathcalls.h b/math/bits/mathcalls.h
>index a48345d..1b82fcd 100644
>--- a/math/bits/mathcalls.h
>+++ b/math/bits/mathcalls.h
>@@ -196,7 +196,9 @@ __MATHDECL_1 (int,__finite,, (_Mdouble_ __value)) __attribute__ ((__const__));
> _Mdouble_END_NAMESPACE
>
> #ifdef __USE_MISC
>-# if !defined __cplusplus || __cplusplus < 201103L /* Conflicts with C++11.  */
>+# if (!defined __cplusplus \
>+      || __cplusplus < 201103L /* isinf conflicts with C++11.  */ \
>+      || __MATH_DECLARING_DOUBLE == 0) /* isinff or isinfl don't.  */
> /* Return 0 if VALUE is finite or NaN, +1 if it
>    is +Infinity, -1 if it is -Infinity.  */
> __MATHDECL_1 (int,isinf,, (_Mdouble_ __value)) __attribute__ ((__const__));
>@@ -232,7 +234,9 @@ __END_NAMESPACE_C99
> __MATHDECL_1 (int,__isnan,, (_Mdouble_ __value)) __attribute__ ((__const__));
>
> #if defined __USE_MISC || (defined __USE_XOPEN && !defined __USE_XOPEN2K)
>-# if !defined __cplusplus || __cplusplus < 201103L /* Conflicts with C++11.  */
>+# if (!defined __cplusplus \
>+      || __cplusplus < 201103L /* isinf conflicts with C++11.  */ \
>+      || __MATH_DECLARING_DOUBLE == 0) /* isinff or isinfl don't.  */
> /* Return nonzero if VALUE is not a number.  */
> __MATHDECL_1 (int,isnan,, (_Mdouble_ __value)) __attribute__ ((__const__));
> # endif
>-- 
>2.4.3
>
Adhemerval Zanella Feb. 3, 2016, 4:55 p.m. UTC | #2
I will quote the email referenced:

> C++11 code using isinf and isnan continues to compile after that
> change, because the C++11 standard library provides its own versions
> conforming to the C++11 requirements. However, the C++11 library
> doesn't provide isinff, isinfl etc. and so code using those
> (non-standard) functions will no longer compile if they are not
> declared by glibc.

This was not clear to me, what kind of build issue are you seeing now?
Using isinf{f,l} by including just <cmath> along with C++11? If it is the
case please open a bugzilla (or update the original) and please commit
the fix.

Also, do we have a testcase on libstdc++ to catch this?

On 01-02-2016 14:34, Jonathan Wakely wrote:
> On 01/02/16 17:21 +0100, Jakub Jelinek wrote:
>> The recent changes disable not just ::is{inf,nan} prototypes that are
>> incompatible with C++11 and later and that are defined in <cmath> or
>> libstdc++ <math.h> wrapper, but also the ::is{inf,nan}{f,l} prototypes,
>> that are not incompatible with C++11.  This patch adds them back.
> 
> N.B. I also posted Jakub's patch at
> https://sourceware.org/ml/libc-alpha/2016-02/msg00020.html with some
> more context. The patch is the same though, so you can ignroe my
> duplicate.
> 
> 
>> ---
>> math/bits/mathcalls.h | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/math/bits/mathcalls.h b/math/bits/mathcalls.h
>> index a48345d..1b82fcd 100644
>> --- a/math/bits/mathcalls.h
>> +++ b/math/bits/mathcalls.h
>> @@ -196,7 +196,9 @@ __MATHDECL_1 (int,__finite,, (_Mdouble_ __value)) __attribute__ ((__const__));
>> _Mdouble_END_NAMESPACE
>>
>> #ifdef __USE_MISC
>> -# if !defined __cplusplus || __cplusplus < 201103L /* Conflicts with C++11.  */
>> +# if (!defined __cplusplus \
>> +      || __cplusplus < 201103L /* isinf conflicts with C++11.  */ \
>> +      || __MATH_DECLARING_DOUBLE == 0) /* isinff or isinfl don't.  */
>> /* Return 0 if VALUE is finite or NaN, +1 if it
>>    is +Infinity, -1 if it is -Infinity.  */
>> __MATHDECL_1 (int,isinf,, (_Mdouble_ __value)) __attribute__ ((__const__));
>> @@ -232,7 +234,9 @@ __END_NAMESPACE_C99
>> __MATHDECL_1 (int,__isnan,, (_Mdouble_ __value)) __attribute__ ((__const__));
>>
>> #if defined __USE_MISC || (defined __USE_XOPEN && !defined __USE_XOPEN2K)
>> -# if !defined __cplusplus || __cplusplus < 201103L /* Conflicts with C++11.  */
>> +# if (!defined __cplusplus \
>> +      || __cplusplus < 201103L /* isinf conflicts with C++11.  */ \
>> +      || __MATH_DECLARING_DOUBLE == 0) /* isinff or isinfl don't.  */
>> /* Return nonzero if VALUE is not a number.  */
>> __MATHDECL_1 (int,isnan,, (_Mdouble_ __value)) __attribute__ ((__const__));
>> # endif
>> -- 
>> 2.4.3
>>
Jonathan Wakely Feb. 3, 2016, 5:15 p.m. UTC | #3
On 03/02/16 14:55 -0200, Adhemerval Zanella wrote:
>I will quote the email referenced:
>
>> C++11 code using isinf and isnan continues to compile after that
>> change, because the C++11 standard library provides its own versions
>> conforming to the C++11 requirements. However, the C++11 library
>> doesn't provide isinff, isinfl etc. and so code using those
>> (non-standard) functions will no longer compile if they are not
>> declared by glibc.
>
>This was not clear to me, what kind of build issue are you seeing now?

There is no issue in libstdc++ this time. The problem comes when C++11
programs try to use the glibc is{inf,nan}{f,l} functions.

>Using isinf{f,l} by including just <cmath> along with C++11? If it is the
>case please open a bugzilla (or update the original) and please commit
>the fix.

Yes, including either <cmath> or <math.h> in C++11 code and trying to
use isinf{f,l}.

>Also, do we have a testcase on libstdc++ to catch this?

No, because nothing in libstdc++ uses or cares about isinf{f,l}, they
are not standard C++ functions. Just like we don't have tests for
other glibc functions such as getwd(3) that aren't part of ISO C++.

We could add a test, but it seems like it's the wrong place for it. It
would have to be limited to only run on *-*-*gnu* targets, since those
functions might not be available elsewhere. However, if glibc doesn't
have any tests that use a C++ compiler maybe libstdc++ is the easiest
place for it.


>On 01-02-2016 14:34, Jonathan Wakely wrote:
>> On 01/02/16 17:21 +0100, Jakub Jelinek wrote:
>>> The recent changes disable not just ::is{inf,nan} prototypes that are
>>> incompatible with C++11 and later and that are defined in <cmath> or
>>> libstdc++ <math.h> wrapper, but also the ::is{inf,nan}{f,l} prototypes,
>>> that are not incompatible with C++11.  This patch adds them back.
>>
>> N.B. I also posted Jakub's patch at
>> https://sourceware.org/ml/libc-alpha/2016-02/msg00020.html with some
>> more context. The patch is the same though, so you can ignroe my
>> duplicate.
>>
>>
>>> ---
>>> math/bits/mathcalls.h | 8 ++++++--
>>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/math/bits/mathcalls.h b/math/bits/mathcalls.h
>>> index a48345d..1b82fcd 100644
>>> --- a/math/bits/mathcalls.h
>>> +++ b/math/bits/mathcalls.h
>>> @@ -196,7 +196,9 @@ __MATHDECL_1 (int,__finite,, (_Mdouble_ __value)) __attribute__ ((__const__));
>>> _Mdouble_END_NAMESPACE
>>>
>>> #ifdef __USE_MISC
>>> -# if !defined __cplusplus || __cplusplus < 201103L /* Conflicts with C++11.  */
>>> +# if (!defined __cplusplus \
>>> +      || __cplusplus < 201103L /* isinf conflicts with C++11.  */ \
>>> +      || __MATH_DECLARING_DOUBLE == 0) /* isinff or isinfl don't.  */
>>> /* Return 0 if VALUE is finite or NaN, +1 if it
>>>    is +Infinity, -1 if it is -Infinity.  */
>>> __MATHDECL_1 (int,isinf,, (_Mdouble_ __value)) __attribute__ ((__const__));
>>> @@ -232,7 +234,9 @@ __END_NAMESPACE_C99
>>> __MATHDECL_1 (int,__isnan,, (_Mdouble_ __value)) __attribute__ ((__const__));
>>>
>>> #if defined __USE_MISC || (defined __USE_XOPEN && !defined __USE_XOPEN2K)
>>> -# if !defined __cplusplus || __cplusplus < 201103L /* Conflicts with C++11.  */
>>> +# if (!defined __cplusplus \
>>> +      || __cplusplus < 201103L /* isinf conflicts with C++11.  */ \
>>> +      || __MATH_DECLARING_DOUBLE == 0) /* isinff or isinfl don't.  */
>>> /* Return nonzero if VALUE is not a number.  */
>>> __MATHDECL_1 (int,isnan,, (_Mdouble_ __value)) __attribute__ ((__const__));
>>> # endif
>>> --
>>> 2.4.3
>>>
Jakub Jelinek Feb. 3, 2016, 5:28 p.m. UTC | #4
On Wed, Feb 03, 2016 at 05:15:36PM +0000, Jonathan Wakely wrote:
> >Using isinf{f,l} by including just <cmath> along with C++11? If it is the
> >case please open a bugzilla (or update the original) and please commit
> >the fix.
> 
> Yes, including either <cmath> or <math.h> in C++11 code and trying to
> use isinf{f,l}.
> 
> >Also, do we have a testcase on libstdc++ to catch this?
> 
> No, because nothing in libstdc++ uses or cares about isinf{f,l}, they
> are not standard C++ functions. Just like we don't have tests for
> other glibc functions such as getwd(3) that aren't part of ISO C++.
> 
> We could add a test, but it seems like it's the wrong place for it. It
> would have to be limited to only run on *-*-*gnu* targets, since those
> functions might not be available elsewhere. However, if glibc doesn't
> have any tests that use a C++ compiler maybe libstdc++ is the easiest
> place for it.

I believe glibc testsuite has some C++ tests, though not many.
You could just test with C++ that you can link (or run, whatever)
#define _GNU_SOURCE 1
#include <math.h>
#include <stdlib.h>

int
main ()
{
  if (isinff (1.0f)
      || !isinff (INFINITY)
      || isinfl (1.0L)
      || !isinfl (INFINITY)
      || isnanf (2.0f)
      || !isnanf (NAN)
      || isnanl (2.0L)
      || !isnanl (NAN))
    abort ();
}
or so.

	Jakub
Mike Frysinger Feb. 3, 2016, 5:40 p.m. UTC | #5
On 03 Feb 2016 14:55, Adhemerval Zanella wrote:
> I will quote the email referenced:
> 
> > C++11 code using isinf and isnan continues to compile after that
> > change, because the C++11 standard library provides its own versions
> > conforming to the C++11 requirements. However, the C++11 library
> > doesn't provide isinff, isinfl etc. and so code using those
> > (non-standard) functions will no longer compile if they are not
> > declared by glibc.
> 
> This was not clear to me, what kind of build issue are you seeing now?
> Using isinf{f,l} by including just <cmath> along with C++11? If it is the
> case please open a bugzilla (or update the original) and please commit
> the fix.

if the change hasn't seen a release yet, then we can just re-use the
existing bug since it's really just a direct follow up to that ?
-mike
Adhemerval Zanella Feb. 3, 2016, 6:04 p.m. UTC | #6
On 03-02-2016 15:40, Mike Frysinger wrote:
> On 03 Feb 2016 14:55, Adhemerval Zanella wrote:
>> I will quote the email referenced:
>>
>>> C++11 code using isinf and isnan continues to compile after that
>>> change, because the C++11 standard library provides its own versions
>>> conforming to the C++11 requirements. However, the C++11 library
>>> doesn't provide isinff, isinfl etc. and so code using those
>>> (non-standard) functions will no longer compile if they are not
>>> declared by glibc.
>>
>> This was not clear to me, what kind of build issue are you seeing now?
>> Using isinf{f,l} by including just <cmath> along with C++11? If it is the
>> case please open a bugzilla (or update the original) and please commit
>> the fix.
> 
> if the change hasn't seen a release yet, then we can just re-use the
> existing bug since it's really just a direct follow up to that ?
> -mike
> 

I do not see why not.
Jonathan Wakely Feb. 9, 2016, 5:23 p.m. UTC | #7
On 03/02/16 16:04 -0200, Adhemerval Zanella wrote:
>
>
>On 03-02-2016 15:40, Mike Frysinger wrote:
>> On 03 Feb 2016 14:55, Adhemerval Zanella wrote:
>>> I will quote the email referenced:
>>>
>>>> C++11 code using isinf and isnan continues to compile after that
>>>> change, because the C++11 standard library provides its own versions
>>>> conforming to the C++11 requirements. However, the C++11 library
>>>> doesn't provide isinff, isinfl etc. and so code using those
>>>> (non-standard) functions will no longer compile if they are not
>>>> declared by glibc.
>>>
>>> This was not clear to me, what kind of build issue are you seeing now?
>>> Using isinf{f,l} by including just <cmath> along with C++11? If it is the
>>> case please open a bugzilla (or update the original) and please commit
>>> the fix.
>>
>> if the change hasn't seen a release yet, then we can just re-use the
>> existing bug since it's really just a direct follow up to that ?
>> -mike
>>
>
>I do not see why not.

Hi, what's the status of this then - do we need to add a testcase or
can it be committed for 2.23? (or wait for 2.24?)

FWIW it's already in the Fedora glibc 2.22 package.
Carlos O'Donell Feb. 9, 2016, 5:38 p.m. UTC | #8
On 02/09/2016 12:23 PM, Jonathan Wakely wrote:
> On 03/02/16 16:04 -0200, Adhemerval Zanella wrote:
>>
>>
>> On 03-02-2016 15:40, Mike Frysinger wrote:
>>> On 03 Feb 2016 14:55, Adhemerval Zanella wrote:
>>>> I will quote the email referenced:
>>>>
>>>>> C++11 code using isinf and isnan continues to compile after that
>>>>> change, because the C++11 standard library provides its own versions
>>>>> conforming to the C++11 requirements. However, the C++11 library
>>>>> doesn't provide isinff, isinfl etc. and so code using those
>>>>> (non-standard) functions will no longer compile if they are not
>>>>> declared by glibc.
>>>>
>>>> This was not clear to me, what kind of build issue are you seeing now?
>>>> Using isinf{f,l} by including just <cmath> along with C++11? If it is the
>>>> case please open a bugzilla (or update the original) and please commit
>>>> the fix.
>>>
>>> if the change hasn't seen a release yet, then we can just re-use the
>>> existing bug since it's really just a direct follow up to that ?
>>> -mike
>>>
>>
>> I do not see why not.
> 
> Hi, what's the status of this then - do we need to add a testcase or
> can it be committed for 2.23? (or wait for 2.24?)

Yes, we want always want a test case. It's perfectly fine to use C++
in  tests in glibc, and if the C++ compiler isn't present those tests
should become UNSUPPORTED tests, which is OK (when bootstrapping).

In summary:
- Add C++ test.
- Use BZ #19439 and provide ChangeLog.
- Post v2

I'll review it and commit, or someone else will.

Cheers,
Carlos.
diff mbox

Patch

diff --git a/math/bits/mathcalls.h b/math/bits/mathcalls.h
index a48345d..1b82fcd 100644
--- a/math/bits/mathcalls.h
+++ b/math/bits/mathcalls.h
@@ -196,7 +196,9 @@  __MATHDECL_1 (int,__finite,, (_Mdouble_ __value)) __attribute__ ((__const__));
 _Mdouble_END_NAMESPACE
 
 #ifdef __USE_MISC
-# if !defined __cplusplus || __cplusplus < 201103L /* Conflicts with C++11.  */
+# if (!defined __cplusplus \
+      || __cplusplus < 201103L /* isinf conflicts with C++11.  */ \
+      || __MATH_DECLARING_DOUBLE == 0) /* isinff or isinfl don't.  */
 /* Return 0 if VALUE is finite or NaN, +1 if it
    is +Infinity, -1 if it is -Infinity.  */
 __MATHDECL_1 (int,isinf,, (_Mdouble_ __value)) __attribute__ ((__const__));
@@ -232,7 +234,9 @@  __END_NAMESPACE_C99
 __MATHDECL_1 (int,__isnan,, (_Mdouble_ __value)) __attribute__ ((__const__));
 
 #if defined __USE_MISC || (defined __USE_XOPEN && !defined __USE_XOPEN2K)
-# if !defined __cplusplus || __cplusplus < 201103L /* Conflicts with C++11.  */
+# if (!defined __cplusplus \
+      || __cplusplus < 201103L /* isinf conflicts with C++11.  */ \
+      || __MATH_DECLARING_DOUBLE == 0) /* isinff or isinfl don't.  */
 /* Return nonzero if VALUE is not a number.  */
 __MATHDECL_1 (int,isnan,, (_Mdouble_ __value)) __attribute__ ((__const__));
 # endif