Constrain tuple_of_iterator_references -> tuple conversion on element convertibility (fixes GCC 9 build break)#9284
Conversation
…element convertibility The conversion from thrust's tuple_of_iterator_references<Us...> to cuda::std::tuple<Ts...> was selectable whenever the tuple structures matched, without checking that each held reference can actually construct the target element. is_constructible therefore reported true for ill-formed conversions such as tuple_of_iterator_references<const int&> -> tuple<int&>, which then hard-error when the constructor body is instantiated -- e.g. during a contiguous_iterator concept check on a thrust zip_iterator. GCC 9 instantiates that path and breaks the build (cudax thrust_zip_iterator example); GCC 11+ happen not to. Constrain both entry points on per-element convertibility: - thrust tuple_of_iterator_references::operator tuple<Us...>(): require (is_convertible_v<Ts, Us> && ...) in addition to is_compatible_tuple_v. - libcu++ tuple's tuple_of_iterator_references constructor: require each _Tp be constructible from the corresponding get<>() result. Both are needed; either alone still leaves is_constructible reporting true. No success-path change: valid ref->ref, ref->value, and const-ref->value conversions still work.
|
/ok to test c8b057d |
|
/ok to test c8b057d |
|
Worried about impact? Review this PR in Change Stack to explore blast radius before you approve or request changes.
OverviewThis PR fixes a GCC 9 / C++17 build break caused by incorrect handling of tuple conversions from Changeslibcudacxx/include/cuda/std/__tuple_dir/tuple.h
thrust/thrust/iterator/detail/tuple_of_iterator_references.h
Technical DetailsThe root cause was that conversions were gated only by structure/size compatibility, not per-element constructibility. This caused trait checks like Impact
WalkthroughThis PR tightens type safety for tuple-of-iterator-references conversions across libcudacxx and thrust. It adds a ChangesTuple type-safety constraints
Suggested reviewers
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 788ac7b1-e4a4-4f21-bbcf-b879c41d6266
📒 Files selected for processing (2)
libcudacxx/include/cuda/std/__tuple_dir/tuple.hthrust/thrust/iterator/detail/tuple_of_iterator_references.h
| template <class _TupleTypes, class _TupleOfIteratorReferences, class _Indices> | ||
| inline constexpr bool __tuple_of_iterator_references_constructible_v = false; | ||
|
|
||
| template <class... _Tp, class _TupleOfIteratorReferences, size_t... _Indices> | ||
| inline constexpr bool __tuple_of_iterator_references_constructible_v<__tuple_types<_Tp...>, | ||
| _TupleOfIteratorReferences, | ||
| __tuple_indices<_Indices...>> = | ||
| (is_constructible_v<_Tp, decltype(::cuda::std::get<_Indices>(::cuda::std::declval<_TupleOfIteratorReferences>()))> | ||
| && ...); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="libcudacxx/include/cuda/std/__tuple_dir/tuple.h"
# Show the include block and the trait segment around the reported lines
sed -n '1,120p' "$file"Repository: NVIDIA/cccl
Length of output: 5434
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether forward.h (or other directly-included headers) provides declval
rg -n "declval" libcudacxx/include/cuda/std/__utility/forward.h libcudacxx/include/cuda/std/__utility/move.h libcudacxx/include/cuda/std/__utility/pair.h libcudacxx/include/cuda/std/__utility/swap.h || true
# Show the actual declval declaration/definition location
rg -n "template\s*<.*>\s*.*declval|struct\s*declval|declval\(" libcudacxx/include/cuda/std/__utility/declval.h || trueRepository: NVIDIA/cccl
Length of output: 219
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="libcudacxx/include/cuda/std/__utility/forward.h"
sed -n '1,140p' "$file"Repository: NVIDIA/cccl
Length of output: 3404
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="libcudacxx/include/cuda/std/__tuple_dir/tuple.h"
rg -n "declval\.h" "$file" || true
rg -n "__utility/declval\.h" "$file" || true
rg -n "declval<" "$file" || trueRepository: NVIDIA/cccl
Length of output: 175
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Where is declval.h included from?
rg -n --max-count 50 "cuda/std/__utility/declval\.h" libcudacxx/include/cuda/std || true
# Where is the underlying __declval declared/defined?
rg -n --max-count 50 "__declval" libcudacxx/include/cuda/std/__utility libcudacxx/include/cuda/std || trueRepository: NVIDIA/cccl
Length of output: 6378
suggestion: Add a direct include for declval in libcudacxx/include/cuda/std/__tuple_dir/tuple.h since __tuple_of_iterator_references_constructible_v uses ::cuda::std::declval; the current build relies on a transitive include via tuple_constraints.h.
diff
`#include` <cuda/std/__utility/forward.h>
+#include <cuda/std/__utility/declval.h>
`#include` <cuda/std/__utility/move.h>Source: Coding guidelines
| template < | ||
| class... Us, | ||
| ::cuda::std::enable_if_t<is_compatible_tuple_v<::cuda::std::tuple<Us...>, ::cuda::std::tuple<Ts...>>, int> = 0> | ||
| ::cuda::std::enable_if_t<is_compatible_tuple_v<::cuda::std::tuple<Us...>, ::cuda::std::tuple<Ts...>>, int> = 0, | ||
| // Structure compatibility alone is not enough: each held reference must actually convert to the | ||
| // requested element, otherwise this operator is selected for ill-formed conversions such as | ||
| // tuple_of_iterator_references<const int&> -> tuple<int&>, which then hard-errors in the body | ||
| // (e.g. during a contiguous_iterator concept check on a zip_iterator under GCC 9). | ||
| ::cuda::std::enable_if_t<(::cuda::std::is_convertible_v<Ts, Us> && ...), int> = 0> | ||
| _CCCL_HOST_DEVICE constexpr operator ::cuda::std::tuple<Us...>() const |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="thrust/thrust/iterator/detail/tuple_of_iterator_references.h"
echo "== Includes in $FILE =="
sed -n '1,120p' "$FILE" | nl -ba | sed -n '1,120p'
echo
echo "== Search for is_convertible_v =="
rg -n "::cuda::std::is_convertible_v" "$FILE"
echo
echo "== Show surrounding code for the operator constraints =="
sed -n '140,220p' "$FILE" | nl -baRepository: NVIDIA/cccl
Length of output: 195
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="thrust/thrust/iterator/detail/tuple_of_iterator_references.h"
echo "== Top of file (includes) =="
sed -n '1,80p' "$FILE" | cat -n
echo
echo "== Search for is_convertible_v =="
rg -n "::cuda::std::is_convertible_v" "$FILE" || true
echo
echo "== Surrounding code (lines ~140-220) =="
sed -n '140,220p' "$FILE" | cat -nRepository: NVIDIA/cccl
Length of output: 6797
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Confirm where is_convertible_v is defined
rg -n "is_convertible_v" libcudacxx cuda 2>/dev/null | head -n 50
# 2) Find the specific header that defines it (search by filename + symbol)
fd "is_convertible" -a . 2>/dev/null | head -n 50
# 3) Inspect that header's contents around the definition
H=$(fd "is_convertible.h" -a . 2>/dev/null | head -n 1 || true)
echo "First match for is_convertible.h: $H"
if [ -n "${H:-}" ]; then
sed -n '1,120p' "$H" | cat -n
rg -n "is_convertible_v" "$H" || true
fi
# 4) Check whether already-included headers include is_convertible.h
INCLUDES=(
"cuda/std/__type_traits/common_reference.h"
"cuda/std/__type_traits/enable_if.h"
)
for inc in "${INCLUDES[@]}"; do
f=$(fd "$(basename "$inc")" cuda libcudacxx 2>/dev/null | head -n 1 || true)
echo "Resolved $inc -> $f"
if [ -n "${f:-}" ]; then
rg -n "`#include` <cuda/std/__type_traits/is_convertible.h>" "$f" || true
rg -n "is_convertible" "$f" | head -n 20 || true
fi
doneRepository: NVIDIA/cccl
Length of output: 7578
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect common_reference.h and enable_if.h for is_convertible includes
for f in \
"libcudacxx/include/cuda/std/__type_traits/common_reference.h" \
"libcudacxx/include/cuda/std/__type_traits/enable_if.h" \
"libcudacxx/include/cuda/std/__tuple_dir/tuple_constraints.h" \
"libcudacxx/include/cuda/std/__tuple_dir/tuple_element.h" \
"libcudacxx/include/cuda/std/tuple"; do
if [ -f "$f" ]; then
echo "==== $f (top includes) ===="
sed -n '1,120p' "$f" | cat -n
echo
rg -n "`#include` <cuda/std/__type_traits/is_convertible.h>" "$f" || true
echo
fi
doneRepository: NVIDIA/cccl
Length of output: 18739
suggestion: tuple_of_iterator_references.h uses ::cuda::std::is_convertible_v in the tuple-conversion SFINAE; add an explicit #include <cuda/std/__type_traits/is_convertible.h> instead of relying on transitive inclusion (currently comes via cuda/std/__type_traits/common_reference.h).
Proposed diff
`#include` <cuda/std/__type_traits/common_reference.h>
`#include` <cuda/std/__type_traits/enable_if.h>
+#include <cuda/std/__type_traits/is_convertible.h>
`#include` <cuda/std/__utility/move.h>Source: Coding guidelines
😬 CI Workflow Results🟥 Finished in 7h 21m: Pass: 61%/172 | Total: 6d 17h | Max: 2h 46m | Hits: 42%/1379700See results here. |
3 participants
Summary
Converting thrust's
tuple_of_iterator_references<Us...>tocuda::std::tuple<Ts...>was selectable whenever the tuple structures matched, without checking that each held reference can actually construct the target element. Sois_constructiblereportstruefor ill-formed conversions such astuple_of_iterator_references<const int&> -> tuple<int&>, which then hard-error when the constructor body is instantiated.This surfaces as a build break: under GCC 9 / C++17 the
contiguous_iteratorconcept check on a thrustzip_iteratorinstantiates that conversion and fails to compile (e.g.cudax/examples/stf/thrust_zip_iterator.cu). GCC 11+ happen not to instantiate it, so they pass. The conversion machinery here was last touched by #9059.Root cause
Two entry points report the ill-formed conversion as viable:
thrust/iterator/detail/tuple_of_iterator_references.h—operator cuda::std::tuple<Us...>()is gated only byis_compatible_tuple_v, which checks tuple structure/size (it normalizesT&->T), not element convertibility.libcudacxx/.../__tuple_dir/tuple.h— thetuple_of_iterator_referencesconverting constructor ("Horrible hack") is constrained only on__is_tuple_of_iterator_references_v+ size, not element convertibility.is_constructible<tuple<int&,char&>, tuple_of_iterator_references<const int&,const char&>>therefore wrongly evaluates totrue.Fix
Constrain both entry points on per-element convertibility:
(is_convertible_v<Ts, Us> && ...)on the conversion operator._Tpbe constructible from the correspondingget<>()result of the incomingtuple_of_iterator_references.Both are required — either one alone still leaves
is_constructiblereportingtrue.Test plan
Minimal trait check (compiler-independent), before -> after:
tuple_of_iterator_references<const int&,const char&>->tuple<int&,char&>(ill-formed)truefalse...<int&,char&>->tuple<int&,char&>truetrue...<int&,char&>->tuple<int,char>truetrue...<const int&,const char&>->tuple<int,char>truetruecudax/examples/stf/thrust_zip_iterator.custill builds (verified locally on GCC 14).This unblocks the cudax GCC 9 / C++17 jobs that are currently red across cudax PRs (e.g. #9248, #9249, #9265).