open-mpi / ompi

Open MPI main development repository
https://www.open-mpi.org
Other
2.18k stars 861 forks source link

warning: use of old-style cast to ‘void*’ [-Wold-style-cast] #6316

Closed visiblehawk closed 5 years ago

visiblehawk commented 5 years ago

Thank you for taking the time to submit an issue!

Background information

What version of Open MPI are you using? (e.g., v1.10.3, v2.1.0, git branch name and hash, etc.)

openmpi-4.0.0

Describe how Open MPI was installed (e.g., from a source/distribution tarball, from a git clone, from an operating system distribution package, etc.)

Compiled from a source/distribution tarball.

Please describe the system on which you are running


Details of the problem

This macro in mpi.h produces so many warnings about old-style-cast in any code using OpenMPI, e.g. OpenFOAM opensource package.

#if !OMPI_BUILDING
#define OMPI_PREDEFINED_GLOBAL(type, global) ((type) ((void *) &(global)))
#else
#define OMPI_PREDEFINED_GLOBAL(type, global) ((type) &(global))
#endif

the warnings:

In file included from db/IOstreams/Pstreams/OPwrite.C:29:
db/IOstreams/Pstreams/OPwrite.C: In static member function ‘static bool Foam::OPstream::write(Foam::Pstream::commsTypes, int, const char*, std::streamsize, int, Foam::label)’:
/home/shadowfax/foam/foam-extend-4.1/ThirdParty/packages/openmpi-4.0.0/platforms/linux64GccDPOpt/include/mpi.h:329:72: warning: use of old-style cast to ‘void*’ [-Wold-style-cast]
#define OMPI_PREDEFINED_GLOBAL(type, global) ((type) ((void *) &(global)))
                                                                    ^
/home/shadowfax/foam/foam-extend-4.1/ThirdParty/packages/openmpi-4.0.0/platforms/linux64GccDPOpt/include/mpi.h:1049:18: note: in expansion of macro ‘OMPI_PREDEFINED_GLOBAL’
#define MPI_BYTE OMPI_PREDEFINED_GLOBAL(MPI_Datatype, ompi_mpi_byte)
              ^~~~~~~~~~~~~~~~~~~~~~
db/IOstreams/Pstreams/OPwrite.C:101:13: note: in expansion of macro ‘MPI_BYTE’
         MPI_BYTE,
jsquyres commented 5 years ago

@Shadow-fax What compiler and version are you using?

visiblehawk commented 5 years ago

Dear jsquyres,

I'm using GCC version 8.2 on Ubuntu 18.10

jsquyres commented 5 years ago

So I think you're saying that you're compiling mpi.h with a C++ compiler, and it's warning about (void*)-style casts instead of static_cast<void*>, right?

If that's the case, I am not a C++ expert: do all C++ compilers support static_cast<void*>? If not, is there a compile-time way to detect of C++ compiler support static_cast<void*> or not?

visiblehawk commented 5 years ago

The OpenFOAM package use openmpi to manage parallel operations, and it includes mpi.h from openmpi package anywhere it needs. The source code of OpenFOAM needs to be compiled with C++ compiler and that is why anywhere it includes and uses mpi.h function, the c++ compiler warns about the usage of old-style-cast.

yes, it should be static_cast<void> instead of (void). I think every modern c++ compiler should support that because it is not new feature...

I have already tried to recompile openmpi-4.0.0 with the following modification, however it does not compile with the "mpi.h:329:55: error: 'static_cast' undeclared (first use in this function)" error.

#define OMPI_PREDEFINED_GLOBAL(type, global) ((type) (static_cast<void *>( &(global))))
jsquyres commented 5 years ago

Open MPI probably fails to compile because it is compiled with a C compiler, not a C++ compiler. At a minimum, I think it would need to be something like this:

#if !OMPI_BUILDING
#if defined(c_plusplus) || defined(__cplusplus)
#define OMPI_PREDEFINED_GLOBAL(type, global) ((static_cast<type>) ((static_cast<void *>) &(global)))
#else
#define OMPI_PREDEFINED_GLOBAL(type, global) ((type) ((void *) &(global)))
#endif
#else
#define OMPI_PREDEFINED_GLOBAL(type, global) ((type) &(global))
#endif

But Open MPI supports fairly old compilers, too. I'm concerned that we'll need something a bit more stringent than just #if defined(c_plusplus) || defined(__cplusplus) to protect the static_cast<> usage.

...that being said, I can easily test as far back as gcc/g++ 4.4.7 (i.e., RHEL 6), and it seems to work. I'll make up a PR and let's see if others can try with ancient c++ compilers and see what happens.