GH-40252: [C++] Make span SFINAE standards-conforming to enable compilation with nvcc

[vyasr]

Rationale for this change

The current code uses an incomplete type in a SFINAE construct.

Closes #40252

What changes are included in this PR?

This PR changes the way that the type is specified to avoid any UB.

Are these changes tested?

I tested locally that code that originally failed to compile with nvcc now compiles successfully. The same code also compiles under clang and gcc. This is a minimal reproducer:

#include <arrow/api.h>
#include <vector>

int main() {
  arrow::util::span<int> x{std::vector<int>{1, 2, 3}};
}

I did not include it here because it is a compile-time rather than runtime issue, and because at present it only manifests on nvcc.

Are there any user-facing changes?

No.

• GitHub Issue: [C++] Span implementation includes non-conformant SFINAE #40252

[github-actions]

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

[vyasr]

I should say that I've tested this with both CUDA 11.8 and CUDA 12.0.

It looks like there are some additional cases that the proposed fix doesn't support according to the current tests, though. I'll have to dig into those cases further.

[felipecrv]

CI builds are complaining.

[vyasr]

CI builds are complaining.

Yes, apologies, that's what I was referring to in my previous comment. It looks like a different construct is needed here, and I need to play with the various cases a bit more.

[vyasr]

@felipecrv @pitrou thanks for your patience. I have something that is working now (still waiting to see if it has any issues on the Windows builds in CI, but I've tested various compilers on Linux and they're happy so I'm hopeful). It's a bit more verbose than the old construct, but equally readable.

[pitrou]

@github-actions crossbow submit -g cpp

[github-actions]

Revision: 729bdc4

Submitted crossbow builds: ursacomputing/crossbow @ actions-ea9fb86209

Task	Status
test-alpine-linux-cpp
test-build-cpp-fuzz
test-conda-cpp
test-conda-cpp-valgrind
test-cuda-cpp
test-debian-11-cpp-amd64
test-debian-11-cpp-i386
test-fedora-39-cpp
test-ubuntu-20.04-cpp
test-ubuntu-20.04-cpp-bundled
test-ubuntu-20.04-cpp-minimal-with-formats
test-ubuntu-20.04-cpp-thread-sanitizer
test-ubuntu-22.04-cpp
test-ubuntu-22.04-cpp-20
test-ubuntu-22.04-cpp-no-threading
test-ubuntu-24.04-cpp
test-ubuntu-24.04-cpp-gcc-14

[pitrou]

Thanks! The AppVeyor is unexpected but also seems unrelated. I'll merge.

[pitrou]

@raulcd This is a possible candidate for 15.0.2, but not critical.

[bdice]

Thanks all! This would be nice to get in 15.0.2 if possible. It blocks RAPIDS updating Arrow from 14 to 15. Not super high priority - so it's fine if it slips to a later patch release or a near-term major release - but it would be nice to keep up with the latest Arrow major version.

[raulcd]

I have created a new Release Candidate to include this issue so we can unblock RAPIDS.

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 7 benchmarking runs that have been run so far on merge-commit ed19a92.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them.

[vyasr]

Thank you for pushing on the patch release!