From 0b50e4fb8195cc99c02818eb75c82648d7a9412d Mon Sep 17 00:00:00 2001 From: David Li Date: Fri, 8 Mar 2024 08:07:56 -0500 Subject: [PATCH] build(c): fix warnings for MSVC (#1600) This leaves warnings about dllexport/dllimport which will require further investigation. Fixes #847. --- c/cmake_modules/AdbcDefines.cmake | 10 ++++++++ c/driver/common/utils.c | 25 +++++++++---------- c/driver/common/utils.h | 5 ++-- c/driver/postgresql/connection.cc | 5 +--- c/driver/postgresql/copy/writer.h | 24 ++++++++++-------- c/driver/postgresql/statement.cc | 10 +++++--- c/driver/sqlite/sqlite.c | 3 +-- ci/scripts/python_wheel_windows_build.bat | 4 ++- .../adbc_driver_manager/_blocking_impl.cc | 4 ++- .../adbc_driver_manager/_lib.pxd | 2 +- .../adbc_driver_manager/_lib.pyx | 2 +- python/adbc_driver_manager/setup.py | 2 +- 12 files changed, 55 insertions(+), 41 deletions(-) diff --git a/c/cmake_modules/AdbcDefines.cmake b/c/cmake_modules/AdbcDefines.cmake index 5cc2eaa60b..69a8c1aa82 100644 --- a/c/cmake_modules/AdbcDefines.cmake +++ b/c/cmake_modules/AdbcDefines.cmake @@ -72,6 +72,16 @@ endif() if(MSVC) set(ADBC_C_CXX_FLAGS_CHECKIN /Wall /WX) set(ADBC_C_CXX_FLAGS_PRODUCTION /Wall) + # Don't warn about strerror_s etc. + add_compile_definitions(_CRT_SECURE_NO_WARNINGS) + # Allow incomplete switch (since MSVC warns even if there's a default case) + add_compile_options(/wd4061) + add_compile_options(/wd4868) + add_compile_options(/wd4710) + add_compile_options(/wd4711) + # Don't warn about padding added after members + add_compile_options(/wd4820) + add_compile_options(/wd5045) elseif(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "GNU") diff --git a/c/driver/common/utils.c b/c/driver/common/utils.c index bb3dd3f0e2..795d79f973 100644 --- a/c/driver/common/utils.c +++ b/c/driver/common/utils.c @@ -238,6 +238,7 @@ struct SingleBatchArrayStream { struct ArrowArray batch; }; static const char* SingleBatchArrayStreamGetLastError(struct ArrowArrayStream* stream) { + (void)stream; return NULL; } static int SingleBatchArrayStreamGetNext(struct ArrowArrayStream* stream, @@ -309,7 +310,7 @@ int StringBuilderInit(struct StringBuilder* builder, size_t initial_size) { } int StringBuilderAppend(struct StringBuilder* builder, const char* fmt, ...) { va_list argptr; - int bytes_available = builder->capacity - builder->size; + int bytes_available = (int)builder->capacity - (int)builder->size; va_start(argptr, fmt); int n = vsnprintf(builder->buffer + builder->size, bytes_available, fmt, argptr); @@ -344,9 +345,7 @@ void StringBuilderReset(struct StringBuilder* builder) { memset(builder, 0, sizeof(*builder)); } -AdbcStatusCode AdbcInitConnectionGetInfoSchema(const uint32_t* info_codes, - size_t info_codes_length, - struct ArrowSchema* schema, +AdbcStatusCode AdbcInitConnectionGetInfoSchema(struct ArrowSchema* schema, struct ArrowArray* array, struct AdbcError* error) { // TODO: use C equivalent of UniqueSchema to avoid incomplete schema @@ -796,29 +795,29 @@ struct AdbcGetObjectsData* AdbcGetObjectsDataInit(struct ArrowArrayView* array_v column->column_name = ArrowArrayViewGetStringUnsafe( get_objects_data->column_name_array, column_index); - column->ordinal_position = ArrowArrayViewGetIntUnsafe( + column->ordinal_position = (int32_t)ArrowArrayViewGetIntUnsafe( get_objects_data->column_position_array, column_index); column->remarks = ArrowArrayViewGetStringUnsafe( get_objects_data->column_remarks_array, column_index); - column->xdbc_data_type = ArrowArrayViewGetIntUnsafe( + column->xdbc_data_type = (int16_t)ArrowArrayViewGetIntUnsafe( get_objects_data->xdbc_data_type_array, column_index); column->xdbc_type_name = ArrowArrayViewGetStringUnsafe( get_objects_data->xdbc_type_name_array, column_index); - column->xdbc_column_size = ArrowArrayViewGetIntUnsafe( + column->xdbc_column_size = (int32_t)ArrowArrayViewGetIntUnsafe( get_objects_data->xdbc_column_size_array, column_index); - column->xdbc_decimal_digits = ArrowArrayViewGetIntUnsafe( + column->xdbc_decimal_digits = (int16_t)ArrowArrayViewGetIntUnsafe( get_objects_data->xdbc_decimal_digits_array, column_index); - column->xdbc_num_prec_radix = ArrowArrayViewGetIntUnsafe( + column->xdbc_num_prec_radix = (int16_t)ArrowArrayViewGetIntUnsafe( get_objects_data->xdbc_num_prec_radix_array, column_index); - column->xdbc_nullable = ArrowArrayViewGetIntUnsafe( + column->xdbc_nullable = (int16_t)ArrowArrayViewGetIntUnsafe( get_objects_data->xdbc_nullable_array, column_index); column->xdbc_column_def = ArrowArrayViewGetStringUnsafe( get_objects_data->xdbc_column_def_array, column_index); - column->xdbc_sql_data_type = ArrowArrayViewGetIntUnsafe( + column->xdbc_sql_data_type = (int16_t)ArrowArrayViewGetIntUnsafe( get_objects_data->xdbc_sql_data_type_array, column_index); - column->xdbc_datetime_sub = ArrowArrayViewGetIntUnsafe( + column->xdbc_datetime_sub = (int16_t)ArrowArrayViewGetIntUnsafe( get_objects_data->xdbc_datetime_sub_array, column_index); - column->xdbc_char_octet_length = ArrowArrayViewGetIntUnsafe( + column->xdbc_char_octet_length = (int32_t)ArrowArrayViewGetIntUnsafe( get_objects_data->xdbc_char_octet_length_array, column_index); column->xdbc_scope_catalog = ArrowArrayViewGetStringUnsafe( get_objects_data->xdbc_scope_catalog_array, column_index); diff --git a/c/driver/common/utils.h b/c/driver/common/utils.h index 1ac6a42a4d..ff75fa7208 100644 --- a/c/driver/common/utils.h +++ b/c/driver/common/utils.h @@ -20,6 +20,7 @@ #include #include #include +#include #include #include "nanoarrow/nanoarrow.h" @@ -122,9 +123,7 @@ AdbcStatusCode BatchToArrayStream(struct ArrowArray* values, struct ArrowSchema* /// Utilities for implementing connection-related functions for drivers /// /// @{ -AdbcStatusCode AdbcInitConnectionGetInfoSchema(const uint32_t* info_codes, - size_t info_codes_length, - struct ArrowSchema* schema, +AdbcStatusCode AdbcInitConnectionGetInfoSchema(struct ArrowSchema* schema, struct ArrowArray* array, struct AdbcError* error); AdbcStatusCode AdbcConnectionGetInfoAppendString(struct ArrowArray* array, diff --git a/c/driver/postgresql/connection.cc b/c/driver/postgresql/connection.cc index f7fc56d9a3..63d23fb296 100644 --- a/c/driver/postgresql/connection.cc +++ b/c/driver/postgresql/connection.cc @@ -643,8 +643,7 @@ AdbcStatusCode PostgresConnection::Commit(struct AdbcError* error) { AdbcStatusCode PostgresConnection::PostgresConnectionGetInfoImpl( const uint32_t* info_codes, size_t info_codes_length, struct ArrowSchema* schema, struct ArrowArray* array, struct AdbcError* error) { - RAISE_ADBC(AdbcInitConnectionGetInfoSchema(info_codes, info_codes_length, schema, array, - error)); + RAISE_ADBC(AdbcInitConnectionGetInfoSchema(schema, array, error)); for (size_t i = 0; i < info_codes_length; i++) { switch (info_codes[i]) { @@ -1131,8 +1130,6 @@ AdbcStatusCode PostgresConnection::GetStatisticNames(struct ArrowArrayStream* ou return status; } return BatchToArrayStream(&array, &schema, out, error); - - return ADBC_STATUS_OK; } AdbcStatusCode PostgresConnection::GetTableSchema(const char* catalog, diff --git a/c/driver/postgresql/copy/writer.h b/c/driver/postgresql/copy/writer.h index ad119ffe96..99791ad433 100644 --- a/c/driver/postgresql/copy/writer.h +++ b/c/driver/postgresql/copy/writer.h @@ -361,19 +361,21 @@ class PostgresCopyDurationFieldWriter : public PostgresCopyFieldWriter { NANOARROW_RETURN_NOT_OK(WriteChecked(buffer, field_size_bytes, error)); int64_t raw_value = ArrowArrayViewGetIntUnsafe(array_view_, index); - int64_t value; + int64_t value = 0; bool overflow_safe = true; switch (TU) { case NANOARROW_TIME_UNIT_SECOND: - if ((overflow_safe = raw_value <= kMaxSafeSecondsToMicros && - raw_value >= kMinSafeSecondsToMicros)) { + overflow_safe = + raw_value <= kMaxSafeSecondsToMicros && raw_value >= kMinSafeSecondsToMicros; + if (overflow_safe) { value = raw_value * 1000000; } break; case NANOARROW_TIME_UNIT_MILLI: - if ((overflow_safe = raw_value <= kMaxSafeMillisToMicros && - raw_value >= kMinSafeMillisToMicros)) { + overflow_safe = + raw_value <= kMaxSafeMillisToMicros && raw_value >= kMinSafeMillisToMicros; + if (overflow_safe) { value = raw_value * 1000; } break; @@ -443,19 +445,21 @@ class PostgresCopyTimestampFieldWriter : public PostgresCopyFieldWriter { NANOARROW_RETURN_NOT_OK(WriteChecked(buffer, field_size_bytes, error)); int64_t raw_value = ArrowArrayViewGetIntUnsafe(array_view_, index); - int64_t value; + int64_t value = 0; bool overflow_safe = true; switch (TU) { case NANOARROW_TIME_UNIT_SECOND: - if ((overflow_safe = raw_value <= kMaxSafeSecondsToMicros && - raw_value >= kMinSafeSecondsToMicros)) { + overflow_safe = + raw_value <= kMaxSafeSecondsToMicros && raw_value >= kMinSafeSecondsToMicros; + if (overflow_safe) { value = raw_value * 1000000; } break; case NANOARROW_TIME_UNIT_MILLI: - if ((overflow_safe = raw_value <= kMaxSafeMillisToMicros && - raw_value >= kMinSafeMillisToMicros)) { + overflow_safe = + raw_value <= kMaxSafeMillisToMicros && raw_value >= kMinSafeMillisToMicros; + if (overflow_safe) { value = raw_value * 1000; } break; diff --git a/c/driver/postgresql/statement.cc b/c/driver/postgresql/statement.cc index 8e5490046f..82e48929cf 100644 --- a/c/driver/postgresql/statement.cc +++ b/c/driver/postgresql/statement.cc @@ -447,15 +447,17 @@ struct BindStream { switch (unit) { case NANOARROW_TIME_UNIT_SECOND: - if ((overflow_safe = val <= kMaxSafeSecondsToMicros && - val >= kMinSafeSecondsToMicros)) { + overflow_safe = + val <= kMaxSafeSecondsToMicros && val >= kMinSafeSecondsToMicros; + if (overflow_safe) { val *= 1000000; } break; case NANOARROW_TIME_UNIT_MILLI: - if ((overflow_safe = val <= kMaxSafeMillisToMicros && - val >= kMinSafeMillisToMicros)) { + overflow_safe = + val <= kMaxSafeMillisToMicros && val >= kMinSafeMillisToMicros; + if (overflow_safe) { val *= 1000; } break; diff --git a/c/driver/sqlite/sqlite.c b/c/driver/sqlite/sqlite.c index b388ccf12b..87390ce351 100644 --- a/c/driver/sqlite/sqlite.c +++ b/c/driver/sqlite/sqlite.c @@ -397,8 +397,7 @@ AdbcStatusCode SqliteConnectionGetInfoImpl(const uint32_t* info_codes, struct ArrowSchema* schema, struct ArrowArray* array, struct AdbcError* error) { - RAISE_ADBC(AdbcInitConnectionGetInfoSchema(info_codes, info_codes_length, schema, array, - error)); + RAISE_ADBC(AdbcInitConnectionGetInfoSchema(schema, array, error)); for (size_t i = 0; i < info_codes_length; i++) { switch (info_codes[i]) { case ADBC_INFO_VENDOR_NAME: diff --git a/ci/scripts/python_wheel_windows_build.bat b/ci/scripts/python_wheel_windows_build.bat index e121a01ccb..b57338ba1c 100644 --- a/ci/scripts/python_wheel_windows_build.bat +++ b/ci/scripts/python_wheel_windows_build.bat @@ -23,7 +23,8 @@ set build_dir=%2 echo "=== (%PYTHON_VERSION%) Building ADBC libpq driver ===" set CMAKE_BUILD_TYPE=release -set CMAKE_GENERATOR=Visual Studio 15 2017 Win64 +set CMAKE_GENERATOR=Visual Studio 17 2022 +set CMAKE_GENERATOR_PLATFORM=x64 set CMAKE_UNITY_BUILD=ON set VCPKG_FEATURE_FLAGS=-manifests set VCPKG_TARGET_TRIPLET=x64-windows-static @@ -38,6 +39,7 @@ pushd %build_dir% cmake ^ -G "%CMAKE_GENERATOR%" ^ + -A "%CMAKE_GENERATOR_PLATFORM%" ^ -DADBC_BUILD_SHARED=ON ^ -DADBC_BUILD_STATIC=OFF ^ -DCMAKE_BUILD_TYPE=%CMAKE_BUILD_TYPE% ^ diff --git a/python/adbc_driver_manager/adbc_driver_manager/_blocking_impl.cc b/python/adbc_driver_manager/adbc_driver_manager/_blocking_impl.cc index 766b3964cd..9cabe50843 100644 --- a/python/adbc_driver_manager/adbc_driver_manager/_blocking_impl.cc +++ b/python/adbc_driver_manager/adbc_driver_manager/_blocking_impl.cc @@ -20,11 +20,13 @@ #if defined(_WIN32) #define NOMINMAX #define WIN32_LEAN_AND_MEAN +#define READ _read #include #include #include #include #else +#define READ read #include #include #include @@ -147,7 +149,7 @@ void InterruptThread() { while (true) { char buf = 0; // Anytime something is written to the pipe, attempt to call the callback - auto bytes_read = read(pipe[0], &buf, 1); + auto bytes_read = READ(pipe[0], &buf, 1); if (bytes_read < 0) { if (errno == EINTR) continue; diff --git a/python/adbc_driver_manager/adbc_driver_manager/_lib.pxd b/python/adbc_driver_manager/adbc_driver_manager/_lib.pxd index e9ea833c36..5c572f7825 100644 --- a/python/adbc_driver_manager/adbc_driver_manager/_lib.pxd +++ b/python/adbc_driver_manager/adbc_driver_manager/_lib.pxd @@ -287,7 +287,7 @@ cdef const CAdbcError* PyAdbcErrorFromArrayStream( CArrowArrayStream* stream, CAdbcStatusCode* status) cdef void check_error(CAdbcStatusCode status, CAdbcError* error) except * -cdef object convert_error(CAdbcStatusCode status, CAdbcError* error) except * +cdef object convert_error(CAdbcStatusCode status, CAdbcError* error) cdef extern from "adbc_driver_manager.h": const char* CAdbcStatusCodeMessage"AdbcStatusCodeMessage"(CAdbcStatusCode code) diff --git a/python/adbc_driver_manager/adbc_driver_manager/_lib.pyx b/python/adbc_driver_manager/adbc_driver_manager/_lib.pyx index 0592c8525e..b96ea6daf2 100644 --- a/python/adbc_driver_manager/adbc_driver_manager/_lib.pyx +++ b/python/adbc_driver_manager/adbc_driver_manager/_lib.pyx @@ -168,7 +168,7 @@ INGEST_OPTION_MODE_CREATE_APPEND = ADBC_INGEST_OPTION_MODE_CREATE_APPEND.decode( INGEST_OPTION_TARGET_TABLE = ADBC_INGEST_OPTION_TARGET_TABLE.decode("utf-8") -cdef object convert_error(CAdbcStatusCode status, CAdbcError* error) except *: +cdef object convert_error(CAdbcStatusCode status, CAdbcError* error): cdef CAdbcErrorDetail c_detail if status == ADBC_STATUS_OK: diff --git a/python/adbc_driver_manager/setup.py b/python/adbc_driver_manager/setup.py index 1b6f1026ea..3ce655ca49 100644 --- a/python/adbc_driver_manager/setup.py +++ b/python/adbc_driver_manager/setup.py @@ -75,7 +75,7 @@ def get_version(pkg_path): build_type = os.environ.get("ADBC_BUILD_TYPE", "release") if sys.platform == "win32": - extra_compile_args = ["/std:c++17", "/DADBC_EXPORTING"] + extra_compile_args = ["/std:c++17", "/DADBC_EXPORTING", "/D_CRT_SECURE_NO_WARNINGS"] if build_type == "debug": extra_compile_args.extend(["/DEBUG:FULL"]) else: