You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ko...@apache.org on 2022/11/15 08:51:06 UTC

[arrow] 10/27: ARROW-18186: [C++][MinGW] Make buildable with clang (#14536)

This is an automated email from the ASF dual-hosted git repository.

kou pushed a commit to branch maint-10.0.x
in repository https://gitbox.apache.org/repos/asf/arrow.git

commit c67738f555f5db6435016a7901ad2c566c46c7d2
Author: Sutou Kouhei <ko...@clear-code.com>
AuthorDate: Wed Nov 2 05:33:10 2022 +0900

    ARROW-18186: [C++][MinGW] Make buildable with clang (#14536)
    
    Error1 (can't use `[[gnu::dllexport]]` with template):
    
        cpp/src/arrow/util/int_util.cc:463:1: error: an attribute list cannot appear here
        INSTANTIATE_ALL()
        ^~~~~~~~~~~~~~~~~
        cpp/src/arrow/util/int_util.cc:454:3: note: expanded from macro 'INSTANTIATE_ALL'
          INSTANTIATE_ALL_DEST(uint8_t)  \
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        cpp/src/arrow/util/int_util.cc:444:3: note: expanded from macro 'INSTANTIATE_ALL_DEST'
          INSTANTIATE(uint8_t, DEST)       \
          ^~~~~~~~~~~~~~~~~~~~~~~~~~
        cpp/src/arrow/util/int_util.cc:440:12: note: expanded from macro 'INSTANTIATE'
          template ARROW_TEMPLATE_EXPORT void TransposeInts( \
                   ^~~~~~~~~~~~~~~~~~~~~
        cpp/src/arrow/util/visibility.h:47:31: note: expanded from macro 'ARROW_TEMPLATE_EXPORT'
        #define ARROW_TEMPLATE_EXPORT ARROW_DLLEXPORT
                                      ^~~~~~~~~~~~~~~
        cpp/src/arrow/util/visibility.h:32:25: note: expanded from macro 'ARROW_DLLEXPORT'
        #define ARROW_DLLEXPORT [[gnu::dllexport]]
                                ^~~~~~~~~~~~~~~~~~
    
    Error2 (unused variable):
    
        cpp/src/arrow/util/io_util.cc:1079:7: warning: variable 'oflag' set but not used [-Wunused-but-set-variable]
          int oflag = _O_CREAT | _O_BINARY | _O_NOINHERIT;
              ^
    
    Error3 (missing field initializers):
    
        cpp/src/arrow/util/io_util.cc:1545:29: warning: missing field 'InternalHigh' initializer [-Wmissing-field-initializers]
          OVERLAPPED overlapped = {0};
                                    ^
    
    Authored-by: Sutou Kouhei <ko...@clear-code.com>
    Signed-off-by: Sutou Kouhei <ko...@clear-code.com>
---
 .github/workflows/cpp.yml                   | 27 +++++++++++++++------------
 cpp/cmake_modules/ThirdpartyToolchain.cmake |  6 +++---
 cpp/src/arrow/util/io_util.cc               | 13 ++-----------
 cpp/src/arrow/util/visibility.h             |  2 +-
 4 files changed, 21 insertions(+), 27 deletions(-)

diff --git a/.github/workflows/cpp.yml b/.github/workflows/cpp.yml
index 3d6b89f2f5..2278f075a8 100644
--- a/.github/workflows/cpp.yml
+++ b/.github/workflows/cpp.yml
@@ -293,18 +293,21 @@ jobs:
           ci/scripts/cpp_test.sh $(pwd) $(pwd)/build
 
   windows-mingw:
-    name: AMD64 Windows MinGW ${{ matrix.mingw-n-bits }} C++
+    name: AMD64 Windows MinGW ${{ matrix.msystem_upper }} C++
     runs-on: windows-2019
     if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
-    # Build may take 1h+ without cache and installing Google Cloud
-    # Storage Testbench may take 20m+ without cache.
+    # Build may take 1h+ without cache.
     timeout-minutes: 120
     strategy:
       fail-fast: false
       matrix:
-        mingw-n-bits:
-          - 32
-          - 64
+        include:
+          - msystem_lower: mingw32
+            msystem_upper: MINGW32
+          - msystem_lower: mingw64
+            msystem_upper: MINGW64
+          - msystem_lower: clang64
+            msystem_upper: CLANG64
     env:
       ARROW_BUILD_SHARED: ON
       ARROW_BUILD_STATIC: OFF
@@ -316,10 +319,9 @@ jobs:
       ARROW_GANDIVA: ON
       ARROW_GCS: ON
       ARROW_HDFS: OFF
-      ARROW_HOME: /mingw${{ matrix.mingw-n-bits }}
+      ARROW_HOME: /${{ matrix.msystem_lower}}
       ARROW_JEMALLOC: OFF
       ARROW_PARQUET: ON
-      ARROW_PYTHON: ON
       ARROW_S3: ON
       ARROW_USE_GLOG: OFF
       ARROW_VERBOSE_THIRDPARTY_BUILD: OFF
@@ -334,11 +336,12 @@ jobs:
       # -DBoost_NO_BOOST_CMAKE=ON
       BOOST_ROOT: ""
       CMAKE_ARGS: >-
-        -DARROW_PACKAGE_PREFIX=/mingw${{ matrix.mingw-n-bits }}
+        -DARROW_PACKAGE_PREFIX=/${{ matrix.msystem_lower}}
         -DBoost_NO_BOOST_CMAKE=ON
       # We can't use unity build because we don't have enough memory on
       # GitHub Actions.
       # CMAKE_UNITY_BUILD: ON
+      GTest_SOURCE: BUNDLED
     steps:
       - name: Disable Crash Dialogs
         run: |
@@ -355,7 +358,7 @@ jobs:
           submodules: recursive
       - uses: msys2/setup-msys2@v2
         with:
-          msystem: MINGW${{ matrix.mingw-n-bits }}
+          msystem: ${{ matrix.msystem_upper }}
           update: true
       - name: Setup MSYS2
         shell: msys2 {0}
@@ -364,8 +367,8 @@ jobs:
         uses: actions/cache@v3
         with:
           path: ccache
-          key: cpp-ccache-mingw${{ matrix.mingw-n-bits }}-${{ hashFiles('cpp/**') }}
-          restore-keys: cpp-ccache-mingw${{ matrix.mingw-n-bits }}-
+          key: cpp-ccache-${{ matrix.msystem_lower}}-${{ hashFiles('cpp/**') }}
+          restore-keys: cpp-ccache-${{ matrix.msystem_lower}}-
       - name: Build
         shell: msys2 {0}
         run: |
diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake
index b7cd31f3d7..b1c3201894 100644
--- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
+++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -1947,7 +1947,7 @@ macro(build_gtest)
                               -Wno-unused-value -Wno-ignored-attributes)
   endif()
 
-  if(MSVC)
+  if(WIN32)
     set(GTEST_CMAKE_CXX_FLAGS "${GTEST_CMAKE_CXX_FLAGS} -DGTEST_CREATE_SHARED_LIBRARY=1")
   endif()
 
@@ -1956,7 +1956,7 @@ macro(build_gtest)
 
   set(_GTEST_LIBRARY_DIR "${GTEST_PREFIX}/lib")
 
-  if(MSVC)
+  if(WIN32)
     set(_GTEST_IMPORTED_TYPE IMPORTED_IMPLIB)
     set(_GTEST_LIBRARY_SUFFIX
         "${CMAKE_GTEST_DEBUG_EXTENSION}${CMAKE_IMPORT_LIBRARY_SUFFIX}")
@@ -1989,7 +1989,7 @@ macro(build_gtest)
       -DCMAKE_MACOSX_RPATH=OFF)
   set(GMOCK_INCLUDE_DIR "${GTEST_PREFIX}/include")
 
-  if(MSVC AND NOT ARROW_USE_STATIC_CRT)
+  if(WIN32 AND NOT ARROW_USE_STATIC_CRT)
     set(GTEST_CMAKE_ARGS ${GTEST_CMAKE_ARGS} -Dgtest_force_shared_crt=ON)
   endif()
 
diff --git a/cpp/src/arrow/util/io_util.cc b/cpp/src/arrow/util/io_util.cc
index a62040f3a7..571b49c1d7 100644
--- a/cpp/src/arrow/util/io_util.cc
+++ b/cpp/src/arrow/util/io_util.cc
@@ -1076,24 +1076,15 @@ Result<FileDescriptor> FileOpenWritable(const PlatformFilename& file_name,
   FileDescriptor fd;
 
 #if defined(_WIN32)
-  int oflag = _O_CREAT | _O_BINARY | _O_NOINHERIT;
   DWORD desired_access = GENERIC_WRITE;
   DWORD share_mode = FILE_SHARE_READ | FILE_SHARE_WRITE;
   DWORD creation_disposition = OPEN_ALWAYS;
 
-  if (append) {
-    oflag |= _O_APPEND;
-  }
-
   if (truncate) {
-    oflag |= _O_TRUNC;
     creation_disposition = CREATE_ALWAYS;
   }
 
-  if (write_only) {
-    oflag |= _O_WRONLY;
-  } else {
-    oflag |= _O_RDWR;
+  if (!write_only) {
     desired_access |= GENERIC_READ;
   }
 
@@ -1542,7 +1533,7 @@ static inline int64_t pread_compat(int fd, void* buf, int64_t nbytes, int64_t po
 #if defined(_WIN32)
   HANDLE handle = reinterpret_cast<HANDLE>(_get_osfhandle(fd));
   DWORD dwBytesRead = 0;
-  OVERLAPPED overlapped = {0};
+  OVERLAPPED overlapped = {};
   overlapped.Offset = static_cast<uint32_t>(pos);
   overlapped.OffsetHigh = static_cast<uint32_t>(pos >> 32);
 
diff --git a/cpp/src/arrow/util/visibility.h b/cpp/src/arrow/util/visibility.h
index 6ab5290ef3..b0fd790295 100644
--- a/cpp/src/arrow/util/visibility.h
+++ b/cpp/src/arrow/util/visibility.h
@@ -26,7 +26,7 @@
 #pragma GCC diagnostic ignored "-Wattributes"
 #endif
 
-#if defined(__cplusplus) && (defined(__GNUC__) || defined(__clang__))
+#if defined(__cplusplus) && defined(__GNUC__) && !defined(__clang__)
 // Use C++ attribute syntax where possible to avoid GCC parser bug
 // (https://stackoverflow.com/questions/57993818/gcc-how-to-combine-attribute-dllexport-and-nodiscard-in-a-struct-de)
 #define ARROW_DLLEXPORT [[gnu::dllexport]]