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

On 02/09/2016 12:38 PM, Carlos O'Donell wrote:
> 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

v2
- Added test case which fails to compile before patch. I saw no easier
  way to test a header inclusion issue like this than to fail the entire
  testsuite when building that test. Adding a conform test just for this
  is overkill and outside the scope of conformtest.
- Regression tested on x86_64 with no regressions.

I'll commit this ASAP if nobody objects.

Adhemerval?

2016-02-09  Jakub Jelinek  <jakub@redhat.com>
	    Jonathan Wakely  <jwakely@redhat.com>
	    Carlos O'Donell  <carlos@redhat.com>

	[BZ 19439]
	* math/Makefile (tests): Add test-math-isinff.
	(CFLAGS-test-math-isinff.cc): Use -std=gnu++11.
	* math/bits/mathcalls.h [__USE_MISC]: Use
	'|| __MATH_DECLARING_DOUBLE == 0' to relax definition of
	functions not in C++11 and which don't conflict e.g. isinff,
	isinfl etc.
	* math/test-math-isinff.cc: New file.

Cheers,
Carlos.

On 09-02-2016 19:54, Carlos O'Donell wrote:
> On 02/09/2016 12:38 PM, Carlos O'Donell wrote:
>> 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
> 
> v2
> - Added test case which fails to compile before patch. I saw no easier
>   way to test a header inclusion issue like this than to fail the entire
>   testsuite when building that test. Adding a conform test just for this
>   is overkill and outside the scope of conformtest.
> - Regression tested on x86_64 with no regressions.
> 
> I'll commit this ASAP if nobody objects.
> 
> Adhemerval?
> 

Sorry for late response.  This is ok for 2.23, thanks!

> 2016-02-09  Jakub Jelinek  <jakub@redhat.com>
> 	    Jonathan Wakely  <jwakely@redhat.com>
> 	    Carlos O'Donell  <carlos@redhat.com>
> 
> 	[BZ 19439]
> 	* math/Makefile (tests): Add test-math-isinff.
> 	(CFLAGS-test-math-isinff.cc): Use -std=gnu++11.
> 	* math/bits/mathcalls.h [__USE_MISC]: Use
> 	'|| __MATH_DECLARING_DOUBLE == 0' to relax definition of
> 	functions not in C++11 and which don't conflict e.g. isinff,
> 	isinfl etc.
> 	* math/test-math-isinff.cc: New file.
> 
> diff --git a/math/Makefile b/math/Makefile
> index 222ee6b..7d573a0 100644
> --- a/math/Makefile
> +++ b/math/Makefile
> @@ -114,6 +114,7 @@ tests = test-matherr test-fenv atest-exp atest-sincos atest-exp2 basic-test \
>  	test-nearbyint-except-2 test-signgam-uchar test-signgam-uchar-init \
>  	test-signgam-uint test-signgam-uint-init test-signgam-ullong \
>  	test-signgam-ullong-init test-nan-overflow test-nan-payload \
> +	test-math-isinff \
>  	$(tests-static)
>  tests-static = test-fpucw-static test-fpucw-ieee-static \
>  	       test-signgam-uchar-static test-signgam-uchar-init-static \
> @@ -220,6 +221,8 @@ CFLAGS-test-signgam-ullong-init.c = -std=c99
>  CFLAGS-test-signgam-ullong-static.c = -std=c99
>  CFLAGS-test-signgam-ullong-init-static.c = -std=c99
>  
> +CFLAGS-test-math-isinff.cc = -std=gnu++11
> +
>  # The -lieee module sets the _LIB_VERSION_ switch to IEEE mode
>  # for error handling in the -lm functions.
>  install-lib += libieee.a
> diff --git a/math/bits/mathcalls.h b/math/bits/mathcalls.h
> index a48345d..9a7b3f0 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 /* isnan conflicts with C++11.  */ \
> +      || __MATH_DECLARING_DOUBLE == 0) /* isnanf or isnanl don't.  */
>  /* Return nonzero if VALUE is not a number.  */
>  __MATHDECL_1 (int,isnan,, (_Mdouble_ __value)) __attribute__ ((__const__));
>  # endif
> diff --git a/math/test-math-isinff.cc b/math/test-math-isinff.cc
> new file mode 100644
> index 0000000..195d753
> --- /dev/null
> +++ b/math/test-math-isinff.cc
> @@ -0,0 +1,48 @@
> +/* Test for bug 19439.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C 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
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#define _GNU_SOURCE 1
> +#include <math.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +static int
> +do_test (void)
> +{
> +  /* Verify that isinff, isinfl, isnanf, and isnanlf are defined
> +     in the header under C++11 and can be called.  Without the
> +     header fix this test will not compile.  */
> +  if (isinff (1.0f)
> +      || !isinff (INFINITY)
> +      || isinfl (1.0L)
> +      || !isinfl (INFINITY)
> +      || isnanf (2.0f)
> +      || !isnanf (NAN)
> +      || isnanl (2.0L)
> +      || !isnanl (NAN))
> +    {
> +      printf ("FAIL: Failed to call is* functions.\n");
> +      exit (1);
> +    }
> +  printf ("PASS: Able to call isinff, isinfl, isnanf, and isnanl.\n");
> +  exit (0);
> +}
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> ---
> 
> Cheers,
> Carlos.
> 
>

On 02/10/2016 11:30 AM, Adhemerval Zanella wrote:
> 
> 
> On 09-02-2016 19:54, Carlos O'Donell wrote:
>> On 02/09/2016 12:38 PM, Carlos O'Donell wrote:
>>> 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
>>
>> v2
>> - Added test case which fails to compile before patch. I saw no easier
>>   way to test a header inclusion issue like this than to fail the entire
>>   testsuite when building that test. Adding a conform test just for this
>>   is overkill and outside the scope of conformtest.
>> - Regression tested on x86_64 with no regressions.
>>
>> I'll commit this ASAP if nobody objects.
>>
>> Adhemerval?
>>
> 
> Sorry for late response.  This is ok for 2.23, thanks!

Committed.

commit 3c47c83a9730c20e602694505b9278c25637b0d0
Author: Carlos O'Donell <carlos@systemhalted.org>
Date:   Sun Feb 14 19:27:06 2016 -0500

    Ensure isinff, isinfl, isnanf, and isnanl are defined (Bug 19439)
    
    In ICO C++11 mode ensure that isinff, isinfl, isnanf, and isnanl
    are defined.  These functions were accidentally removed from the
    header as part of commit d9b965fa56350d6eea9f7f438a0714c7ffbb183f,
    but being GNU extensions, they should have been left in place.

Cheers,
Carlos.

"Carlos O'Donell" <carlos@redhat.com> writes:

> diff --git a/math/test-math-isinff.cc b/math/test-math-isinff.cc
> new file mode 100644
> index 0000000..195d753
> --- /dev/null
> +++ b/math/test-math-isinff.cc
> @@ -0,0 +1,48 @@
> +/* Test for bug 19439.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C 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
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#define _GNU_SOURCE 1
> +#include <math.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +static int
> +do_test (void)
> +{
> +  /* Verify that isinff, isinfl, isnanf, and isnanlf are defined
> +     in the header under C++11 and can be called.  Without the
> +     header fix this test will not compile.  */
> +  if (isinff (1.0f)
> +      || !isinff (INFINITY)
> +      || isinfl (1.0L)
> +      || !isinfl (INFINITY)
> +      || isnanf (2.0f)
> +      || !isnanf (NAN)
> +      || isnanl (2.0L)
> +      || !isnanl (NAN))

That doesn't compile on platforms where long == long double, since they
do not declare any *l functions (arm, hppa, mips/o32).

test-math-isinff.cc: In function 'int do_test()':
test-math-isinff.cc:33:22: error: 'isinfl' was not declared in this scope
       || isinfl (1.0L)
                      ^
test-math-isinff.cc:37:22: error: 'isnanl' was not declared in this scope
       || isnanl (2.0L)
                      ^

Andreas.

On Tue, Feb 23, 2016 at 04:23:42PM +0100, Andreas Schwab wrote:
> > +static int
> > +do_test (void)
> > +{
> > +  /* Verify that isinff, isinfl, isnanf, and isnanlf are defined
> > +     in the header under C++11 and can be called.  Without the
> > +     header fix this test will not compile.  */
> > +  if (isinff (1.0f)
> > +      || !isinff (INFINITY)
> > +      || isinfl (1.0L)
> > +      || !isinfl (INFINITY)
> > +      || isnanf (2.0f)
> > +      || !isnanf (NAN)
> > +      || isnanl (2.0L)
> > +      || !isnanl (NAN))
> 
> That doesn't compile on platforms where long == long double, since they
> do not declare any *l functions (arm, hppa, mips/o32).
> 
> test-math-isinff.cc: In function 'int do_test()':
> test-math-isinff.cc:33:22: error: 'isinfl' was not declared in this scope
>        || isinfl (1.0L)
>                       ^
> test-math-isinff.cc:37:22: error: 'isnanl' was not declared in this scope
>        || isnanl (2.0L)
>                       ^

Thus the *l stuff needs to be guarded with
#ifndef __NO_LONG_DOUBLE_MATH

	Jakub

On 02/23/2016 10:23 AM, Andreas Schwab wrote:
> That doesn't compile on platforms where long == long double, since they
> do not declare any *l functions (arm, hppa, mips/o32).
> 
> test-math-isinff.cc: In function 'int do_test()':
> test-math-isinff.cc:33:22: error: 'isinfl' was not declared in this scope
>        || isinfl (1.0L)
>                       ^
> test-math-isinff.cc:37:22: error: 'isnanl' was not declared in this scope
>        || isnanl (2.0L)
>                       ^

I'll fix this shortly and backport to 2.23 branch.

Any suggestions for how you want to fix it are welcome.

Cheers,
Carlos.