pcl icon indicating copy to clipboard operation
pcl copied to clipboard

[common] #pragma warning(default: 4201) in point_types.hpp causes confusing behavior

Open jasjuang opened this issue 4 years ago • 0 comments

Environment

  • Operating system (Windows/Mac/Linux, 32/64 bits): Windows 10
  • Compiler: Visual Studio 2019

Steps to reproduce

vcpkg install pcl

CMakeLists.txt

set(CMAKE_TOOLCHAIN_FILE $ENV{VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake)

cmake_minimum_required(VERSION 3.20)

project(example)

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /W4 /WX /wd4201")

find_package(PCL REQUIRED)

add_executable(${PROJECT_NAME} main.cpp)

target_link_libraries(${PROJECT_NAME} PUBLIC ${PCL_LIBRARIES})

main.cpp

#include "pcl/point_types.h"

static union { 
    float data[4]; 
    struct { 
      float x; 
      float y; 
      float z; 
    }; 
};

int main() { return 0; }

This results in compilation error:

1>C:\Users\jasju\Desktop\test\main.cpp(9,6): warning C4201: nonstandard extension used: nameless struct/union
1>Done building project "example.vcxproj" -- FAILED.

which was very confusing because /wd4201 was specified in the compilation flag, initially I thought it was a Visual Studio bug, but when I was recreating a minimal example, I realized if I simply comment out #include "pcl/point_types.h" it actually compiles, so I decided to look into point_types.h and then I realized it first disables warning 4201 at https://github.com/PointCloudLibrary/pcl/blob/master/common/include/pcl/point_types.h#L53 and then it re-enable warning 4201 back at https://github.com/PointCloudLibrary/pcl/blob/master/common/include/pcl/impl/point_types.hpp#L2481, which causes the /wd4201 flag set by the downstream project useless.

Arbitrarily disabling and enabling compilation flags in the headers that will be distributed to the downstream seems like a bad practice that will cause weird issues like this. Would it be possible for PCL to remove the warning flags alteration in the headers and add /wd4201 into it's own CMAKE_CXX_FLAGS instead?

jasjuang avatar Nov 18 '21 04:11 jasjuang