PointCloudLibrary / PointCloudLibrary/pcl

Consider treating user specified CMAKE_CXX_FLAGS as extra flags

Open
#2,580 3 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

module: cmake status: stale
Dominant language
C++
Stars
11.1k
Forks
4.7k
Avg merge
4d 10h
Merged PRs (30d)
6

Description

The currently implemented logic is as follows: if CMAKE_CXX_FLAGS are not set, then add some flags that PCL thinks are necessary (including architecture and SSE flags). Conversely, if CMAKE_CXX_FLAGS is set explicitly by the user, then don't touch everything, assume the user knows exactly what he wants.

This logic existed before this PR. Only thing it changed is fixed a potential nasty bug associated with it.

In general, the fact that vcpkg explicitly sets flags warrants a revision of the logic. Perhaps CMAKE_CXX_FLAGS should rather be treated as extra flags that are to be appended to all other flags set by PCL.

Originally posted by @taketwo in https://github.com/PointCloudLibrary/pcl/pull/2100#issuecomment-431847295

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

Start by tracing the existing CMAKE_CXX_FLAGS handling and the logic that adds PCL's architecture and SSE flags; compare behavior when flags are absent, explicitly set, and supplied by vcpkg. Done means the revised behavior is defined for these cases and validated without reintroducing the potential bug linked from the issue.

Written by the indexing model from the issue text.

Assessment

Tech stack
cmake, cpp
Domain
build-system
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.