View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0001806OpenFOAMBugpublic2015-07-30 22:452015-08-04 08:50
Reporteralexeym Assigned Tohenry  
PrioritynormalSeveritytweakReproducibilityalways
Status closedResolutionno change required 
Summary0001806: Compiler diagnostic flags
DescriptionCurrently compiler warnings are suppressed with -Wno-... style flags, maybe it would be better to suppress individual warnings using pragma mechanism (which is supported at least by gcc and clang) instead of global switch?

Currently biggest source of warning is unused parameters. OpenFOAM-dev-unused-parameters.patch is a patch that comments out unused parameters.

The second major source of warnings is Boost and CGAL. There is an attempt to solve it with -Wno-... flags, yet it could be solved with pragmas. OpenFOAM-dev-pragmas.patch adds corresponding statements. Also invalid offset warning is suppressed in certain cases.
Steps To ReproduceBuild with just -Wall and -Wextra diagnostic flags
TagsNo tags attached.

Activities

alexeym

2015-07-30 22:45

reporter  

OpenFOAM-dev-unused-parameters.patch (586,660 bytes)

alexeym

2015-07-30 22:45

reporter  

OpenFOAM-dev-pragmas.patch (9,722 bytes)    
diff --git a/applications/utilities/mesh/generation/foamyMesh/conformalVoronoiMesh/conformalVoronoiMesh/CGALTriangulation3DKernel.H b/applications/utilities/mesh/generation/foamyMesh/conformalVoronoiMesh/conformalVoronoiMesh/CGALTriangulation3DKernel.H
index cdafc7a..8b9e1db 100644
--- a/applications/utilities/mesh/generation/foamyMesh/conformalVoronoiMesh/conformalVoronoiMesh/CGALTriangulation3DKernel.H
+++ b/applications/utilities/mesh/generation/foamyMesh/conformalVoronoiMesh/conformalVoronoiMesh/CGALTriangulation3DKernel.H
@@ -33,7 +33,23 @@ Description
 
 // * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
 
+// Ignore unused parameter warning in Boost libraries
+#if defined(__clang__)
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunused-parameter"
+#pragma clang diagnostic ignored "-Wtautological-undefined-compare"
+#endif
+#if defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-parameter"
+#endif
 #include "CGAL/Delaunay_triangulation_3.h"
+#if defined(__clang__)
+#pragma clang diagnostic pop
+#endif
+#if defined(__GNUC__)
+#pragma GCC diagnostic pop
+#endif
 
 #ifdef CGAL_INEXACT
 
diff --git a/applications/utilities/mesh/generation/foamyMesh/conformalVoronoiMesh/conformalVoronoiMesh/conformalVoronoiMesh.H b/applications/utilities/mesh/generation/foamyMesh/conformalVoronoiMesh/conformalVoronoiMesh/conformalVoronoiMesh.H
index f6d4448..f121f45 100644
--- a/applications/utilities/mesh/generation/foamyMesh/conformalVoronoiMesh/conformalVoronoiMesh/conformalVoronoiMesh.H
+++ b/applications/utilities/mesh/generation/foamyMesh/conformalVoronoiMesh/conformalVoronoiMesh/conformalVoronoiMesh.H
@@ -43,7 +43,22 @@ SourceFiles
 
 // Include uint.H before CGAL headers to define __STDC_LIMIT_MACROS
 #include "uint.H"
+// To ignore unused parameter warning in Boost libraries
+#if defined(__clang__)
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunused-parameter"
+#endif
+#if defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-parameter"
+#endif
 #include "CGALTriangulation3Ddefs.H"
+#if defined(__clang__)
+#pragma clang diagnostic pop
+#endif
+#if defined(__GNUC__)
+#pragma GCC diagnostic pop
+#endif
 #include "searchableSurfaces.H"
 #include "conformationSurfaces.H"
 #include "cellShapeControl.H"
diff --git a/applications/utilities/mesh/generation/foamyMesh/conformalVoronoiMesh/conformalVoronoiMesh/indexedVertex/indexedVertex.H b/applications/utilities/mesh/generation/foamyMesh/conformalVoronoiMesh/conformalVoronoiMesh/indexedVertex/indexedVertex.H
index e4b012e..43cf60d 100644
--- a/applications/utilities/mesh/generation/foamyMesh/conformalVoronoiMesh/conformalVoronoiMesh/indexedVertex/indexedVertex.H
+++ b/applications/utilities/mesh/generation/foamyMesh/conformalVoronoiMesh/conformalVoronoiMesh/indexedVertex/indexedVertex.H
@@ -37,7 +37,23 @@ SourceFiles
 #ifndef indexedVertex_H
 #define indexedVertex_H
 
+// Ignore unused parameter warning in Boost libraries
+#if defined(__clang__)
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunused-parameter"
+#pragma clang diagnostic ignored "-Wtautological-undefined-compare"
+#endif
+#if defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-parameter"
+#endif
 #include "CGAL/Triangulation_3.h"
+#if defined(__clang__)
+#pragma clang diagnostic pop
+#endif
+#if defined(__GNUC__)
+#pragma GCC diagnostic pop
+#endif
 #include "CGALTriangulation3DKernel.H"
 #include "tensor.H"
 #include "triad.H"
diff --git a/applications/utilities/mesh/generation/foamyMesh/foamyQuadMesh/CGALTriangulation2DKernel.H b/applications/utilities/mesh/generation/foamyMesh/foamyQuadMesh/CGALTriangulation2DKernel.H
index 012192b..3efde24 100644
--- a/applications/utilities/mesh/generation/foamyMesh/foamyQuadMesh/CGALTriangulation2DKernel.H
+++ b/applications/utilities/mesh/generation/foamyMesh/foamyQuadMesh/CGALTriangulation2DKernel.H
@@ -33,7 +33,23 @@ Description
 
 // * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
 
+// Ignore unused parameter warning in Boost libraries
+#if defined(__clang__)
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunused-parameter"
+#pragma clang diagnostic ignored "-Wtautological-undefined-compare"
+#endif
+#if defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-parameter"
+#endif
 #include "CGAL/Delaunay_triangulation_2.h"
+#if defined(__clang__)
+#pragma clang diagnostic pop
+#endif
+#if defined(__GNUC__)
+#pragma GCC diagnostic pop
+#endif
 
 #ifdef CGAL_INEXACT
 
diff --git a/src/lagrangian/basic/particle/particleIO.C b/src/lagrangian/basic/particle/particleIO.C
index 7d61717..2254400 100644
--- a/src/lagrangian/basic/particle/particleIO.C
+++ b/src/lagrangian/basic/particle/particleIO.C
@@ -26,6 +26,13 @@ License
 #include "particle.H"
 #include "IOstreams.H"
 
+#if defined(__clang__)
+#pragma clang diagnostic ignored "-Winvalid-offsetof"
+#endif
+#if defined(__GNUC__)
+#pragma GCC diagnostic ignored "-Winvalid-offsetof"
+#endif
+
 // * * * * * * * * * * * * * * Static Data Members * * * * * * * * * * * * * //
 
 Foam::string Foam::particle::propertyList_ = Foam::particle::propertyList();
diff --git a/src/lagrangian/intermediate/parcels/Templates/CollidingParcel/CollidingParcelIO.C b/src/lagrangian/intermediate/parcels/Templates/CollidingParcel/CollidingParcelIO.C
index 25aee6f..f2cfde4 100644
--- a/src/lagrangian/intermediate/parcels/Templates/CollidingParcel/CollidingParcelIO.C
+++ b/src/lagrangian/intermediate/parcels/Templates/CollidingParcel/CollidingParcelIO.C
@@ -27,6 +27,13 @@ License
 #include "IOstreams.H"
 #include "IOField.H"
 
+#if defined(__clang__)
+#pragma clang diagnostic ignored "-Winvalid-offsetof"
+#endif
+#if defined(__GNUC__)
+#pragma GCC diagnostic ignored "-Winvalid-offsetof"
+#endif
+
 // * * * * * * * * * * * * * * Static Data Members * * * * * * * * * * * * * //
 
 template<class ParcelType>
diff --git a/src/lagrangian/intermediate/parcels/Templates/KinematicParcel/KinematicParcelIO.C b/src/lagrangian/intermediate/parcels/Templates/KinematicParcel/KinematicParcelIO.C
index 79ef7f2..da3bfe4 100644
--- a/src/lagrangian/intermediate/parcels/Templates/KinematicParcel/KinematicParcelIO.C
+++ b/src/lagrangian/intermediate/parcels/Templates/KinematicParcel/KinematicParcelIO.C
@@ -28,6 +28,13 @@ License
 #include "IOField.H"
 #include "Cloud.H"
 
+#if defined(__clang__)
+#pragma clang diagnostic ignored "-Winvalid-offsetof"
+#endif
+#if defined(__GNUC__)
+#pragma GCC diagnostic ignored "-Winvalid-offsetof"
+#endif
+
 // * * * * * * * * * * * * * * Static Data Members * * * * * * * * * * * * * //
 
 template<class ParcelType>
diff --git a/src/lagrangian/intermediate/parcels/Templates/ThermoParcel/ThermoParcelIO.C b/src/lagrangian/intermediate/parcels/Templates/ThermoParcel/ThermoParcelIO.C
index 0cf0be0..3374ab0 100644
--- a/src/lagrangian/intermediate/parcels/Templates/ThermoParcel/ThermoParcelIO.C
+++ b/src/lagrangian/intermediate/parcels/Templates/ThermoParcel/ThermoParcelIO.C
@@ -26,6 +26,13 @@ License
 #include "ThermoParcel.H"
 #include "IOstreams.H"
 
+#if defined(__clang__)
+#pragma clang diagnostic ignored "-Winvalid-offsetof"
+#endif
+#if defined(__GNUC__)
+#pragma GCC diagnostic ignored "-Winvalid-offsetof"
+#endif
+
 // * * * * * * * * * * * * * * Static Data Members * * * * * * * * * * * * * //
 
 template<class ParcelType>
diff --git a/src/lagrangian/molecularDynamics/molecule/molecule/moleculeIO.C b/src/lagrangian/molecularDynamics/molecule/molecule/moleculeIO.C
index c7829e6..4128f69 100644
--- a/src/lagrangian/molecularDynamics/molecule/molecule/moleculeIO.C
+++ b/src/lagrangian/molecularDynamics/molecule/molecule/moleculeIO.C
@@ -27,6 +27,13 @@ License
 #include "IOstreams.H"
 #include "moleculeCloud.H"
 
+#if defined(__clang__)
+#pragma clang diagnostic ignored "-Winvalid-offsetof"
+#endif
+#if defined(__GNUC__)
+#pragma GCC diagnostic ignored "-Winvalid-offsetof"
+#endif
+
 // * * * * * * * * * * * * * * Static Data Members * * * * * * * * * * * * * //
 
 const std::size_t Foam::molecule::sizeofFields_
diff --git a/src/renumber/SloanRenumber/SloanRenumber.C b/src/renumber/SloanRenumber/SloanRenumber.C
index 45eec70..6a5ed7b 100644
--- a/src/renumber/SloanRenumber/SloanRenumber.C
+++ b/src/renumber/SloanRenumber/SloanRenumber.C
@@ -35,7 +35,22 @@ License
 #include <vector>
 #include <iostream>
 #include <boost/graph/adjacency_list.hpp>
+// Disabling warning for unused parameters in Boost
+#if defined(__clang__)
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunused-parameter"
+#endif
+#if defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-parameter"
+#endif
 #include <boost/graph/sloan_ordering.hpp>
+#if defined(__clang__)
+#pragma clang diagnostic pop
+#endif
+#if defined(__GNUC__)
+#pragma GCC diagnostic pop
+#endif
 #include <boost/graph/properties.hpp>
 #include <boost/graph/bandwidth.hpp>
 #include <boost/graph/profile.hpp>
diff --git a/wmake/src/wmkdep.l b/wmake/src/wmkdep.l
index cf0be5e..0c63eb8 100644
--- a/wmake/src/wmkdep.l
+++ b/wmake/src/wmkdep.l
@@ -54,6 +54,15 @@ void importDir(const char* dirName);
 
 #   undef yywrap        /* sometimes a macro by default */
 
+#if defined(__clang__)
+#pragma clang diagnostic ignored "-Wunneeded-internal-declaration"
+#pragma clang diagnostic ignored "-Wunused-function"
+#endif
+#if defined(__GNUC__)
+#pragma GCC diagnostic ignored "-Wunneeded-internal-declaration"
+#pragma GCC diagnostic ignored "-Wunused-function"
+#endif
+
 %}
 
 %x CMNT CFNAME SCFNAME JFNAME FFNAME
OpenFOAM-dev-pragmas.patch (9,722 bytes)   

henry

2015-08-01 14:12

manager   ~0005154

> Currently compiler warnings are suppressed with -Wno-... style flags, maybe it would be better to suppress individual warnings using pragma mechanism (which is supported at least by gcc and clang) instead of global switch?

I am OK with the global switches; they are used to suppress warning we are not interested in. The alternative would be to switch on individual warnings rather than using -Wall -Wextra and switching off the few we are not interested in.

Is the pragma mechanism also supported by all other C++ compilers people used for OpenFOAM, in particular the latest Intel compiler?

> Currently biggest source of warning is unused parameters.

I do not see the issue with unused parameters; the compiler simply removes them and their presence helps with undertanding the function prototype. Warnings are only generated because -Wall -Wextra switches it on; if we do not use these are switch off this particular warning the issue goes away.

alexeym

2015-08-03 15:16

reporter   ~0005175

> Is the pragma mechanism also supported by all other C++ compilers people used for OpenFOAM, in particular the latest Intel compiler?

According to https://software.intel.com/fr-fr/node/524560, ICC supports 'pragma warning'

> I do not see the issue with unused parameters; the compiler simply removes them and their presence helps with undertanding the function prototype. Warnings are only generated because -Wall -Wextra switches it on; if we do not use these are switch off this particular warning the issue goes away.

Warnings are another safety layer, preventing certain types of bugs. In case of unused parameters they are in general typos (for example, when we have class property name with underscore and parameter with the same name and without underscore, it is easy to lose/add extra underscore). Since compiler provides this diagnostics why not use it. And all the places where this is intentionally suppressed will be marked with pragmas, so it will be easy to find them.

henry

2015-08-03 15:55

manager   ~0005177

I think the pragma method is very messy; a separate set of warning suppressions would be needed for each compiler adding clutter to the code which would need to be added to for each additional compiler supported. I think warning suppressions should be handled using command-line arguments.

I would consider unused parameters information just that, not a warning about dubious code. If it were a serious problem it would be flagged as an error whereas it is in fact only commented on by the compiler if all the "warning" switches are on. I think it should be considered an information switch rather than a warning switch.

alexeym

2015-08-04 08:36

reporter   ~0005183

> I think the pragma method is very messy

I agree that implementation in the patch is far from elegance, yet it could be rewritten as a separate macro in separate header, so warning suppression code looks like:

FOAM_SUPPRESS_WARNINGS("-Wunused-parameter");
#include <header_producing_warnings.h>
FOAM_RESTORE_WARNINGS;

So each new compiler will be added to the macro.

> I would consider unused parameters information just that, not a warning about dubious code. If it were a serious problem it would be flagged as an error whereas it is in fact only commented on by the compiler if all the "warning" switches are on.

In fact there is no strict separation between error and warning (and there is -Werror flag). I am not quite sure there is cases with excessive diagnostics, yet there are certain bugs that can be avoided due to additional warnings.

But since these additional warnings would be beneficial, in general, only for third-party software developers (and, I guess, for academic community, as -Wall and -Wextra flags help to filter out errors, obvious for experienced developer, and not-so-obvious for student/PhD/post-doc), feel free to close bug report.

Issue History

Date Modified Username Field Change
2015-07-30 22:45 alexeym New Issue
2015-07-30 22:45 alexeym File Added: OpenFOAM-dev-unused-parameters.patch
2015-07-30 22:45 alexeym File Added: OpenFOAM-dev-pragmas.patch
2015-08-01 14:12 henry Note Added: 0005154
2015-08-03 15:16 alexeym Note Added: 0005175
2015-08-03 15:55 henry Note Added: 0005177
2015-08-04 08:36 alexeym Note Added: 0005183
2015-08-04 08:50 henry Status new => closed
2015-08-04 08:50 henry Assigned To => henry
2015-08-04 08:50 henry Resolution open => no change required