Skip to content

[thrust] Fix missing qualifiers for basic_common_reference#9292

Merged
miscco merged 2 commits into
NVIDIA:mainfrom
miscco:fix_tuple_of_iterator_references_once_more
Jun 8, 2026
Merged

[thrust] Fix missing qualifiers for basic_common_reference#9292
miscco merged 2 commits into
NVIDIA:mainfrom
miscco:fix_tuple_of_iterator_references_once_more

Conversation

@miscco
Copy link
Copy Markdown
Contributor

@miscco miscco commented Jun 8, 2026

If a tuple is compatible with tuple_of_iterator_references we want to treat it as if it had the same types as the tuple

However, we need to also consider the qualifiers

If a tuple is compatible with `tuple_of_iterator_references` we want to treat it as if it had the same types as the `tuple`

However, we need to also consider the qualifiers
@miscco miscco requested a review from a team as a code owner June 8, 2026 07:46
@miscco miscco requested a review from gevtushenko June 8, 2026 07:46
@github-project-automation github-project-automation Bot moved this to Todo in CCCL Jun 8, 2026
@cccl-authenticator-app cccl-authenticator-app Bot moved this from Todo to In Review in CCCL Jun 8, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 8, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c8dfed83-50af-4963-b485-2ab99ffff0b0

📥 Commits

Reviewing files that changed from the base of the PR and between 2b2f7af and a8ec64c.

📒 Files selected for processing (1)
  • thrust/thrust/iterator/detail/tuple_of_iterator_references.h

Note: CodeRabbit is enabled on this repository as a convenience for maintainers
and contributors. Use your best judgment when considering its review comments and
suggestions — a suggested change may be inadequate, unnecessary, or safe to ignore.
Contributors are not expected to address every comment. Human reviews are what
ultimately matter for merging.

Summary

This PR fixes missing cv/ref qualifier handling for cuda::std::basic_common_reference when treating THRUST_NS_QUALIFIER::detail::tuple_of_iterator_references as compatible with cuda::std::tuple. Instead of constructing qualified tuple element types directly, the two partial specializations now delegate to basic_common_reference instantiated on the corresponding unadorned tuple types with the original qualifier helpers. The implementation is simplified and an explanatory comment was added.

Changes

File: thrust/thrust/iterator/detail/tuple_of_iterator_references.h

  • Reworked two partial specializations of cuda::std::basic_common_reference that handle compatibility between tuple_of_iterator_references<...> and tuple<...>:
    • Replaced direct formation of tuple<_TQual<_TTypes>...> / tuple<_UQual<_UTypes>...> with inheritance from basic_common_reference< tuple<_...>, tuple<_...>, _TQual, _UQual >. This ensures qualifiers are considered when computing element-wise common reference types.
    • Adjusted the enable_if_t compatibility condition to use unqualified tuple<...> type expressions.
    • Simplified implementation and added an explanatory comment.

Impact

Correctly accounts for cv/ref qualifiers when computing common reference types for tuples compatible with tuple_of_iterator_references, preventing incorrect/common-missed qualifier propagation and improving type correctness in generic tuple reference resolution.

suggestion:

Walkthrough

Two partial specializations of cuda::std::basic_common_reference in tuple_of_iterator_references.h now delegate their resulting type to basic_common_reference instantiated on the corresponding plain tuple<...> types and use unqualified tuple<...> in the enable_if compatibility checks.

Changes

Common Reference Delegation

Layer / File(s) Summary
basic_common_reference specializations
thrust/thrust/iterator/detail/tuple_of_iterator_references.h
Both basic_common_reference<tuple_of_iterator_references, tuple> and basic_common_reference<tuple, tuple_of_iterator_references> specializations now inherit their type from basic_common_reference<tuple<...>, tuple<...>, _TQual, _UQual> and use unqualified tuple<...> expressions in their enable_if_t conditions.

Possibly related PRs

  • NVIDIA/cccl#9261: Earlier PR modifying the same basic_common_reference handling for tuple_of_iterator_references and cuda::std::tuple type computation.

Suggested reviewers

  • ericniebler
  • elstehle

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b359e9d2-15aa-4ab0-b9ed-be3edceb83c0

📥 Commits

Reviewing files that changed from the base of the PR and between 5c20539 and 2b2f7af.

📒 Files selected for processing (1)
  • thrust/thrust/iterator/detail/tuple_of_iterator_references.h

Comment thread thrust/thrust/iterator/detail/tuple_of_iterator_references.h Outdated
Comment thread thrust/thrust/iterator/detail/tuple_of_iterator_references.h Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 8, 2026

🥳 CI Workflow Results

🟩 Finished in 2h 23m: Pass: 100%/116 | Total: 4d 03h | Max: 2h 16m | Hits: 28%/705556

See results here.

@miscco miscco merged commit dd4451d into NVIDIA:main Jun 8, 2026
139 checks passed
@miscco miscco deleted the fix_tuple_of_iterator_references_once_more branch June 8, 2026 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants