From c38c6b6a15de0eb2f45192c18ceb7620e9172810 Mon Sep 17 00:00:00 2001 From: David Li Date: Fri, 23 Jun 2023 15:19:23 -0400 Subject: [PATCH] build(c): force C++11 for drivers for R's sake Fixes #815. Fixes #842. --- c/cmake_modules/AdbcDefines.cmake | 42 ++++++++++++++++++++++------- c/cmake_modules/BuildUtils.cmake | 13 +++++++-- c/cmake_modules/DefineOptions.cmake | 3 +++ c/driver/common/CMakeLists.txt | 1 + c/driver/postgresql/connection.cc | 6 ++--- c/validation/CMakeLists.txt | 1 + 6 files changed, 52 insertions(+), 14 deletions(-) diff --git a/c/cmake_modules/AdbcDefines.cmake b/c/cmake_modules/AdbcDefines.cmake index 25dfcbac8e..34166788b2 100644 --- a/c/cmake_modules/AdbcDefines.cmake +++ b/c/cmake_modules/AdbcDefines.cmake @@ -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 diff --git a/c/cmake_modules/BuildUtils.cmake b/c/cmake_modules/BuildUtils.cmake index df2590a905..6d6438d336 100644 --- a/c/cmake_modules/BuildUtils.cmake +++ b/c/cmake_modules/BuildUtils.cmake @@ -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 @@ -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 "$" @@ -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 "$") @@ -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 @@ -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}) diff --git a/c/cmake_modules/DefineOptions.cmake b/c/cmake_modules/DefineOptions.cmake index 42b8f4f51f..b6dd1079d2 100644 --- a/c/cmake_modules/DefineOptions.cmake +++ b/c/cmake_modules/DefineOptions.cmake @@ -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 diff --git a/c/driver/common/CMakeLists.txt b/c/driver/common/CMakeLists.txt index 33dd1c8d0a..0da24bb782 100644 --- a/c/driver/common/CMakeLists.txt +++ b/c/driver/common/CMakeLists.txt @@ -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") diff --git a/c/driver/postgresql/connection.cc b/c/driver/postgresql/connection.cc index 611cd51325..0e79f63052 100644 --- a/c/driver/postgresql/connection.cc +++ b/c/driver/postgresql/connection.cc @@ -107,7 +107,7 @@ class PqResultHelper { AdbcStatusCode Execute() { std::vector 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()); } @@ -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++; diff --git a/c/validation/CMakeLists.txt b/c/validation/CMakeLists.txt index 3c83f95c34..2f6549b5e7 100644 --- a/c/validation/CMakeLists.txt +++ b/c/validation/CMakeLists.txt @@ -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/"