GH-48588: [C++] use std::span #48615

chakkk309 · 2025-12-21T08:00:07Z

Rationale for this change

Replace custom arrow::util::span implementation with C++20 std::span as suggested in issue #48588. This modernizes the codebase and removes ~130 lines of custom code.

What changes are included in this PR?

Removed src/arrow/util/span.h (custom span implementation)
Replaced all arrow::util::span<T> usage with std::span<T> (95 occurrences)
Updated all #include "arrow/util/span.h" to #include  (41 files)
Modified span tests to use std namespace

Are these changes tested?

Yes - created comprehensive tests verifying std::span compatibility with all original arrow::util::span functionality including construction, subspan, element access, and byte conversion.

Are there any user-facing changes?

No user-facing changes.

GitHub Issue: [C++] C++20: use std::span #48588

github-actions · 2025-12-21T08:00:32Z

⚠️ GitHub issue #48588 has been automatically assigned in GitHub to PR creator.

WillAyd

Thanks for the PR!

WillAyd · 2025-12-23T13:17:58Z

cpp/src/arrow/array/data.h

 #include "arrow/util/bit_util.h"
 #include "arrow/util/macros.h"
-#include "arrow/util/span.h"
+#include <span>


Can you move the span include to be with the other stdlib headers, rather than down here with the arrow headers?

WillAyd · 2025-12-23T13:19:33Z

cpp/src/arrow/util/span_test.cc

-  static_assert(std::is_same_v<decltype(span(vec)), span<int>>);
-  static_assert(std::is_same_v<decltype(span(const_arr)), span<const int>>);
-  static_assert(std::is_same_v<decltype(span(const_vec)), span<const int>>);
+  static_assert(std::is_same_v<decltype(span(arr)), std::span<int>>);


Suggested change

static_assert(std::is_same_v<decltype(span(arr)), std::span<int>>);

static_assert(std::is_same_v<decltype(std::span(arr)), std::span<int>>);

Should these have the namespace too?

WillAyd · 2025-12-23T13:19:45Z

cpp/src/arrow/util/span_test.cc

 using testing::PrintToString;

-namespace arrow::util {
+namespace arrow_util_span_test {


Is this still required for other members?

No, i think this test file is no longer needed, so i will remove it.

pitrou · 2026-01-12T16:13:09Z

cpp/src/arrow/array/util.cc

 // specific language governing permissions and limitations
 // under the License.

-#include "arrow/array/util.h"


Our style conventions currently favor putting the .h inclusion corresponding to a .cc file at the top of said .cc file, so you could leave this as-is.

(no strong feelings from me, though)

pitrou · 2026-01-12T16:13:50Z

cpp/src/arrow/array/util.cc

 #include <cstring>
 #include <limits>
 #include <memory>
+#include <span>


If you didn't have to do any code-level changes in this file, then probably  does not need to be included at all?

Done. this file no longer includes  since it isn’t used.

pitrou · 2026-01-12T16:14:13Z

cpp/src/arrow/compute/kernels/aggregate_pivot.cc


 using arrow::internal::VisitSetBitRunsVoid;
-using arrow::util::span;
+using std::span;


Can we remove this and spell std::span in full everywhere?

Sure, will do the updates.

pitrou · 2026-01-12T16:14:29Z

cpp/src/arrow/compute/kernels/hash_aggregate.cc

 using ::arrow::internal::checked_cast;
 using ::arrow::internal::FirstTimeBitmapWriter;
-using ::arrow::util::span;
+using std::span;


pitrou · 2026-01-12T16:14:41Z

cpp/src/arrow/compute/kernels/hash_aggregate_pivot.cc

 namespace arrow::compute::internal {

-using ::arrow::util::span;
+using std::span;


pitrou · 2026-01-12T16:14:55Z

cpp/src/arrow/compute/kernels/pivot_internal.cc

 namespace arrow::compute::internal {

-using ::arrow::util::span;
+using std::span;


Same in all other similar places too :)

pitrou · 2026-01-12T16:15:43Z

cpp/src/arrow/util/float16_test.cc

+#include <span>
+#include <utility>


Can we move those up again? The GTest inclusion should be separate from standard library inclusions.

Thanks, updated. The GTest include is now separated from the stdlib headers in the affected test files.

pitrou · 2026-01-12T16:16:11Z

cpp/src/arrow/util/secure_string.cc

-#if defined(ARROW_USE_OPENSSL)
-#  include <openssl/crypto.h>
-#  include <openssl/opensslv.h>
-#endif
-
 #include "arrow/util/windows_compatibility.h"
-#if defined(_WIN32)
-#  include <windows.h>
-#endif
-


Why did you remove all these? Most probably they're required.

This was an accidental removal on my part, and I’ll restore the required items.

pitrou · 2026-01-12T16:16:34Z

cpp/src/arrow/util/secure_string.h

 #include <string>

-#include "arrow/util/span.h"
+#include <span>


If there's no other change in this header, then  is probably not required?

 is required here because SecureString::as_span() returns std::span, so the header needs the include.

pitrou · 2026-01-12T16:17:24Z

cpp/src/parquet/encryption/encryption_internal.cc

+#include <openssl/aes.h>
+#include <openssl/err.h>
+#include <openssl/evp.h>
+#include <openssl/rand.h>


Just like with GTest, can we keep OpenSSL inclusions separate from standard library inclusions?

pitrou · 2026-01-12T16:17:45Z

cpp/src/parquet/encryption/encryption_internal.h

 #include <vector>

-#include "arrow/util/span.h"
+#include <span>


Please move this up with stdlib inclusions.

pitrou · 2026-01-12T16:19:05Z

cpp/src/parquet/metadata.cc

 #include <utility>
 #include <vector>

+#include "parquet/metadata.h"


Let's keep this at the top of file as it was.

pitrou · 2026-01-12T16:19:19Z

cpp/src/parquet/size_statistics.h

 #include <vector>

-#include "arrow/util/span.h"
+#include <span>


pitrou · 2026-01-12T16:23:46Z

cpp/src/parquet/thrift_internal.h

 #include <utility>
 #include <vector>

+#include "parquet/windows_compatibility.h"


Let's keep this in its former place.

pitrou · 2026-01-12T16:24:51Z

Thanks for doing this @chakkk309! Can you make sure you address all the review comments I posted? Also, please watch for CI results and try to address any failures if they look related :-)

refactor: replace arrow::util::span with std::span

01ec550

chakkk309 requested a review from wgtmac as a code owner December 21, 2025 08:00

github-actions bot added Component: Parquet Component: C++ awaiting review Awaiting review labels Dec 21, 2025

WillAyd requested changes Dec 23, 2025

View reviewed changes

chakkk309 added 3 commits December 23, 2025 23:00

refactor: replace arrow::util::span with std::span

ce3a066

refactor: replace arrow::util::span with std::span

1bd778b

refactor: remove useless test span_test.cc

459cd5e

chakkk309 requested a review from WillAyd December 24, 2025 13:18

pitrou reviewed Jan 12, 2026

View reviewed changes

github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jan 12, 2026

chakkk309 added 2 commits January 14, 2026 17:45

refactor: fix include ordering and std::span usag

0f8def7

Merge origin/main

6ac6108

fix：secure_string OpenSSL and Windows includes

16163b6

	static_assert(std::is_same_v<decltype(span(arr)), std::span<int>>);
	static_assert(std::is_same_v<decltype(std::span(arr)), std::span<int>>);

GH-48588: [C++] use std::span #48615

Are you sure you want to change the base?

GH-48588: [C++] use std::span #48615

Conversation

chakkk309 commented Dec 21, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

github-actions bot commented Dec 21, 2025

Uh oh!

WillAyd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chakkk309 Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pitrou commented Jan 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chakkk309 commented Dec 21, 2025 •

edited by github-actions bot

Loading

chakkk309 Jan 14, 2026 •

edited

Loading