From 47ed583b497ffc1ce274260869468ae27e813eb8 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Thu, 21 Dec 2023 09:19:40 +0900 Subject: [PATCH 1/2] GH-39333: [C++] Don't use "if constexpr" in lambda It seems that it's not portable. At least it doesn't work as expected with Visual Studio 2017: C:/arrow/cpp/src/arrow/array/array_nested.cc(291): error C2065: 'validity': undeclared identifier (compiling source file C:\arrow-build\src\arrow\CMakeFiles\arrow_shared.dir\Unity\unity_0_cxx.cxx) [C:\arrow-build\src\arrow\arrow_shared.vcxproj] C:/arrow/cpp/src/arrow/array/array_nested.cc(660): note: see reference to function template instantiation 'arrow::Result> arrow::`anonymous-namespace'::FlattenListViewArray(const ListViewArrayT &,arrow::MemoryPool *)' being compiled with [ ListViewArrayT=arrow::ListViewArray ] (compiling source file C:\arrow-build\src\arrow\CMakeFiles\arrow_shared.dir\Unity\unity_0_cxx.cxx) memory_pool.cc C:/arrow/cpp/src/arrow/array/array_nested.cc(291): error C2065: 'list_view_array_offset': undeclared identifier (compiling source file C:\arrow-build\src\arrow\CMakeFiles\arrow_shared.dir\Unity\unity_0_cxx.cxx) [C:\arrow-build\src\arrow\arrow_shared.vcxproj] Use "if constexpr" in outer lambda. --- cpp/src/arrow/array/array_nested.cc | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/cpp/src/arrow/array/array_nested.cc b/cpp/src/arrow/array/array_nested.cc index 03f3e5af29908..b2cf316d93cd3 100644 --- a/cpp/src/arrow/array/array_nested.cc +++ b/cpp/src/arrow/array/array_nested.cc @@ -267,7 +267,6 @@ template Result> FlattenListViewArray(const ListViewArrayT& list_view_array, MemoryPool* memory_pool) { using offset_type = typename ListViewArrayT::offset_type; - const int64_t list_view_array_offset = list_view_array.offset(); const int64_t list_view_array_length = list_view_array.length(); std::shared_ptr value_array = list_view_array.values(); @@ -282,18 +281,24 @@ Result> FlattenListViewArray(const ListViewArrayT& list_v } } - const auto* validity = list_view_array.data()->template GetValues(0, 0); const auto* offsets = list_view_array.data()->template GetValues(1); const auto* sizes = list_view_array.data()->template GetValues(2); - auto is_null_or_empty = [&](int64_t i) { - if constexpr (HasNulls) { + std::function is_null_or_empty; + const auto* validity = list_view_array.data()->template GetValues(0, 0); + const int64_t list_view_array_offset = list_view_array.offset(); + if constexpr (HasNulls) { + is_null_or_empty = [&](int64_t i) { if (!bit_util::GetBit(validity, list_view_array_offset + i)) { return true; } - } - return sizes[i] == 0; - }; + return sizes[i] == 0; + }; + } else { + ARROW_UNUSED(validity); + ARROW_UNUSED(list_view_array_offset); + is_null_or_empty = [&](int64_t i) { return sizes[i] == 0; }; + } // Index of the first valid, non-empty list-view. int64_t first_i = 0; From e2ee3d31a663222d6c97feb20bff106908052321 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Thu, 21 Dec 2023 09:58:37 +0100 Subject: [PATCH 2/2] Apply simpler solution --- cpp/src/arrow/array/array_nested.cc | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/cpp/src/arrow/array/array_nested.cc b/cpp/src/arrow/array/array_nested.cc index b2cf316d93cd3..acdd0a0742468 100644 --- a/cpp/src/arrow/array/array_nested.cc +++ b/cpp/src/arrow/array/array_nested.cc @@ -267,6 +267,7 @@ template Result> FlattenListViewArray(const ListViewArrayT& list_view_array, MemoryPool* memory_pool) { using offset_type = typename ListViewArrayT::offset_type; + const int64_t list_view_array_offset = list_view_array.offset(); const int64_t list_view_array_length = list_view_array.length(); std::shared_ptr value_array = list_view_array.values(); @@ -281,24 +282,16 @@ Result> FlattenListViewArray(const ListViewArrayT& list_v } } + const auto* validity = list_view_array.data()->template GetValues(0, 0); const auto* offsets = list_view_array.data()->template GetValues(1); const auto* sizes = list_view_array.data()->template GetValues(2); - std::function is_null_or_empty; - const auto* validity = list_view_array.data()->template GetValues(0, 0); - const int64_t list_view_array_offset = list_view_array.offset(); - if constexpr (HasNulls) { - is_null_or_empty = [&](int64_t i) { - if (!bit_util::GetBit(validity, list_view_array_offset + i)) { - return true; - } - return sizes[i] == 0; - }; - } else { - ARROW_UNUSED(validity); - ARROW_UNUSED(list_view_array_offset); - is_null_or_empty = [&](int64_t i) { return sizes[i] == 0; }; - } + auto is_null_or_empty = [&](int64_t i) { + if (HasNulls && !bit_util::GetBit(validity, list_view_array_offset + i)) { + return true; + } + return sizes[i] == 0; + }; // Index of the first valid, non-empty list-view. int64_t first_i = 0;