PointCloudLibrary / PointCloudLibrary/pcl

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

Open
#5,040 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

kind: bug status: triage
Dominant language
C++
Stars
11.1k
Forks
4.7k
Avg merge
4d 10h
Merged PRs (30d)
6

Description

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?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Inspect common/include/pcl/point_types.h and common/include/pcl/impl/point_types.hpp at the warning-pragmas described in the issue. Reproduce the Windows example with /W4 /WX /wd4201, then verify that including pcl/point_types.h no longer overrides the downstream warning setting. There are no tests or follow-up comments mentioned in the payload.

Written by the indexing model from the issue text.

Assessment

Tech stack
cmake, cpp
Domain
build-system
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.