| Message ID | 20160201162152.GH3017@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
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 >
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 >>
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 >>>
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
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
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.
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.
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 --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
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(-)