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.

Sign up for GitHub

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

Jump to bottom

Propagate CXX version used to compile abseil #1117

Closed
wants to merge 2 commits into from
Closed

Propagate CXX version used to compile abseil #1117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
67592e3
propagate C++ standard used to compile as requirement
h-vetinari Mar 2, 2022
eb5b39b
also set target_compile_features for abseil_dll
h-vetinari Mar 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions CMake/AbseilDll.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,25 @@ function(absl_make_dll)
PRIVATE
${ABSL_DEFAULT_LINKOPTS}
)

if(ABSL_PROPAGATE_CXX_STD)
# Abseil libraries require C++11 as the current minimum standard.
# Top-level application CMake projects should ensure a consistent C++
# standard for all compiled sources by setting CMAKE_CXX_STANDARD.
target_compile_features(abseil_dll PUBLIC cxx_std_${ABSL_CXX_STANDARD})
Copy link

Choose a reason for hiding this comment

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

Is ABSL_CXX_STANDARD always defined? I think the default is set here:

abseil-cpp/absl/copts/AbseilConfigureCopts.cmake

Line 97 in 4a1ccf1

set(ABSL_CXX_STANDARD "${CMAKE_CXX_STANDARD}")

And that looks like it can be the empty string? And if is the empty string, then the C++ standard in effect depends on the compiler (and compiler version).

Copy link
Contributor

Choose a reason for hiding this comment

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

@coryan see the example patterns in the Abseil CMake README here:
https://github.com/abseil/abseil-cpp/tree/master/CMake#step-by-step-instructions

I think library authors (including Abseil itself) should NOT set CMAKE_CXX_STANDARD, which should be left up to the end application project.

When a library project is being directly developed or CI tested, then it can safely set those things without affecting its clients (which is the pattern the if(CMAKE_SOURCE_DIR STREQUAL my_lib_project_SOURCE_DIR) check in the 2nd README example shows; it can also be set via the cmake command-line in build scripts).

else()
# Note: This is legacy (before CMake 3.8) behavior. Setting the
# target-level CXX_STANDARD property to ABSL_CXX_STANDARD (which is
# initialized by CMAKE_CXX_STANDARD) should have no real effect, since
# that is the default value anyway.
#
# CXX_STANDARD_REQUIRED does guard against the top-level CMake project
# not having enabled CMAKE_CXX_STANDARD_REQUIRED (which prevents
# "decaying" to an older standard if the requested one isn't available).
set_property(TARGET abseil_dll PROPERTY CXX_STANDARD ${ABSL_CXX_STANDARD})
set_property(TARGET abseil_dll PROPERTY CXX_STANDARD_REQUIRED ON)
endif()

set_property(TARGET abseil_dll PROPERTY LINKER_LANGUAGE "CXX")
target_include_directories(
abseil_dll
Expand Down
4 changes: 2 additions & 2 deletions CMake/AbseilHelpers.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ Cflags: -I\${includedir}${PC_CFLAGS}\n")
# Abseil libraries require C++11 as the current minimum standard.
# Top-level application CMake projects should ensure a consistent C++
# standard for all compiled sources by setting CMAKE_CXX_STANDARD.
target_compile_features(${_NAME} PUBLIC cxx_std_11)
target_compile_features(${_NAME} PUBLIC cxx_std_${ABSL_CXX_STANDARD})
else()
# Note: This is legacy (before CMake 3.8) behavior. Setting the
# target-level CXX_STANDARD property to ABSL_CXX_STANDARD (which is
Expand Down Expand Up @@ -420,7 +420,7 @@ function(absl_cc_test)
# Abseil libraries require C++11 as the current minimum standard.
# Top-level application CMake projects should ensure a consistent C++
# standard for all compiled sources by setting CMAKE_CXX_STANDARD.
target_compile_features(${_NAME} PUBLIC cxx_std_11)
target_compile_features(${_NAME} PUBLIC cxx_std_${ABSL_CXX_STANDARD})
else()
# Note: This is legacy (before CMake 3.8) behavior. Setting the
# target-level CXX_STANDARD property to ABSL_CXX_STANDARD (which is
Expand Down