ARROW-17545: [C++][CI] Mandate C++17 instead of C++11 (apache#13991)

This PR switches our build system to require C++17 instead of C++11.

Because the conda packaging jobs are out of sync with the conda-forge files, the Windows conda packaging jobs are broken with this change. The related task (sync conda packaging files with conda-forge) is tracked in ARROW-17635.


Authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>

Author: pitrou

.env

@@ -86,8 +86,8 @@ ARROW_R_DEV=TRUE
 R_PRUNE_DEPS=FALSE
 TZ=UTC
 
-# -1 does not attempt to install a devtoolset version, any positive integer will install devtoolset-n
-DEVTOOLSET_VERSION=-1
+# Any non-empty string will install devtoolset-${DEVTOOLSET_VERSION}
+DEVTOOLSET_VERSION=
 
 # Used through docker-compose.yml and serves as the default version for the
 # ci/scripts/install_vcpkg.sh script. Prefer to use short SHAs to keep the

.github/workflows/r.yml

@@ -119,12 +119,13 @@ jobs:
       fail-fast: false
       matrix:
         config:
-          - { org: "rstudio", image: "r-base", tag: "4.0-centos7" }
-          - { org: "rhub", image: "debian-gcc-devel", tag: "latest" }
+          - { org: "rstudio", image: "r-base", tag: "4.0-centos7", devtoolset: "8" }
+          - { org: "rhub", image: "debian-gcc-devel", tag: "latest", devtoolset: "" }
     env:
       R_ORG: ${{ matrix.config.org }}
       R_IMAGE: ${{ matrix.config.image }}
       R_TAG: ${{ matrix.config.tag }}
+      DEVTOOLSET_VERSION: ${{ matrix.config.devtoolset }}
     steps:
       - name: Checkout Arrow
         uses: actions/checkout@v3
@@ -184,11 +185,6 @@ jobs:
         run: |
           ci/scripts/ccache_setup.sh
           echo "CCACHE_DIR=$(cygpath --absolute --windows ccache)" >> $GITHUB_ENV
-      # We must enable actions/cache before r-lib/actions/setup-r to ensure
-      # using system tar instead of tar provided by Rtools.
-      # We can use tar provided by Rtools when we drop support for Rtools 3.5.
-      # Because Rtools 4.0 or later has zstd. actions/cache requires zstd
-      # when tar is GNU tar.
       - name: Cache ccache
         uses: actions/cache@v2
         with:
@@ -268,8 +264,6 @@ jobs:
         with:
           r-version: ${{ matrix.config.rversion }}
           rtools-version: ${{ matrix.config.rtools }}
-          # RSPM keeps install times short for 3.6
-          use-public-rspm: true
           Ncpus: 2
       - uses: r-lib/actions/setup-r-dependencies@v2
         env:

c_glib/meson.build

@@ -21,7 +21,7 @@ project('arrow-glib', 'c', 'cpp',
         license: 'Apache-2.0',
         default_options: [
           'c_std=c99',
-          'cpp_std=c++11',
+          'cpp_std=c++17',
         ])
 
 version = '10.0.0-SNAPSHOT'

ci/conan/all/test_package/CMakeLists.txt

@@ -30,5 +30,5 @@ find_package(Arrow REQUIRED)
 
 add_executable(${PROJECT_NAME} test_package.cpp)
 target_link_libraries(${PROJECT_NAME} arrow::arrow)
-target_compile_features(${PROJECT_NAME} PRIVATE cxx_std_11)
+target_compile_features(${PROJECT_NAME} PRIVATE cxx_std_17)
 target_compile_definitions(${PROJECT_NAME} PRIVATE WITH_JEMALLOC)

ci/docker/linux-r.dockerfile

@@ -27,7 +27,7 @@ ENV R_BIN=${r_bin}
 ARG r_dev=FALSE
 ENV ARROW_R_DEV=${r_dev}
 
-ARG devtoolset_version=-1
+ARG devtoolset_version=
 ENV DEVTOOLSET_VERSION=${devtoolset_version}
 
 ARG r_prune_deps=FALSE

ci/scripts/cpp_build.sh

@@ -142,7 +142,7 @@ cmake \
   -DCMAKE_VERBOSE_MAKEFILE=${CMAKE_VERBOSE_MAKEFILE:-OFF} \
   -DCMAKE_C_FLAGS="${CFLAGS:-}" \
   -DCMAKE_CXX_FLAGS="${CXXFLAGS:-}" \
-  -DCMAKE_CXX_STANDARD="${CMAKE_CXX_STANDARD:-11}" \
+  -DCMAKE_CXX_STANDARD="${CMAKE_CXX_STANDARD:-17}" \
   -DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR:-lib} \
   -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX:-${ARROW_HOME}} \
   -DCMAKE_UNITY_BUILD=${CMAKE_UNITY_BUILD:-OFF} \

ci/scripts/r_docker_configure.sh

@@ -57,7 +57,7 @@ if [ ${R_CUSTOM_CCACHE} = "true" ]; then
 CCACHE=ccache
 CC=\$(CCACHE) gcc\$(VER)
 CXX=\$(CCACHE) g++\$(VER)
-CXX11=\$(CCACHE) g++\$(VER)" >> ~/.R/Makevars
+CXX17=\$(CCACHE) g++\$(VER)" >> ~/.R/Makevars
 
   mkdir -p ~/.ccache/
   echo "max_size = 5.0G
@@ -69,7 +69,7 @@ fi
 
 # Special hacking to try to reproduce quirks on centos using non-default build
 # tooling.
-if [[ "$DEVTOOLSET_VERSION" -gt 0 ]]; then
+if [[ -n "$DEVTOOLSET_VERSION" ]]; then
   $PACKAGE_MANAGER install -y centos-release-scl
   $PACKAGE_MANAGER install -y "devtoolset-$DEVTOOLSET_VERSION"
 fi

ci/scripts/r_test.sh

@@ -26,8 +26,21 @@ pushd ${source_dir}
 
 printenv
 
+if [[ -n "$DEVTOOLSET_VERSION" ]]; then
+  # enable the devtoolset version to use it
+  source /opt/rh/devtoolset-$DEVTOOLSET_VERSION/enable
+
+  # Build images which require the devtoolset don't have CXX17 variables
+  # set as the system compiler doesn't support C++17
+  mkdir -p ~/.R
+  echo "CC = $(which gcc) -fPIC" >> ~/.R/Makevars
+  echo "CXX17 = $(which g++) -fPIC" >> ~/.R/Makevars
+  echo "CXX17STD = -std=c++17" >> ~/.R/Makevars
+  echo "CXX17FLAGS = ${CXX11FLAGS}" >> ~/.R/Makevars
+fi
+
 # Run the nixlibs.R test suite, which is not included in the installed package
- ${R_BIN} -e 'setwd("tools"); testthat::test_dir(".")'
+${R_BIN} -e 'setwd("tools"); testthat::test_dir(".")'
 
 # Before release, we always copy the relevant parts of the cpp source into the
 # package. In some CI checks, we will use this version of the source:
@@ -77,11 +90,6 @@ export ARROW_DEBUG_MEMORY_POOL=trap
 export TEXMFCONFIG=/tmp/texmf-config
 export TEXMFVAR=/tmp/texmf-var
 
-if [[ "$DEVTOOLSET_VERSION" -gt 0 ]]; then
-  # enable the devtoolset version to use it
-  source /opt/rh/devtoolset-$DEVTOOLSET_VERSION/enable
-fi
-
 # Make sure we aren't writing to the home dir (CRAN _hates_ this but there is no official check)
 BEFORE=$(ls -alh ~/)
 

cpp/CMakeLists.txt

@@ -561,10 +561,10 @@ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${ARROW_CXXFLAGS}")
 # C++ specific flags.
 set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${CXX_COMMON_FLAGS} ${ARROW_CXXFLAGS}")
 
-# Remove --std=c++11 to avoid errors from C compilers
-string(REPLACE "-std=c++11" "" CMAKE_C_FLAGS ${CMAKE_C_FLAGS})
+# Remove --std=c++17 to avoid errors from C compilers
+string(REPLACE "-std=c++17" "" CMAKE_C_FLAGS ${CMAKE_C_FLAGS})
 
-# Add C++-only flags, like -std=c++11
+# Add C++-only flags, like -std=c++17
 set(CMAKE_CXX_FLAGS "${CXX_ONLY_FLAGS} ${CMAKE_CXX_FLAGS}")
 
 # ASAN / TSAN / UBSAN

cpp/build-support/update-flatbuffers.sh

@@ -33,9 +33,6 @@ OUT_DIR="$SOURCE_DIR/generated"
 FILES=($(find $FORMAT_DIR -name '*.fbs'))
 FILES+=("$SOURCE_DIR/arrow/ipc/feather.fbs")
 
-# add compute ir files
-FILES+=($(find "$TOP/experimental/computeir" -name '*.fbs'))
-
 $FLATC --cpp --cpp-std c++11 \
   --scoped-enums \
   -o "$OUT_DIR" \

cpp/cmake_modules/SetupCxxFlags.cmake

@@ -122,12 +122,14 @@ if(NOT DEFINED CMAKE_C_STANDARD)
   set(CMAKE_C_STANDARD 11)
 endif()
 
-# This ensures that things like c++11 get passed correctly
+# This ensures that things like c++17 get passed correctly
 if(NOT DEFINED CMAKE_CXX_STANDARD)
-  set(CMAKE_CXX_STANDARD 11)
+  set(CMAKE_CXX_STANDARD 17)
+elseif(${CMAKE_CXX_STANDARD} VERSION_LESS 17)
+  message(FATAL_ERROR "Cannot set a CMAKE_CXX_STANDARD smaller than 17")
 endif()
 
-# We require a C++11 compliant compiler
+# We require a C++17 compliant compiler
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 
 # ARROW-6848: Do not use GNU (or other CXX) extensions
@@ -440,11 +442,11 @@ elseif(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_CXX_COMPILER_ID STRE
   # Don't complain about optimization passes that were not possible
   set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-pass-failed")
 
-  if(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang")
-    # Depending on the default OSX_DEPLOYMENT_TARGET (< 10.9), libstdc++ may be
-    # the default standard library which does not support C++11. libc++ is the
-    # default from 10.9 onward.
-    set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -stdlib=libc++")
+  # Avoid clang / libc++ error about C++17 aligned allocation on macOS.
+  # See https://chromium.googlesource.com/chromium/src/+/eee44569858fc650b635779c4e34be5cb0c73186%5E%21/#F0
+  # for details.
+  if(APPLE)
+    set(CXX_ONLY_FLAGS "${CXX_ONLY_FLAGS} -fno-aligned-new")
   endif()
 endif()
 

cpp/cmake_modules/ThirdpartyToolchain.cmake

@@ -2088,7 +2088,7 @@ macro(build_benchmark)
   endif()
 
   if(NOT MSVC)
-    set(GBENCHMARK_CMAKE_CXX_FLAGS "${EP_CXX_FLAGS} -std=c++11")
+    set(GBENCHMARK_CMAKE_CXX_FLAGS "${EP_CXX_FLAGS} -std=c++17")
   endif()
 
   if(APPLE AND (CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_CXX_COMPILER_ID

cpp/examples/minimal_build/CMakeLists.txt

@@ -24,10 +24,10 @@ option(ARROW_LINK_SHARED "Link to the Arrow shared library" ON)
 find_package(Arrow REQUIRED)
 
 if(NOT DEFINED CMAKE_CXX_STANDARD)
-  set(CMAKE_CXX_STANDARD 11)
+  set(CMAKE_CXX_STANDARD 17)
 endif()
 
-# We require a C++11 compliant compiler
+# We require a C++17 compliant compiler
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 
 if(NOT DEFINED CMAKE_BUILD_TYPE)

cpp/examples/parquet/parquet_arrow/CMakeLists.txt

@@ -26,12 +26,12 @@ include(GNUInstallDirs)
 
 option(PARQUET_LINK_SHARED "Link to the Parquet shared library" ON)
 
-# This ensures that things like gnu++11 get passed correctly
+# This ensures that things like -std=gnu++... get passed correctly
 if(NOT DEFINED CMAKE_CXX_STANDARD)
-  set(CMAKE_CXX_STANDARD 11)
+  set(CMAKE_CXX_STANDARD 17)
 endif()
 
-# We require a C++11 compliant compiler
+# We require a C++17 compliant compiler
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 
 # Look for installed packages the system

cpp/src/arrow/CMakeLists.txt

@@ -636,6 +636,8 @@ endif()
 
 foreach(LIB_TARGET ${ARROW_LIBRARIES})
   target_compile_definitions(${LIB_TARGET} PRIVATE ARROW_EXPORTING)
+  # C++17 is required to compile against Arrow C++ headers and libraries
+  target_compile_features(${LIB_TARGET} PUBLIC cxx_std_17)
 endforeach()
 
 if(ARROW_WITH_BACKTRACE)

cpp/src/arrow/dataset/file_csv.h

@@ -98,7 +98,8 @@ class ARROW_DS_EXPORT CsvFileWriteOptions : public FileWriteOptions {
   std::shared_ptr<csv::WriteOptions> write_options;
 
  protected:
-  using FileWriteOptions::FileWriteOptions;
+  explicit CsvFileWriteOptions(std::shared_ptr<FileFormat> format)
+      : FileWriteOptions(std::move(format)) {}
 
   friend class CsvFileFormat;
 };

cpp/src/arrow/dataset/file_ipc.h

@@ -90,7 +90,8 @@ class ARROW_DS_EXPORT IpcFileWriteOptions : public FileWriteOptions {
   std::shared_ptr<const KeyValueMetadata> metadata;
 
  protected:
-  using FileWriteOptions::FileWriteOptions;
+  explicit IpcFileWriteOptions(std::shared_ptr<FileFormat> format)
+      : FileWriteOptions(std::move(format)) {}
 
   friend class IpcFileFormat;
 };

cpp/src/arrow/dataset/file_parquet.h

@@ -226,7 +226,8 @@ class ARROW_DS_EXPORT ParquetFileWriteOptions : public FileWriteOptions {
   std::shared_ptr<parquet::ArrowWriterProperties> arrow_writer_properties;
 
  protected:
-  using FileWriteOptions::FileWriteOptions;
+  explicit ParquetFileWriteOptions(std::shared_ptr<FileFormat> format)
+      : FileWriteOptions(std::move(format)) {}
 
   friend class ParquetFileFormat;
 };

cpp/src/arrow/memory_pool.cc

@@ -229,6 +229,8 @@ class DebugAllocator {
       *out = memory_pool::internal::kZeroSizeArea;
     } else {
       ARROW_ASSIGN_OR_RAISE(int64_t raw_size, RawSize(size));
+      DCHECK(raw_size > size) << "bug in raw size computation: " << raw_size
+                              << " for size " << size;
       RETURN_NOT_OK(WrappedAllocator::AllocateAligned(raw_size, out));
       InitAllocatedArea(*out, size);
     }
@@ -250,6 +252,8 @@ class DebugAllocator {
       return Status::OK();
     }
     ARROW_ASSIGN_OR_RAISE(int64_t raw_new_size, RawSize(new_size));
+    DCHECK(raw_new_size > new_size)
+        << "bug in raw size computation: " << raw_new_size << " for size " << new_size;
     RETURN_NOT_OK(
         WrappedAllocator::ReallocateAligned(old_size + kOverhead, raw_new_size, ptr));
     InitAllocatedArea(*ptr, new_size);

cpp/src/arrow/util/aligned_storage.h

@@ -123,7 +123,26 @@ class AlignedStorage {
   }
 
  private:
+#if !defined(__clang__) && defined(__GNUC__) && defined(__i386__)
+  // Workaround for GCC bug on i386:
+  //   alignof(int64 | float64) can give different results depending on the
+  //   compilation context, leading to internal ABI mismatch manifesting
+  //   in incorrect propagation of Result<int64 | float64> between
+  //   compilation units.
+  // (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88115)
+  static constexpr size_t alignment() {
+    if (std::is_integral_v<T> && sizeof(T) == 8) {
+      return 4;
+    } else if (std::is_floating_point_v<T> && sizeof(T) == 8) {
+      return 4;
+    }
+    return alignof(T);
+  }
+
+  typename std::aligned_storage<sizeof(T), alignment()>::type data_;
+#else
   typename std::aligned_storage<sizeof(T), alignof(T)>::type data_;
+#endif
 };
 
 }  // namespace internal

cpp/src/arrow/util/int_util_test.cc

@@ -17,6 +17,7 @@
 
 #include <algorithm>
 #include <cstdint>
+#include <limits>
 #include <random>
 #include <string>
 #include <utility>
@@ -594,5 +595,59 @@ TEST(CheckIntegersInRange, UnsignedInts) {
   CheckInRangeFails(uint64(), "[0, 10000000000, 10000000000]", "[0, 9999999999]");
 }
 
+template <typename T>
+class TestAddWithOverflow : public ::testing::Test {
+ public:
+  void CheckOk(T a, T b, T expected_result = {}) {
+    ARROW_SCOPED_TRACE("a = ", a, ", b = ", b);
+    T result;
+    ASSERT_FALSE(AddWithOverflow(a, b, &result));
+    ASSERT_EQ(result, expected_result);
+  }
+
+  void CheckOverflow(T a, T b) {
+    ARROW_SCOPED_TRACE("a = ", a, ", b = ", b);
+    T result;
+    ASSERT_TRUE(AddWithOverflow(a, b, &result));
+  }
+};
+
+using SignedIntegerTypes = ::testing::Types<int8_t, int16_t, int32_t, int64_t>;
+
+TYPED_TEST_SUITE(TestAddWithOverflow, SignedIntegerTypes);
+
+TYPED_TEST(TestAddWithOverflow, Basics) {
+  using T = TypeParam;
+
+  const T almost_max = std::numeric_limits<T>::max() - T{2};
+  const T almost_min = std::numeric_limits<T>::min() + T{2};
+
+  this->CheckOk(T{1}, T{2}, T{3});
+  this->CheckOk(T{-1}, T{2}, T{1});
+  this->CheckOk(T{-1}, T{-2}, T{-3});
+
+  this->CheckOk(almost_min, T{0}, almost_min);
+  this->CheckOk(almost_min, T{-2}, almost_min - T{2});
+  this->CheckOk(almost_min, T{1}, almost_min + T{1});
+  this->CheckOverflow(almost_min, T{-3});
+  this->CheckOverflow(almost_min, almost_min);
+
+  this->CheckOk(almost_max, T{0}, almost_max);
+  this->CheckOk(almost_max, T{2}, almost_max + T{2});
+  this->CheckOk(almost_max, T{-1}, almost_max - T{1});
+  this->CheckOverflow(almost_max, T{3});
+  this->CheckOverflow(almost_max, almost_max);
+
+  // In 2's complement, almost_min == - almost_max - 1
+  this->CheckOk(almost_min, almost_max, T{-1});
+  this->CheckOk(almost_max, almost_min, T{-1});
+  this->CheckOk(almost_min - T{1}, almost_max, T{-2});
+  this->CheckOk(almost_min + T{1}, almost_max, T{0});
+  this->CheckOk(almost_min + T{2}, almost_max, T{1});
+  this->CheckOk(almost_min, almost_max - T{1}, T{-2});
+  this->CheckOk(almost_min, almost_max + T{1}, T{0});
+  this->CheckOk(almost_min, almost_max + T{2}, T{1});
+}
+
 }  // namespace internal
 }  // namespace arrow

cpp/src/arrow/util/launder.h

@@ -22,7 +22,7 @@
 namespace arrow {
 namespace internal {
 
-#if __cplusplus >= 201703L
+#if __cpp_lib_launder
 using std::launder;
 #else
 template <class T>