Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compilation error with MSYS2 mingw-w64, 'M_PI' was not defined in this scope #3679

Closed
FSund opened this issue Feb 26, 2020 · 9 comments · Fixed by #3756
Closed

Compilation error with MSYS2 mingw-w64, 'M_PI' was not defined in this scope #3679

FSund opened this issue Feb 26, 2020 · 9 comments · Fixed by #3756

Comments

@FSund
Copy link
Contributor

FSund commented Feb 26, 2020

When compiling pcl on Windows using MSYS2 mimgw-w64 I get a lot of 'M_PI' was not declared in this scope errors.

Full error log

[  0%] Building CXX object common/CMakeFiles/pcl_common.dir/src/bearing_angle_image.cpp.obj
cd /C/msys64/home/filip.sund/code/pcl-fsund/build/common && /C/msys64/mingw64/bin/g++.exe  -DBOOST_LIB_DIAGNOSTIC -DBOOST_THREAD_USE_LIB -DPCLAPI_EXPORTS -DvtkRenderingContext2D_AUTOINIT="1(vtkRenderingContextOpenGL2)" -DvtkRenderingCore_AUTOINIT="3(vtkInteractionStyle,vtkRenderingFreeType,vtkRenderingOpenGL2)" -I/C/msys64/mingw64/include/vtk-8.2 -I/C/msys64/mingw64/include/freetype2 -I/C/msys64/mingw64/include/double-conversion -I/C/msys64/home/filip.sund/code/pcl-fsund/build/include -I/C/msys64/home/filip.sund/code/pcl-fsund/common/include -isystem /C/msys64/mingw64/include/eigen3 -isystem /C/msys64/home/filip.sund/code/pcl-fsund/recognition/include/pcl/recognition/3rdparty  -Wabi=11 -Wall -Wextra -Wno-unknown-pragmas -fno-strict-aliasing -Wno-format-extra-args -Wno-sign-compare -Wno-invalid-offsetof -Wno-conversion -march=native -msse4.2 -mfpmath=sse  -mwin32 -mthreads -O3 -DNDEBUG   -DBOOST_DISABLE_ASSERTS -DEIGEN_NO_DEBUG -std=c++14 -o CMakeFiles/pcl_common.dir/src/bearing_angle_image.cpp.obj -c /C/msys64/home/filip.sund/code/pcl-fsund/common/src/bearing_angle_image.cpp
C:/msys64/home/filip.sund/code/pcl-fsund/common/src/bearing_angle_image.cpp: In member function 'double pcl::BearingAngleImage::getAngle(const pcl::PointXYZ&, const pcl::PointXYZ&)':
C:/msys64/home/filip.sund/code/pcl-fsund/common/src/bearing_angle_image.cpp:78:73: error: 'M_PI' was not declared in this scope
   78 |     theta = std::acos ((a + b - c) / (2 * sqrt (a) * sqrt (b))) * 180 / M_PI;
      |                                                                         ^~~~
make[2]: *** [common/CMakeFiles/pcl_common.dir/build.make:258: common/CMakeFiles/pcl_common.dir/src/bearing_angle_image.cpp.obj] Error 1
make[2]: Leaving directory '/home/filip.sund/code/pcl-fsund/build'
make[1]: *** [CMakeFiles/Makefile2:1033: common/CMakeFiles/pcl_common.dir/all] Error 2
make[1]: Leaving directory '/home/filip.sund/code/pcl-fsund/build'
make: *** [Makefile:152: all] Error 2

The first error is in bearing_angle_image.cpp, but I see M_PI is used in many other places as well.

Exact steps to reproduce are:

  • Install MSYS2 and all dependencies on Windows 10
  • Clone pcl
cd pcl
mkdir build
cd build
cmake -G"MSYS Makefiles" -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/usr ..
make

How to fix

What I did was to just add a top-level add_definitions(-D_USE_MATH_DEFINES) to CMakeLists.txt, but I realize this is not the proper way to fix this.

I see some trickery is happening on Windows in pcl_macros.h, but I am not certain if there is a similar way to do it for MSYS2.

I think the root issue is that the CMake command set(CMAKE_CXX_STANDARD 14) causes the -std=gnu++14 flag to be added on Linux, but on Windows+MSYS2 the -std=c++14 flag is added instead. This that M_PI is not defined on MSYS2, since that macro is not part of the C++14 standard. This is not true. Since we have set(CMAKE_CXX_EXTENSIONS OFF) Linux, the flag is the same on Linux and Windows/MSYS2.

Your Environment

  • Operating System and version: Windows 10, Version 10.0.18363 Build 18363
  • Compiler: g++.exe (Rev2, Built by MSYS2 project) 9.2.0
  • PCL Version: commit b1d9c7e
@kunaltyagi
Copy link
Member

I think mingw is simply broken due to assumption of msvc. Since it seems you are using MinGW, could you please help to improve the support on CI? That'd be a nice help apart from PR to resolve the issues.

@FSund
Copy link
Contributor Author

FSund commented Feb 26, 2020

What assumption of MSVC do you mean? That Windows == MSVC?

I have no experience with CI unfortunately.

@taketwo
Copy link
Member

taketwo commented Feb 26, 2020

Regarding your fix idea, we already have that define in pcl_macros.h:

#ifndef _USE_MATH_DEFINES
#define _USE_MATH_DEFINES

Regarding gnu++14, is this true? Because we have set(CMAKE_CXX_EXTENSIONS OFF) in our CMakeLists, so extensions should not be used.

@kunaltyagi
Copy link
Member

kunaltyagi commented Feb 26, 2020

That Windows == MSVC?

Yeah. We compile and test on Windows using only MSVC

Regarding gnu++14, is this true?

No, I tested and PCL definitely uses c++14 and not gnu++14 as the compiler flag

I have no experience with CI unfortunately

It's not a big issue. I'd suggest to look at the yaml files in the .ci folder. The mingw would be a hybrid between Windows and Ubuntu-16 yaml files. If you document what commands you run to compile and test, the CI is essentially exactly those commands.

@FSund
Copy link
Contributor Author

FSund commented Feb 27, 2020

Sorry, I was just assuming the gnu++14 flag was being added on Linux, didn't realize you had set(CMAKE_CXX_EXTENSIONS OFF).

It's kind of strange the #define _USE_MATH_DEFINES in pcl_macros.h doesn't seem to be registering when I'm compiling. I'll have a look into

I'll have a look at the yaml files and try to make a PR after I've fixed the compilation issues I'm having.

@FSund
Copy link
Contributor Author

FSund commented Feb 27, 2020

I think I might be on to something. The first 'M_PI' was not declared error I get is in bearing_angle_image.cpp at line 78

theta = std::acos ((a + b - c) / (2 * sqrt (a) * sqrt (b))) * 180 / M_PI;

Now, the first include in this file is

#include <pcl/common/eigen.h>

and the first lines of eigen.h are

#pragma once
#ifndef NOMINMAX
#define NOMINMAX
#endif
#if defined __GNUC__
# pragma GCC system_header
#elif defined __SUNPRO_CC
# pragma disable_warn
#endif
#include <cmath>

So this is the first time the cmath header is included in the context of bearing_angle_image.cpp, and no _USE_MATH_DEFINES has been defined so far. This means that M_PI should not be available at all in bearing_angle_image.cpp, or am I misunderstanding something?

I'm not sure why compilation works on other platforms/compilers though, since the code would be the same for those.

Now if we just swap L55 and L56 of eigen.h

#include <cmath>
#include <pcl/ModelCoefficients.h>

it seems like the error would go away, since ModelCoefficients.h includes pcl_macros.h (a couple of includes deep) before the cmath header is included. But I'm still not sure if this is the actual issue here, since compilation works on other platforms. EDIT: Or just remove L55, since cmath is included deeper down anyway, with the correct define ahead of it.

@kunaltyagi
Copy link
Member

Could you check if #3545 fixes this issue?

@FSund
Copy link
Contributor Author

FSund commented Feb 27, 2020

I'll check that PR asap, but I've made several discoveries lately:

  • Seems like Eigen includes cmath in some places, so you need to be very careful of the order in which Eigen is included, since any includes after that (even with _USE_MATH_DEFINES) will not lead to M_PI being defined.

  • You already have the necessary M_PI and similar defines in pcl_macros.h, but they are only enabled for Windows+MSVC. Instead of relying on M_PI being defined in external headers I would just enable this code for all platforms, or perhaps put the definitions in a separate file. Any reasons why this would be a bad idea?

    #ifndef _MATH_DEFINES_DEFINED
    #define _MATH_DEFINES_DEFINED
    #define M_E 2.71828182845904523536 // e
    #define M_LOG2E 1.44269504088896340736 // log2(e)
    #define M_LOG10E 0.434294481903251827651 // log10(e)
    #define M_LN2 0.693147180559945309417 // ln(2)
    #define M_LN10 2.30258509299404568402 // ln(10)
    #define M_PI 3.14159265358979323846 // pi
    #define M_PI_2 1.57079632679489661923 // pi/2
    #define M_PI_4 0.785398163397448309616 // pi/4
    #define M_1_PI 0.318309886183790671538 // 1/pi
    #define M_2_PI 0.636619772367581343076 // 2/pi
    #define M_2_SQRTPI 1.12837916709551257390 // 2/sqrt(pi)
    #define M_SQRT2 1.41421356237309504880 // sqrt(2)
    #define M_SQRT1_2 0.707106781186547524401 // 1/sqrt(2)
    #endif

    I changed
    #if defined _WIN32 && defined _MSC_VER

    to

    #if defined _WIN32 && (defined _MSC_VER || defined __MINGW32__)
    

    and everything compiled fine with mingw-w64 (as long as I remember add_definitions(-DPCL_ONLY_CORE_POINT_TYPES)).

@FSund
Copy link
Contributor Author

FSund commented Feb 27, 2020

#3545 did not fix the issue, compilation fails in the same place as before

Building CXX object common/CMakeFiles/pcl_common.dir/src/bearing_angle_image.cpp.obj
cd /C/msys64/home/filip.sund/code/pcl/build/common && /C/msys64/mingw64/bin/g++.exe  -DBOOST_LIB_DIAGNOSTIC -DBOOST_THREAD_USE_LIB -DPCLAPI_EXPORTS -DPCL_ONLY_CORE_POINT_TYPES -DvtkRenderingContext2D_AUTOINIT="1(vtkRenderingContextOpenGL2)" -DvtkRenderingCore_AUTOINIT="3(vtkInteractionStyle,vtkRenderingFreeType,vtkRenderingOpenGL2)" -I/C/msys64/mingw64/include/vtk-8.2 -I/C/msys64/mingw64/include/freetype2 -I/C/msys64/mingw64/include/double-conversion -I/C/msys64/home/filip.sund/code/pcl/build/include -I/C/msys64/home/filip.sund/code/pcl/common/include -isystem /C/msys64/mingw64/include/eigen3 -isystem /C/msys64/home/filip.sund/code/pcl/recognition/include/pcl/recognition/3rdparty  -Wabi=11 -Wall -Wextra -Wno-unknown-pragmas -fno-strict-aliasing -Wno-format-extra-args -Wno-sign-compare -Wno-invalid-offsetof -Wno-conversion -march=native -msse4.2 -mfpmath=sse  -mwin32 -mthreads -O3 -DNDEBUG   -DBOOST_DISABLE_ASSERTS -DEIGEN_NO_DEBUG -std=c++14 -o CMakeFiles/pcl_common.dir/src/bearing_angle_image.cpp.obj -c /C/msys64/home/filip.sund/code/pcl/common/src/bearing_angle_image.cpp
C:/msys64/home/filip.sund/code/pcl/common/src/bearing_angle_image.cpp: In member function 'double pcl::BearingAngleImage::getAngle(const pcl::PointXYZ&, const pcl::PointXYZ&)':
C:/msys64/home/filip.sund/code/pcl/common/src/bearing_angle_image.cpp:84:73: error: 'M_PI' was not declared in this scope
   84 |     theta = std::acos ((a + b - c) / (2 * sqrt (a) * sqrt (b))) * 180 / M_PI;
      |                                                                         ^~~~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants