On 11/12/2016 05:30 PM, Paul Eggert wrote: > Thanks, I installed your two recent patches.
Thanks! > I don't know C++ well; > perhaps Bruno or another reviewer can double-check if they have the time. > > I noticed that with the patches installed, the command './gnulib-tool > --test --with-c++-tests frexp frexpl frexpf frexp-nolibm' fails with > diagnostics like the following (Fedora 24, GCC 6.2.1 20160916) > > make[3]: Entering directory > '/home/eggert/src/gnu/gnulib/testdir4584/build/gltests' > g++ -DHAVE_CONFIG_H -I. -I../../gltests -DGNULIB_STRICT_CHECKING=1 -I. > -I../../gltests -I.. -I../../gltests/.. -I../gllib > -I../../gltests/../gllib -MT test-math-c++.o -MD -MP -MF > .deps/test-math-c++.Tpo -c -o test-math-c++.o > ../../gltests/test-math-c++.cc > ../../gltests/test-math-c++.cc:37:45: error: invalid static_cast from > type ‘<unresolved overloaded function type>’ to type ‘int (*)(float)’ > = static_cast<rettype(*)parameters>(func) > ^ > ../../gltests/test-math-c++.cc:32:3: note: in expansion of macro > ‘OVERLOADED_CHECK’ > OVERLOADED_CHECK (func, rettype1, parameters1, _1); \ > ^~~~~~~~~~~~~~~~ > > > However, similar failures occur even without the patches (the patches > fix one of the failures, actually) so this is not a regression. I looked at this today. The problem here is that these functions (signbit, isfinite, isnan, etc.) return bool in a conforming C++11 implementation: http://en.cppreference.com/w/cpp/numeric/math/signbit Tweaking the test the obvious way to expect bool return types makes the test pass with gcc >= 6. However, if we _just_ do that, the test starts failing with gcc < 6, which didn't have the new libstdc++ math.h wrapper that makes gcc fully C++11 conforming. We end up with gnulib providing the overloads as returning int. If we tweak the gnulib replacements to define the functions as returning bool (with the obvious return type change to _GL_MATH_CXX_REAL_FLOATING_DECL_1/_GL_MATH_CXX_REAL_FLOATING_DECL_2)), we stumble on this on Fedora 23, at least: make[3]: Entering directory '/home/pedro/brno/pedro/src/gnulib/src/testdir11559/build/gltests' Making all in . make[4]: Entering directory '/home/pedro/brno/pedro/src/gnulib/src/testdir11559/build/gltests' g++ -std=gnu++11 -Wall -Wno-unused-variable -DHAVE_CONFIG_H -I. -I../../gltests -DGNULIB_STRICT_CHECKING=1 -I. -I../../gltests -I.. -I../../gltests/.. -I../gllib -I../../gltests/../gllib -MT test-math-c++.o -MD -MP -MF .deps/test-math-c++.Tpo -c -o test-math-c++.o ../../gltests/test-math-c++.cc In file included from ../../gltests/test-math-c++.cc:22:0: ../gllib/math.h: In function ‘bool isinf(double)’: ../gllib/math.h:2422:1: error: ambiguating new declaration of ‘bool isinf(double)’ _GL_MATH_CXX_REAL_FLOATING_DECL_2 (isinf) ^ In file included from /usr/include/features.h:365:0, from /usr/include/math.h:26, from ../gllib/math.h:27, from ../../gltests/test-math-c++.cc:22: /usr/include/bits/mathcalls.h:201:1: note: old declaration ‘int isinf(double)’ __MATHDECL_1 (int,isinf,, (_Mdouble_ __value)) __attribute__ ((__const__)); ^ In file included from ../../gltests/test-math-c++.cc:22:0: ../gllib/math.h: In function ‘bool isnan(double)’: ../gllib/math.h:2540:1: error: ambiguating new declaration of ‘bool isnan(double)’ _GL_MATH_CXX_REAL_FLOATING_DECL_2 (isnan) ^ See <https://sourceware.org/bugzilla/show_bug.cgi?id=19439> for background. It seems to me that the spirit of gnulib should be to define C/POSIX replacements, while largely moving out of the way of the C++ runtime. I don't think providing replacements for C++ runtime holes (which is much larger than what it inherits from C) should be in scope for gnulib. So I think this should be addressed by defining the replacements for these functions in the GNULIB_NAMESPACE namespace instead of in the global namespace, like all other function replacements. And given gnulib aims at C/POSIX, I think it's better/simpler if the replacements follow the C interface, and thus return int, not bool. So, here's a patch that works for me on Fedora 23, tested with gcc/g++ 4.7, 4.8, 4.9, 5.3.1 (system compiler) and 7 (trunk), all in both c++03 and c++11 modes, using this script: ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ #!/bin/bash set -e test="./gnulib-tool --test --with-c++-tests frexp frexpl frexpf frexp-nolibm isnan isnanf isnand isnanl isfinite isinf" WARN_CFLAGS="-Wall -Wno-unused-variable" PREFIX="/opt/gcc-4.7/bin" CC="$PREFIX/gcc -Wall" CXX="$PREFIX/g++ -std=gnu++03 $WARN_CFLAGS" $test PREFIX="/opt/gcc-4.7/bin" CC="$PREFIX/gcc -Wall" CXX="$PREFIX/g++ -std=gnu++0x $WARN_CFLAGS" $test PREFIX="/opt/gcc-4.8/bin" CC="$PREFIX/gcc -Wall" CXX="$PREFIX/g++ -std=gnu++03 $WARN_CFLAGS" $test PREFIX="/opt/gcc-4.8/bin" CC="$PREFIX/gcc -Wall" CXX="$PREFIX/g++ -std=gnu++11 $WARN_CFLAGS" $test PREFIX="/opt/gcc-4.9/bin" CC="$PREFIX/gcc -Wall" CXX="$PREFIX/g++ -std=gnu++03 $WARN_CFLAGS" $test PREFIX="/opt/gcc-4.9/bin" CC="$PREFIX/gcc -Wall" CXX="$PREFIX/g++ -std=gnu++11 $WARN_CFLAGS" $test # gcc 5.3.1 on Fedora 23 CC="gcc -Wall" CXX="g++ -std=gnu++03 -Wall -Wno-unused-variable" $test CC="gcc -Wall" CXX="g++ -std=gnu++11 -Wall -Wno-unused-variable" $test # gcc trunk on Fedora 23 PREFIX="/opt/gcc/bin" CC="$PREFIX/gcc -Wall" CXX="$PREFIX/g++ -std=gnu++03 $WARN_CFLAGS" $test PREFIX="/opt/gcc/bin" CC="$PREFIX/gcc -Wall" CXX="$PREFIX/g++ -std=gnu++11 $WARN_CFLAGS" $test ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >From 0163126ec110fba504995a2aabebf09610cad1c4 Mon Sep 17 00:00:00 2001 From: Pedro Alves <pal...@redhat.com> Date: Mon, 14 Nov 2016 17:08:23 +0000 Subject: [PATCH] Fix real-floating argument functions in C++ mode ChangeLog: 2016-11-14 Pedro Alves <pal...@redhat.com> Fix real-floating argument functions in C++ mode. Define define isfinite, isinf, isnan, signbit in the gnulib namespace instead of in the global namespace. * lib/math.in.h (_GL_MATH_CXX_REAL_FLOATING_DECL_NS): New macro. (isfinite, isinf, isnan, signbit): Use it. * tests/test-math-c++.cc: Test the isfinite, isinf, isnan, signbit overloads in the GNULIB_NAMESPACE namespace, instead of the global namespace. --- lib/math.in.h | 34 ++++++++++++++++++++++++++++++---- tests/test-math-c++.cc | 5 +++-- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/lib/math.in.h b/lib/math.in.h index 9a194cb..8bbd25d 100644 --- a/lib/math.in.h +++ b/lib/math.in.h @@ -80,6 +80,16 @@ func (long double l) \ } #endif +/* Define the FUNC overloads in the GNULIB_NAMESPACE namespace. */ +#ifdef GNULIB_NAMESPACE +# define _GL_MATH_CXX_REAL_FLOATING_DECL_NS(func) \ +namespace GNULIB_NAMESPACE { \ +_GL_MATH_CXX_REAL_FLOATING_DECL_2 (func) \ +} +#else +# define _GL_MATH_CXX_REAL_FLOATING_DECL_NS(func) +#endif + /* Helper macros to define a portability warning for the classification macro FUNC called with VALUE. POSIX declares the classification macros with an argument of real-floating (that is, @@ -2044,10 +2054,14 @@ _GL_EXTERN_C int gl_isfinitel (long double x); gl_isfinitef (x)) # endif # ifdef __cplusplus -# ifdef isfinite +# if defined isfinite || defined GNULIB_NAMESPACE _GL_MATH_CXX_REAL_FLOATING_DECL_1 (isfinite) # undef isfinite +# ifdef GNULIB_NAMESPACE +_GL_MATH_CXX_REAL_FLOATING_DECL_NS (isfinite) +# else _GL_MATH_CXX_REAL_FLOATING_DECL_2 (isfinite) +# endif # endif # endif #elif defined GNULIB_POSIXCHECK @@ -2071,10 +2085,14 @@ _GL_EXTERN_C int gl_isinfl (long double x); gl_isinff (x)) # endif # ifdef __cplusplus -# ifdef isinf +# if defined isinf || defined GNULIB_NAMESPACE _GL_MATH_CXX_REAL_FLOATING_DECL_1 (isinf) # undef isinf +# ifdef GNULIB_NAMESPACE +_GL_MATH_CXX_REAL_FLOATING_DECL_NS (isinf) +# else _GL_MATH_CXX_REAL_FLOATING_DECL_2 (isinf) +# endif # endif # endif #elif defined GNULIB_POSIXCHECK @@ -2189,10 +2207,14 @@ _GL_EXTERN_C int rpl_isnanl (long double x) _GL_ATTRIBUTE_CONST; __builtin_isnanf ((float)(x))) # endif # ifdef __cplusplus -# ifdef isnan +# if defined isnan || defined GNULIB_NAMESPACE _GL_MATH_CXX_REAL_FLOATING_DECL_1 (isnan) # undef isnan +# ifdef GNULIB_NAMESPACE +_GL_MATH_CXX_REAL_FLOATING_DECL_NS (isnan) +# else _GL_MATH_CXX_REAL_FLOATING_DECL_2 (isnan) +# endif # endif # else /* Ensure isnan is a macro. */ @@ -2264,10 +2286,14 @@ _GL_EXTERN_C int gl_signbitl (long double arg); gl_signbitf (x)) # endif # ifdef __cplusplus -# ifdef signbit +# if defined signbit || defined GNULIB_NAMESPACE _GL_MATH_CXX_REAL_FLOATING_DECL_1 (signbit) # undef signbit +# ifdef GNULIB_NAMESPACE +_GL_MATH_CXX_REAL_FLOATING_DECL_NS (signbit) +# else _GL_MATH_CXX_REAL_FLOATING_DECL_2 (signbit) +# endif # endif # endif #elif defined GNULIB_POSIXCHECK diff --git a/tests/test-math-c++.cc b/tests/test-math-c++.cc index f0f1448..cc7378c 100644 --- a/tests/test-math-c++.cc +++ b/tests/test-math-c++.cc @@ -24,7 +24,8 @@ #include "signature.h" /* Signature check for a function that takes a real-floating argument. - Check that each overloaded function with the specified signature exists. */ + Check that each overloaded function with the specified signature + exists in the GNULIB_NAMESPACE namespace. */ #define REAL_FLOATING_CHECK(func,\ rettype1, parameters1,\ rettype2, parameters2,\ @@ -34,7 +35,7 @@ OVERLOADED_CHECK (func, rettype3, parameters3, _3) #define OVERLOADED_CHECK(func, rettype, parameters, suffix) \ static rettype (* _GL_UNUSED signature_check_ ## func ## suffix) parameters \ - = static_cast<rettype(*)parameters>(func) + = static_cast<rettype(*)parameters>(GNULIB_NAMESPACE::func) /* Keep these checks in the same order as math.in.h! */ -- 2.5.5
