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

Remove CXX_STD and inherit CMAKE_CXX_STANDARD from parent project #2005

Closed
wants to merge 2 commits into from

Conversation

mhx
Copy link
Contributor

@mhx mhx commented May 26, 2023

This follows up on a discussion in #1949 and the comment by @ot that the C++ standard for folly should be (easily) settable by a parent project.

There are two commits in this PR:

  • The first commit removes the redundant CXX_STD, which causes -std= to be passed to the compiler twice.
  • The second commit introduces a check before setting CMAKE_CXX_STANDARD, so a value set by a parent project won't be overridden.

mhx added 2 commits May 26, 2023 23:23
The comment states that "We don't use CMAKE_CXX_STANDARD [...]",
however, the toplevel CMakeLists.txt sets `CMAKE_CXX_STANDARD`,
which makes the use of `CXX_STD` redundant. When checking the
actual compiler invocation, two distinct `-std=` options are
passed to the compiler as a result.
This allows projects that include folly through `add_subdirectory`
to be built with a different (newer) standard and have folly inherit
the standard from the parent project.
@mhx
Copy link
Contributor Author

mhx commented Jun 20, 2023

ping

set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
if(NOT CMAKE_CXX_STANDARD)
set(CMAKE_CXX_STANDARD 17)
Copy link

@eugene536 eugene536 Jul 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw) we can set CMAKE_CXX_STANDARD as a CACHE variable and remove the if

set(CMAKE_CXX_STANDARD "17" CACHE STRING  "folly C++ standard")

@facebook-github-bot
Copy link
Contributor

@Orvid has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@Orvid merged this pull request in 47d454c.

facebook-github-bot pushed a commit to facebook/hhvm that referenced this pull request Jul 31, 2023
Summary:
This follows up on a discussion in facebook/folly#1949 and the comment by ot that the C++ standard for folly should be (easily) settable by a parent project.

There are two commits in this PR:

- The first commit removes the redundant `CXX_STD`, which causes `-std=` to be passed to the compiler twice.
- The second commit introduces a check before setting `CMAKE_CXX_STANDARD`, so a value set by a parent project won't be overridden.

X-link: facebook/folly#2005

Reviewed By: Gownta

Differential Revision: D47343926

Pulled By: Orvid

fbshipit-source-id: a712d117a6d6e2d125ee71f5f8d47ee5ceb3106b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants