You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by li...@apache.org on 2023/06/24 16:27:03 UTC
[arrow-adbc] branch main updated: build(c): force C++11 for drivers for R's sake (#844)
This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
The following commit(s) were added to refs/heads/main by this push:
new 518f21d0 build(c): force C++11 for drivers for R's sake (#844)
518f21d0 is described below
commit 518f21d0646277f9b5402c827c83e8a7bea9e807
Author: David Li <li...@gmail.com>
AuthorDate: Sat Jun 24 12:26:58 2023 -0400
build(c): force C++11 for drivers for R's sake (#844)
Fixes #815.
Fixes #842.
---
c/cmake_modules/AdbcDefines.cmake | 32 +++++++++++++++++++++++---------
c/cmake_modules/BuildUtils.cmake | 17 ++++++++++++-----
c/cmake_modules/DefineOptions.cmake | 3 +++
c/driver/common/CMakeLists.txt | 1 +
c/driver/postgresql/connection.cc | 6 +++---
c/validation/CMakeLists.txt | 1 +
c/validation/adbc_validation_util.h | 3 +--
7 files changed, 44 insertions(+), 19 deletions(-)
diff --git a/c/cmake_modules/AdbcDefines.cmake b/c/cmake_modules/AdbcDefines.cmake
index 25dfcbac..466c90d8 100644
--- a/c/cmake_modules/AdbcDefines.cmake
+++ b/c/cmake_modules/AdbcDefines.cmake
@@ -60,16 +60,30 @@ 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("${ADBC_BUILD_WARNING_LEVEL}" STREQUAL "")
+ string(TOLOWER "${CMAKE_BUILD_TYPE}" _lower_build_type)
+ if("${_lower_build_type}" STREQUAL "release")
+ set(ADBC_BUILD_WARNING_LEVEL "PRODUCTION")
+ else()
+ set(ADBC_BUILD_WARNING_LEVEL "CHECKIN")
endif()
+endif()
+
+if(MSVC)
+ set(ADBC_C_CXX_FLAGS_CHECKIN /Wall /WX)
+ set(ADBC_C_CXX_FLAGS_PRODUCTION /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_CHECKIN -Wall -Werror)
+ set(ADBC_C_CXX_FLAGS_PRODUCTION -Wall)
+else()
+ message(WARNING "Unknown compiler: ${CMAKE_CXX_COMPILER_ID}")
+endif()
+
+macro(adbc_configure_target TARGET)
+ target_compile_options(${TARGET}
+ PRIVATE ${ADBC_C_CXX_FLAGS_${ADBC_BUILD_WARNING_LEVEL}})
endmacro()
# Common testing setup
diff --git a/c/cmake_modules/BuildUtils.cmake b/c/cmake_modules/BuildUtils.cmake
index df2590a9..de1a7b2a 100644
--- a/c/cmake_modules/BuildUtils.cmake
+++ b/c/cmake_modules/BuildUtils.cmake
@@ -166,7 +166,6 @@ function(ADD_ARROW_LIB LIB_NAME)
add_library(${LIB_NAME}_objlib OBJECT ${ARG_SOURCES})
# Necessary to make static linking into other shared libraries work properly
set_property(TARGET ${LIB_NAME}_objlib PROPERTY POSITION_INDEPENDENT_CODE 1)
- set_property(TARGET ${LIB_NAME}_objlib PROPERTY CXX_STANDARD 17)
set_property(TARGET ${LIB_NAME}_objlib PROPERTY CXX_STANDARD_REQUIRED ON)
if(ARG_DEPENDENCIES)
add_dependencies(${LIB_NAME}_objlib ${ARG_DEPENDENCIES})
@@ -194,6 +193,9 @@ 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)
else()
# Prepare arguments for separate compilation of static and shared libs below
# TODO: add PCH directives
@@ -209,7 +211,7 @@ function(ADD_ARROW_LIB LIB_NAME)
if(BUILD_SHARED)
add_library(${LIB_NAME}_shared SHARED ${LIB_DEPS})
- set_property(TARGET ${LIB_NAME}_shared PROPERTY CXX_STANDARD 17)
+ target_compile_features(${LIB_NAME}_shared PRIVATE cxx_std_11)
set_property(TARGET ${LIB_NAME}_shared PROPERTY CXX_STANDARD_REQUIRED ON)
adbc_configure_target(${LIB_NAME}_shared)
if(EXTRA_DEPS)
@@ -255,6 +257,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}>"
@@ -304,7 +309,7 @@ function(ADD_ARROW_LIB LIB_NAME)
if(BUILD_STATIC)
add_library(${LIB_NAME}_static STATIC ${LIB_DEPS})
- set_property(TARGET ${LIB_NAME}_shared PROPERTY CXX_STANDARD 17)
+ target_compile_features(${LIB_NAME}_static PRIVATE cxx_std_11)
set_property(TARGET ${LIB_NAME}_shared PROPERTY CXX_STANDARD_REQUIRED ON)
adbc_configure_target(${LIB_NAME}_static)
if(EXTRA_DEPS)
@@ -342,6 +347,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}>")
@@ -584,6 +592,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 +646,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 42b8f4f5..b6dd1079 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 33dd1c8d..0da24bb7 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 611cd513..0e79f630 100644
--- a/c/driver/postgresql/connection.cc
+++ b/c/driver/postgresql/connection.cc
@@ -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());
}
@@ -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 3c83f95c..2f6549b5 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/"
diff --git a/c/validation/adbc_validation_util.h b/c/validation/adbc_validation_util.h
index a239e769..fec5e758 100644
--- a/c/validation/adbc_validation_util.h
+++ b/c/validation/adbc_validation_util.h
@@ -200,7 +200,7 @@ struct StreamReader {
/// \brief Read an AdbcGetInfoData struct with RAII safety
struct GetObjectsReader {
- explicit GetObjectsReader(struct ArrowArrayView* array_view) : array_view_(array_view) {
+ explicit GetObjectsReader(struct ArrowArrayView* array_view) {
// TODO: this swallows any construction errors
get_objects_data_ = AdbcGetObjectsDataInit(array_view);
}
@@ -214,7 +214,6 @@ struct GetObjectsReader {
}
private:
- struct ArrowArrayView* array_view_;
struct AdbcGetObjectsData* get_objects_data_;
};