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 <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
apache · Nov 15, 2022 · c67738f · c67738f
1 parent 9d66933
commit c67738f
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 27 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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]]