GH-44898: [C++] Fix compilation error on GCC 8

[pitrou]

Rationale for this change

The span implementation featured a complicated SFINAE construct to enable the span(range...) constructor, which failed compiling on gcc 8.x.

What changes are included in this PR?

Replace this complicated SFINAE with a much simpler one that avoids the recursive span template dependency while keeping the intended semantics.

Are these changes tested?

Yes, by existing tests.

Are there any user-facing changes?

No.

• GitHub Issue: [C++] Compilation error on gcc 8 #44898

[pitrou]

@github-actions crossbow submit -g cpp -g r

[github-actions]

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

[github-actions]

Revision: 02ca3b5

Submitted crossbow builds: ursacomputing/crossbow @ actions-f791cb99c4

Task	Status
example-cpp-minimal-build-static
example-cpp-minimal-build-static-system-dependency
example-cpp-tutorial
r-binary-packages
r-recheck-most
test-alpine-linux-cpp
test-build-cpp-fuzz
test-conda-cpp
test-conda-cpp-valgrind
test-cuda-cpp-ubuntu-20.04-cuda-11.2.2
test-cuda-cpp-ubuntu-22.04-cuda-11.7.1
test-debian-12-cpp-amd64
test-debian-12-cpp-i386
test-fedora-39-cpp
test-r-arrow-backwards-compatibility
test-r-clang-sanitizer
test-r-depsource-bundled
test-r-depsource-system
test-r-dev-duckdb
test-r-devdocs
test-r-extra-packages
test-r-gcc-11
test-r-gcc-12
test-r-install-local
test-r-install-local-minsizerel
test-r-linux-as-cran
test-r-linux-rchk
test-r-linux-valgrind
test-r-macos-as-cran
test-r-minimal-build
test-r-offline-maximal
test-r-offline-minimal
test-r-rhub-debian-gcc-devel-lto-latest
test-r-rhub-debian-gcc-release-custom-ccache
test-r-rhub-ubuntu-release-latest
test-r-rocker-r-ver-latest
test-r-rstudio-r-base-4.1-opensuse155
test-r-rstudio-r-base-4.2-focal
test-r-ubuntu-22.04
test-r-versions
test-ubuntu-20.04-cpp
test-ubuntu-20.04-cpp-bundled
test-ubuntu-22.04-cpp
test-ubuntu-22.04-cpp-20
test-ubuntu-22.04-cpp-emscripten
test-ubuntu-22.04-cpp-no-threading
test-ubuntu-24.04-cpp
test-ubuntu-24.04-cpp-bundled-offline
test-ubuntu-24.04-cpp-gcc-13-bundled
test-ubuntu-24.04-cpp-gcc-14
test-ubuntu-24.04-cpp-minimal-with-formats
test-ubuntu-24.04-cpp-thread-sanitizer
test-ubuntu-r-sanitizer

[pitrou]

@github-actions crossbow submit wheelcp312*

[github-actions]

Revision: 02ca3b5

Submitted crossbow builds: ursacomputing/crossbow @ actions-8100cd399e

Task	Status
wheel-macos-monterey-cp312-cp312-amd64
wheel-macos-monterey-cp312-cp312-arm64
wheel-manylinux-2-28-cp312-cp312-amd64
wheel-manylinux-2-28-cp312-cp312-arm64
wheel-manylinux-2014-cp312-cp312-amd64
wheel-manylinux-2014-cp312-cp312-arm64
wheel-windows-cp312-amd64

[assignUser]

Very nice simplification, thanks for the quick fix!

[assignUser]

The r failures seem unrelated and are also failing in nightlies

[zanmato1984]

Hi @pitrou , could you elaborate a bit more on why GCC 8 complains about the original code and how this change fixes it?

[pitrou]

Well, I'm not sure. I didn't manage to make sense of the error messages, so instead I opted to trying to remove complexity from the existing constructs until it fixed the issue :)

[zanmato1984]

I guess this kind of compiler version specific error can be hard to interpret (I tried to decode it too but no avail). So I'm good as long as the compiler doesn't complain.

[zanmato1984]

LGTM.

BTW, since the removed trait was introduced for #40252 (nvcc), did we try this fix on nvcc (from eyeball check it is supposed to work though)? Or shall we ask @vyasr for help verifying?

[pitrou]

I don't have a nvcc setup ready, but @vyasr might want to take a look indeed.

[conbench-apache-arrow]

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

There were 132 benchmark results with an error:

• Commit Run on arm64-t4g-2xlarge-linux at 2024-12-06 11:05:05Z
  • tpch (R) with engine=arrow, format=parquet, language=R, memory_map=False, query_id=TPCH-07, scale_factor=1
  • tpch (R) with engine=arrow, format=native, language=R, memory_map=False, query_id=TPCH-09, scale_factor=1
• and 130 more (see the report linked below)

There were no benchmark performance regressions. 🎉

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

[vyasr]

@pitrou @zanmato1984 I ran the exact same test from #40252 with both the commit right after this PR merged and the latest main, and both compiled under nvcc so I think we're fine. I do still see a number of compiler warnings (including the ones from #40252 but those aren't blockers.

[pitrou]

Thanks for the feedback @vyasr !