- removed comment
gcc 4.6+ compiler bug for isnan
It seems that gcc 4.6 and up has a problem with compiling the isnan function.
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48891
I find that simply referring to the namespace explicitly solves it.
I had to make similar changes in Carpet/CarpetLib/src/defs.hh, but there I had to fix isinf as well. Probably the isnormal, copysign, etc. functions should get std:: for consistency.
Keyword:
Comments (14)
-
-
- removed comment
Unfortunately, there are other C++ compilers that do not handle "std::isnan" correctly. Currently, it seems that the idiom
using namespace std; isnan(x);
is handled correctly by all compilers I tested. If this is not so, then we will need to introduce an autoconf test for this.
Note also that modern GCC versions always return false from isnan() with -ffast-math (or -Ofast) in C++, but not in C.
Note also that some of isnan's colleagues are not part of older C++ standards.
There are also issues depending on whether <cmath> or <math.h> are included, possibly also depending on their order.
-
- removed comment
the using namespace std construct fails for me with ambiguous definition errors (see $1122). Explicitly using std::isnan helped but is difficult since isnan is not required in the older C++ standards it seems. I get errors from both Vectors (which is not so sophisticated) and CarpetLib's defs.hh (which seems to go to great lengths to find good versions). The error does not happen with gcc 4.7. So if it was not for a "popular" home OS being affected, I would not consider this very major.
-
- changed status to open
- removed comment
I am working on a patch.
-
- removed comment
I attach a patch that solves this problem by using autoconf to decided how to call isnan etc.
In addition to providing a macro CCTK_ISNAN, there is also a C function CCTK_isnan defined that can be called from C++, providing a fallback if necessary.
If C++ does not provide a proper implementation of isnan, i.e. one that can be called as std::isnan, then define a macro isnan to correct this. That is, one can now always call std::isnan to get a good definition of isnan.
Add a test thorn TestMath.
Remove the respective magic from Carpet.
Update various locations that use isnan.
Update thorn NaNChecker: - use std::isnan - simplify check_for logic - don't call C++ isnan, which is known to be optimised away in certain cases; instead, call the C isnan (via CCTK_isnan) that is not optimised away
-
- changed status to open
- removed comment
Tested on: bethe carver datura fermi gordon gpc hopper kraken lonestar nvidia orca pandora philip queenbee ranger surveyor titan trestles zwicky.
-
- removed comment
I read through the patch (autoconf and all) and it seems ok. I cannot obtain the TestMath thorn or the McLachlan '_NV' thorns though it seems. I get:
svn co https://svn.cactuscode.org/arrangements/CactusTest/TestMath TestMath svn: Could not open the requested SVN filesystem
Also trying to compile after applying the patch, it seems as if a file cctk_Math.h seems missing from the patch.
-
- removed comment
I attached the missing files.
The *_NV thorns are not supposed to be mentioned in the thorn list; these are non-vectorised versions of the BSSN thorns that I use for testing.
-
- removed comment
With the missing files I could compile on my Ubuntu (where I had problems before) test system and ran the full testsuite. Most tests pass.
Please apply.
-
- changed status to open
- removed comment
-
- changed status to resolved
- removed comment
Applied.
-
- removed comment
Does the above discussion apply to isnormal, isinf and signbit as well? We have workarounds in datura.cfg for all four.
-
- removed comment
Yes, this applys it isfinite isinf isnormal signbit as well.
The workarounds are not for C++ code, they are for the linker, since Intel and GCC disagree. The workarounds need to stay in place.
-
- changed status to closed
- edited description
- Log in to comment
see also
#1122which is a virtual duplicate and contains the error message obseved. Affected thorns are (at least: Vectors CarpetLib).