Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG]: CTAD is broken for thrust::pair #1642

Closed
1 task done
jrhemstad opened this issue Apr 16, 2024 · 2 comments · Fixed by #1643
Closed
1 task done

[BUG]: CTAD is broken for thrust::pair #1642

jrhemstad opened this issue Apr 16, 2024 · 2 comments · Fixed by #1643
Assignees
Labels
bug Something isn't working right.

Comments

@jrhemstad
Copy link
Collaborator

Is this a duplicate?

Type of Bug

Compile-time Error

Component

Thrust

Describe the bug

The refactor to make thrust::pair an alias for cuda::std::pair ended up breaking CTAD because CTAD doesn't work for alias templates.

How to Reproduce

Try to compile:

#include <thrust/pair.h>
auto p = thrust::pair{42,42};

yields:

<source>(3): error: argument list for alias template "thrust::THRUST_200400_520_NS::pair" is missing
  auto p = thrust::pair{42,42};

Expected behavior

CTAD should work for thrust::pair.

Reproduction link

https://godbolt.org/z/1s6as73bb

Operating System

No response

nvidia-smi output

No response

NVCC version

No response

@jrhemstad jrhemstad added the bug Something isn't working right. label Apr 16, 2024
@github-project-automation github-project-automation bot moved this to Todo in CCCL Apr 16, 2024
@bernhardmgruber
Copy link
Contributor

I had this problem in the past, so here are some thoughts: I could not find an easy solution in C++17, but it works out of the box in C++20 (via P1021]. A deduction guide for the alias does not work. The best solution I found back than (for a more complex type) was to inherit instead of alias in C++17, and redefine the public API where necessary:

namespace thrust {
    template <typename T, typename U>
    struct pair : cuda::std::pair<T, U> { };
    // ... specialize tuple protocol for thrust::pair
}

Given the comparable simple implementation of cuda::std::pair, we may just have a distinct (duplicate) pair implementation in thrust until C++20.

@miscco
Copy link
Collaborator

miscco commented Apr 17, 2024

Yeah that is definitely an option. I just hate the additional boilerplate that we need to use.

Mind that we then also need to specialize all of {cuda::}std::tuple_size and {cuda::}std::tuple_element

@jrhemstad jrhemstad linked a pull request Apr 24, 2024 that will close this issue
@github-project-automation github-project-automation bot moved this from Todo to Done in CCCL May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working right.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants