View Issue Details

IDProjectCategoryView StatusLast Update
0001474OpenFOAM[All Projects] Bugpublic2016-10-04 08:35
ReporterwyldckatAssigned Tohenry 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Versiondev 
Fixed in Version 
Summary0001474: Feedback on the 64-bit labels recently implemented
DescriptionNot sure if this is going to be the only issue, but so far the first one I found, at least with GCC 4.6.3, is that this was missing:

  -D__STDC_LIMIT_MACROS

in the build options. Without it, tons of the following happen during building:

  /home/ofuser/OpenFOAM/OpenFOAM-dev/src/OpenFOAM/lnInclude/label.H:61:1: error: ‘INT64_MIN’ was not declared in this scope
  /home/ofuser/OpenFOAM/OpenFOAM-dev/src/OpenFOAM/lnInclude/label.H:62:1: error: ‘INT64_MAX’ was not declared in this scope

similar happens with the 32-bit labels.
Additional InformationQuick fix/patch:

diff --git a/wmake/rules/linux64Gcc/c++ b/wmake/rules/linux64Gcc/c++
index a23dd82..2e88664 100644
--- a/wmake/rules/linux64Gcc/c++
+++ b/wmake/rules/linux64Gcc/c++
@@ -9,7 +9,7 @@ CC = g++ -m64
 
 include $(RULES)/c++$(WM_COMPILE_OPTION)
 
-ptFLAGS = -DNoRepository -ftemplate-depth-100
+ptFLAGS = -DNoRepository -ftemplate-depth-100 -D__STDC_LIMIT_MACROS
 
 c++FLAGS = $(GFLAGS) $(c++WARN) $(c++OPT) $(c++DBUG) $(ptFLAGS) $(LIB_HEADER_DIRS) -fPIC
TagsNo tags attached.

Activities

henry

2015-01-03 21:14

manager   ~0003484

Maintaning backward compatibility with old gcc versions is becoming increasingly problematic and at the moment I only test back to gcc-4.7.2 for releases. I have tested the latest OpenFOAM-dev with gcc-4.8.1, 4.9.2 and Clang-3.5.0.

I will add the -D__STDC_LIMIT_MACROS to the gcc rules for 4.5 and 4.6 and with check if it is needed for 4.7.

henry

2015-01-03 21:23

manager   ~0003485

I am building with gcc-4.7.2 and -D__STDC_LIMIT_MACROS is not needed

henry

2015-01-03 23:34

manager   ~0003493

I am now building with gcc-4.6.4 and -D__STDC_LIMIT_MACROS is not needed

henry

2015-01-03 23:35

manager   ~0003494

Is this an issue with the gcc version or your system header files?

wyldckat

2015-01-04 01:25

updater   ~0003495

I haven't managed yet to test on more systems other than on my main machine, which is using Ubuntu 12.04 x86_64. The header it's using is the default one at "/usr/include/stdint.h".

Tomorrow (er, today) I'll test on a few more virtual machines with other Linux Distributions/versions, to assess what's going on. I do have a tendency to manage some crazy building conditions sometimes... even if I'm mostly using standard installations with standard options...

wyldckat

2015-01-04 11:13

updater   ~0003496

OK, I've isolated the source of this issue. This was a fix that was implemented in libc:
- Bug report: https://sourceware.org/bugzilla/show_bug.cgi?id=15366
- The commit that fixed this: https://sourceware.org/git/?p=glibc.git;a=commit;h=1ef74943ce2f114c78b215af57c2ccc72ccdb0b7
- Was made part of libc as of version 2.18: https://sourceware.org/git/?p=glibc.git;a=tag;h=f470138ba1124af9cffb839912538bbab2d0ca07

Therefore, if the libc library in use is 2.18 or newer (10 Aug 2013), then this definition "__STDC_LIMIT_MACROS" is not necessary.

OK, this is certainly annoying... CentOS/RHEL 7.0 is using libc 2.17, which doesn't have this fix yet.


I'll test if "std::numeric_limits<int32_t>::max()" is more generic than "INT32_MAX".
Source for this idea: http://stackoverflow.com/questions/3233054/error-int32-max-was-not-declared-in-this-scope

henry

2015-01-04 12:06

manager   ~0003497

> I'll test if "std::numeric_limits<int32_t>::max()" is more generic than "INT32_MAX".

This requires C++11 support which we don't yet rely on in order to maintain support for gcc-4.5 and gcc-4.6. Initially I used cstdint rather than stdint.h but this would have limited gcc versions to 4.7 or higher and required the use of the C++11 support option. At some point I would like to use much of the C++11 functionality in OpenFOAM but we have to wait until support for Ubuntu 12.04 and other old GNU/Linux releases is no longer needed.

I have successfully completed testing of OpenFOAM-dev with gcc-4.[5-8].4, gcc-4.9.2 and clang-3.5.0: no problems.

wyldckat

2015-01-04 13:31

updater   ~0003498

> I have successfully completed testing of OpenFOAM-dev with gcc-4.[5-8].4, gcc-4.9.2 and clang-3.5.0: no problems.

My guess is that you did not have any problems because your system has got "libc" 2.18, 2.19 or newer. You might not have as much success with a system with "libc" 2.17 or older.


While testing with "numeric_limits", I spotted a few stray "INT_MAX" (and UINT_MAX) macros that can be harmful sometime in the future. Perhaps it's best to either switch them to "INT32_MAX" or something more explicitly generic. You can find them by running:

   find src -name *.[CH] -type f | xargs grep INT_MAX


In the mean time I'll continue testing with "numeric_limits".
As far as I can figure out, there was a fix made on "numeric_limits::min/max" in C++11 to have it return as "constexpr" or something like that. But otherwise, this is something that has existed for far longer than C++11.


I'll test "numeric_limits::min/max" on my current system, on a much older and on a much newer system, to assess if using "numeric_limits" is bullet-proof or not to "libc" versions and C++ standards.

henry

2015-01-04 13:46

manager   ~0003499

I will check the use of INT_MAX as you suggest. If "numeric_limits::min/max" proves problematic on old systems as an interim solution I am OK adding

#define __STDC_LIMIT_MACROS

just before the inclusion of stdint.h as suggested in the bug-reports you dug-up.

wyldckat

2015-01-04 15:20

updater   ~0003501

Last edited: 2015-01-04 15:21

View 2 revisions

Now I understand it better... this is why you had stated that "numeric_limits::min/max" was C++11 only:

  static const label labelMin = INT_SIZE(INT, _MIN);
  static const label labelMax = INT_SIZE(INT, _MAX);

precisely due to the new feature "constexpr" in C++11.

-----------------
edit: Mainly because of this:
        syncTools::syncEdgeList
        (
            mesh_,
            edgeLevel,
            ifEqEqOp<labelMax>(),
            labelMin
        );

----------------


Then I'll test with:
   #define __STDC_LIMIT_MACROS

before all "#include <stdint.h>" in OpenFOAM's own source code.

henry

2015-01-04 15:45

manager   ~0003503

> find src -name *.[CH] -type f | xargs grep INT_MAX

Most of these occurrences relate specifically to type int and hence using INT_MAX is correct. The only exceptions are in ptscotchDecomp.C and
 scotchDecomp.C which should now use labelMax. I will push this change.

henry

2015-01-04 18:22

manager   ~0003504

I am testing using numeric_limits at the moment and so far I have not had any problems. I will test build with gcc-4.[5-9] and clang-3.5.0 before pushing the change.

henry

2015-01-04 18:51

manager   ~0003505

I have hit the same problem you hit, back to

#define __STDC_LIMIT_MACROS

henry

2015-01-04 20:18

manager   ~0003506

Resolved by commit 097f23e11a3ee5c4e93ffc68b89614265459e6d5
by inserting

#define __STDC_LIMIT_MACROS

before

#include <stdint.h>

When we stop supporting old GNU/Linux versions and upgrade to C++11 this nonsense will be replaced with cstdint and numeric_limits.

wyldckat

2015-01-04 22:05

updater   ~0003507

Sorry, gotta reopen this report with some more information. This happened both with the hacks I had tried before your commit, as well with a fresh "git pull":

    In file included from /home/ofuser/OpenFOAM/OpenFOAM-dev/src/OpenFOAM/lnInclude/doubleFloat.H:29:0,
                    from /home/ofuser/OpenFOAM/OpenFOAM-dev/src/OpenFOAM/lnInclude/floatScalar.H:38,
                    from /home/ofuser/OpenFOAM/OpenFOAM-dev/src/OpenFOAM/lnInclude/scalar.H:39,
                    from /home/ofuser/OpenFOAM/OpenFOAM-dev/src/OpenFOAM/lnInclude/VectorSpace.H:43,
                    from /home/ofuser/OpenFOAM/OpenFOAM-dev/src/OpenFOAM/lnInclude/Vector.H:44,
                    from /home/ofuser/OpenFOAM/OpenFOAM-dev/src/OpenFOAM/lnInclude/Tensor.H:41,
                    from /home/ofuser/OpenFOAM/OpenFOAM-dev/src/OpenFOAM/lnInclude/tensor.H:38,
                    from lnInclude/indexedVertex.H:42,
                    from conformalVoronoiMesh/CGALTriangulation3Ddefs.H:43,
                    from conformalVoronoiMesh/conformalVoronoiMesh.H:44,
                    from conformalVoronoiMesh/conformalVoronoiMeshFeaturePoints.C:26:
    /home/ofuser/OpenFOAM/OpenFOAM-dev/src/OpenFOAM/lnInclude/label.H:61:1: error: ‘INT32_MIN’ was not declared in this scope
    /home/ofuser/OpenFOAM/OpenFOAM-dev/src/OpenFOAM/lnInclude/label.H:62:1: error: ‘INT32_MAX’ was not declared in this scope



Attached is a patch file "foamyIncludeOrder.patch" with the changes that seemed to fix this issue.
I haven't taken the time to further debug this, but something in the "CGALTriangulation3Ddefs.H" and "CGALTriangulation2Ddefs.H" is interfering with the "#define __STDC_LIMIT_MACROS".

I think I deduced it now... CGAL's headers also include the "stdint.h" before OpenFOAM's own headers.

wyldckat

2015-01-04 22:06

updater  

foamyIncludeOrder.patch (2,559 bytes)
diff --git a/applications/utilities/mesh/generation/foamyHexMesh/conformalVoronoiMesh/conformalVoronoiMesh/conformalVoronoiMesh.H b/applications/utilities/mesh/generation/foamyHexMesh/conformalVoronoiMesh/conformalVoronoiMesh/conformalVoronoiMesh.H
index eb1cf68..b85fb37 100644
--- a/applications/utilities/mesh/generation/foamyHexMesh/conformalVoronoiMesh/conformalVoronoiMesh/conformalVoronoiMesh.H
+++ b/applications/utilities/mesh/generation/foamyHexMesh/conformalVoronoiMesh/conformalVoronoiMesh/conformalVoronoiMesh.H
@@ -41,8 +41,8 @@ SourceFiles
 #ifndef conformalVoronoiMesh_H
 #define conformalVoronoiMesh_H
 
-#include "CGALTriangulation3Ddefs.H"
 #include "uint.H"
+#include "CGALTriangulation3Ddefs.H"
 #include "searchableSurfaces.H"
 #include "conformationSurfaces.H"
 #include "cellShapeControl.H"
diff --git a/applications/utilities/mesh/generation/foamyHexMesh/conformalVoronoiMesh/conformalVoronoiMesh/featurePointConformer/featurePointConformer.H b/applications/utilities/mesh/generation/foamyHexMesh/conformalVoronoiMesh/conformalVoronoiMesh/featurePointConformer/featurePointConformer.H
index e3c415b..e832b29 100644
--- a/applications/utilities/mesh/generation/foamyHexMesh/conformalVoronoiMesh/conformalVoronoiMesh/featurePointConformer/featurePointConformer.H
+++ b/applications/utilities/mesh/generation/foamyHexMesh/conformalVoronoiMesh/conformalVoronoiMesh/featurePointConformer/featurePointConformer.H
@@ -39,12 +39,12 @@ SourceFiles
 #ifndef featurePointConformer_H
 #define featurePointConformer_H
 
-#include "CGALTriangulation3Ddefs.H"
 #include "vector.H"
 #include "DynamicList.H"
 #include "List.H"
 #include "extendedFeatureEdgeMesh.H"
 #include "pointPairs.H"
+#include "CGALTriangulation3Ddefs.H"
 
 // * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
 
diff --git a/applications/utilities/mesh/generation/foamyQuadMesh/CV2D.H b/applications/utilities/mesh/generation/foamyQuadMesh/CV2D.H
index 4948196..e3e316a 100644
--- a/applications/utilities/mesh/generation/foamyQuadMesh/CV2D.H
+++ b/applications/utilities/mesh/generation/foamyQuadMesh/CV2D.H
@@ -117,8 +117,6 @@ SourceFiles
 #define CGAL_INEXACT
 #define CGAL_HIERARCHY
 
-#include "CGALTriangulation2Ddefs.H"
-
 #include "Time.H"
 #include "point2DFieldFwd.H"
 #include "dictionary.H"
@@ -134,6 +132,8 @@ SourceFiles
 #include "relaxationModel.H"
 #include "cellSizeAndAlignmentControls.H"
 
+#include "CGALTriangulation2Ddefs.H"
+
 // * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
 
 namespace Foam
foamyIncludeOrder.patch (2,559 bytes)

henry

2015-01-04 23:46

manager   ~0003508

I applied the patch but re-ordered a bit and added a comment. Maybe it would be more obvious if we simply define __STDC_LIMIT_MACROS before the CGAL headers are included.

Please re-open if there are any remaining issues with this.

user4

2015-06-26 12:03

  ~0005004

There is a problem with

- compiling Pstream
- with IntelMPI
- with g++ (clang++ is ok)

What seems to be happening:
- Intel mpi.h pulls in /usr/include/stdint.h
 but without setting __STDC_LIMIT_MACROS
- so then when it comes to uint32.H it does not include stdin.h anymore.

Solution:

add
    #define __STDC_LIMIT_MACROS
to
    src/Pstream/mpi/UIPread.C
    src/Pstream/mpi/UPstream.C
    src/Pstream/mpi/UOPwrite.C
    src/Pstream/mpi/PstreamGlobals.C
before
    #include "mpi.h"

henry

2015-06-26 12:11

manager   ~0005005

I want to avoid the proliferation of __STDC_LIMIT_MACROS which is a temporary work-around for old C++ compilers. Why not move the include of mpi.h after the OpenFOAM headers?

user4

2015-06-26 12:37

  ~0005006

Sounds good. Is what we're doing already in e.g. ptscotchDecomp.C and zoltanRenumber.C

henry

2016-10-04 08:35

manager   ~0006956

#define __STDC_LIMIT_MACROS removed from OpenFOAM-dev:

commit dee026476892e0870b59fbc0cbe8de2fa46d0508

Issue History

Date Modified Username Field Change
2015-01-03 20:39 wyldckat New Issue
2015-01-03 21:14 henry Note Added: 0003484
2015-01-03 21:23 henry Note Added: 0003485
2015-01-03 23:34 henry Note Added: 0003493
2015-01-03 23:35 henry Note Added: 0003494
2015-01-04 01:25 wyldckat Note Added: 0003495
2015-01-04 11:13 wyldckat Note Added: 0003496
2015-01-04 12:06 henry Note Added: 0003497
2015-01-04 13:31 wyldckat Note Added: 0003498
2015-01-04 13:46 henry Note Added: 0003499
2015-01-04 15:20 wyldckat Note Added: 0003501
2015-01-04 15:21 wyldckat Note Edited: 0003501 View Revisions
2015-01-04 15:45 henry Note Added: 0003503
2015-01-04 18:22 henry Note Added: 0003504
2015-01-04 18:51 henry Note Added: 0003505
2015-01-04 20:18 henry Note Added: 0003506
2015-01-04 20:18 henry Status new => resolved
2015-01-04 20:18 henry Resolution open => fixed
2015-01-04 20:18 henry Assigned To => henry
2015-01-04 22:05 wyldckat Note Added: 0003507
2015-01-04 22:05 wyldckat Status resolved => feedback
2015-01-04 22:05 wyldckat Resolution fixed => reopened
2015-01-04 22:06 wyldckat File Added: foamyIncludeOrder.patch
2015-01-04 23:46 henry Note Added: 0003508
2015-01-04 23:46 henry Status feedback => resolved
2015-01-04 23:46 henry Resolution reopened => fixed
2015-06-26 12:03 user4 Note Added: 0005004
2015-06-26 12:11 henry Note Added: 0005005
2015-06-26 12:37 user4 Note Added: 0005006
2016-10-04 08:35 henry Note Added: 0006956