Skip to content

Commit

Permalink
build(c): force C++11 for drivers for R's sake
Browse files Browse the repository at this point in the history
Fixes apache#815.
Fixes apache#842.
  • Loading branch information
lidavidm committed Jun 23, 2023
1 parent d5c7ead commit c38c6b6
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 14 deletions.
42 changes: 33 additions & 9 deletions c/cmake_modules/AdbcDefines.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,40 @@ if(CXX_LINKER_SUPPORTS_VERSION_SCRIPT)
endif()

# Set common build options
macro(adbc_configure_target TARGET)
if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
target_compile_options(${TARGET}
PRIVATE -Wall
-Werror
-Wextra
-Wpedantic
-Wno-unused-parameter
-Wunused-result)
if(NOT ADBC_BUILD_WARNING_LEVEL OR ADBC_BUILD_WARNING_LEVEL STREQUAL "")
if("${CMAKE_BUILD_TYPE}" STREQUAL "RELEASE")
set(ADBC_BUILD_WARNING_LEVEL "PRODUCTION")
else()
set(ADBC_BUILD_WARNING_LEVEL "CHECKIN")
endif()
endif()

set(ADBC_C_CXX_FLAGS)
if("${ADBC_BUILD_WARNING_LEVEL}" STREQUAL "CHECKIN")
if(MSVC)
set(ADBC_C_CXX_FLAGS /Wall /Werror)
elseif(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang"
OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang"
OR CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
set(ADBC_C_CXX_FLAGS -Wall -Werror)
else()
message(WARNING "Unknown compiler: ${CMAKE_CXX_COMPILER_ID}")
endif()
elseif("${ADBC_BUILD_WARNING_LEVEL}" STREQUAL "PRODUCTION")
if(MSVC)
set(CMAKE_C_FLAGS /Wall)
set(ADBC_C_CXX_FLAGS /Wall)
elseif(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang"
OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang"
OR CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
set(ADBC_C_CXX_FLAGS -Wall)
else()
message(WARNING "Unknown compiler: ${CMAKE_CXX_COMPILER_ID}")
endif()
endif()

macro(adbc_configure_target TARGET)
target_compile_options(${TARGET} PRIVATE ${ADBC_C_CXX_FLAGS})
endmacro()

# Common testing setup
Expand Down
13 changes: 11 additions & 2 deletions c/cmake_modules/BuildUtils.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@ function(ADD_ARROW_LIB LIB_NAME)
target_link_libraries(${LIB_NAME}_objlib
PRIVATE ${ARG_SHARED_LINK_LIBS} ${ARG_SHARED_PRIVATE_LINK_LIBS}
${ARG_STATIC_LINK_LIBS})
adbc_configure_target(${LIB_NAME}_objlib)
# https://github.com/apache/arrow-adbc/issues/81
target_compile_features(${LIB_NAME}_objlib PRIVATE cxx_std_11)
set_property(TARGET ${LIB_NAME}_objlib PROPERTY CXX_STANDARD 11)
else()
# Prepare arguments for separate compilation of static and shared libs below
# TODO: add PCH directives
Expand Down Expand Up @@ -255,6 +259,9 @@ function(ADD_ARROW_LIB LIB_NAME)
VERSION "${ADBC_FULL_SO_VERSION}"
SOVERSION "${ADBC_SO_VERSION}")

# https://github.com/apache/arrow-adbc/issues/81
target_compile_features(${LIB_NAME}_shared PRIVATE cxx_std_11)

target_link_libraries(${LIB_NAME}_shared
LINK_PUBLIC
"$<BUILD_INTERFACE:${ARG_SHARED_LINK_LIBS}>"
Expand Down Expand Up @@ -342,6 +349,9 @@ function(ADD_ARROW_LIB LIB_NAME)
PROPERTIES LIBRARY_OUTPUT_DIRECTORY "${OUTPUT_PATH}"
OUTPUT_NAME ${LIB_NAME_STATIC})

# https://github.com/apache/arrow-adbc/issues/81
target_compile_features(${LIB_NAME}_static PRIVATE cxx_std_11)

if(ARG_STATIC_INSTALL_INTERFACE_LIBS)
target_link_libraries(${LIB_NAME}_static LINK_PUBLIC
"$<INSTALL_INTERFACE:${ARG_STATIC_INSTALL_INTERFACE_LIBS}>")
Expand Down Expand Up @@ -584,6 +594,7 @@ function(ADD_TEST_CASE REL_TEST_NAME)

set(TEST_PATH "${EXECUTABLE_OUTPUT_PATH}/${TEST_NAME}")
add_executable(${TEST_NAME} ${SOURCES})
adbc_configure_target(${TEST_NAME})

# With OSX and conda, we need to set the correct RPATH so that dependencies
# are found. The installed libraries with conda have an RPATH that matches
Expand Down Expand Up @@ -637,8 +648,6 @@ function(ADD_TEST_CASE REL_TEST_NAME)
add_test(${TEST_NAME} ${TEST_PATH} ${ARG_TEST_ARGUMENTS})
endif()

adbc_configure_target(${TEST_NAME})

# Add test as dependency of relevant targets
add_dependencies(all-tests ${TEST_NAME})
foreach(TARGET ${ARG_LABELS})
Expand Down
3 changes: 3 additions & 0 deletions c/cmake_modules/DefineOptions.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
#----------------------------------------------------------------------
set_option_category("Compile and link")

define_option_string(ADBC_BUILD_WARNING_LEVEL
"CHECKIN to enable Werror, PRODUCTION otherwise" "")

define_option_string(ADBC_CXXFLAGS
"Compiler flags to append when compiling ADBC C++ libraries" "")
define_option_string(ADBC_GO_BUILD_TAGS
Expand Down
1 change: 1 addition & 0 deletions c/driver/common/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
# under the License.

add_library(adbc_driver_common STATIC utils.c)
adbc_configure_target(adbc_driver_common)
set_target_properties(adbc_driver_common PROPERTIES POSITION_INDEPENDENT_CODE ON)
target_include_directories(adbc_driver_common PRIVATE "${REPOSITORY_ROOT}"
"${REPOSITORY_ROOT}/c/vendor")
Expand Down
6 changes: 3 additions & 3 deletions c/driver/postgresql/connection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class PqResultHelper {
AdbcStatusCode Execute() {
std::vector<const char*> param_c_strs;

for (auto index = 0; index < param_values_.size(); index++) {
for (size_t index = 0; index < param_values_.size(); index++) {
param_c_strs.push_back(param_values_[index].c_str());
}

Expand Down Expand Up @@ -375,8 +375,8 @@ class PqGetObjectsHelper {
const char** table_types = table_types_;
while (*table_types != NULL) {
auto table_type_str = std::string(*table_types);
if (auto search = kPgTableTypes.find(table_type_str);
search != kPgTableTypes.end()) {
auto search = kPgTableTypes.find(table_type_str);
if (search != kPgTableTypes.end()) {
table_type_filter.push_back(search->second);
}
table_types++;
Expand Down
1 change: 1 addition & 0 deletions c/validation/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
# under the License.

add_library(adbc_validation OBJECT adbc_validation.cc adbc_validation_util.cc)
adbc_configure_target(adbc_validation)
target_compile_features(adbc_validation PRIVATE cxx_std_17)
target_include_directories(adbc_validation SYSTEM
PRIVATE "${REPOSITORY_ROOT}" "${REPOSITORY_ROOT}/c/driver/"
Expand Down

0 comments on commit c38c6b6

Please sign in to comment.