-
Notifications
You must be signed in to change notification settings - Fork 204
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
Drop implementation of thrust::pair
and thrust::tuple
#2395
Drop implementation of thrust::pair
and thrust::tuple
#2395
Conversation
We previously moved them back to proper class definitions, as using alias declarations broke CTAD. Thanks to @bernhardmgruber who realized that instead of making them an alias we can just pull them in and be done with it.
inline _CCCL_HOST_DEVICE tuple_of_iterator_references() | ||
: super_t() | ||
{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: why do we need to call the base class ctor explicitly? tuple_of_iterator_references() = default;
should do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont remember exactly why, but there were some instances, where certain compilers complained about deleted default constructors 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know these compilers? Can we add a comment about this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather not, its not really helping and running full CI just to find that some old nvcc with some old host compiler is broken does not really add value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So at what point in time can I remove this workaround then? This is the value of that comment. I will see this line at a brainless point in the future and drop it, because it is semantically unnecessary. A comment can tell me to keep it because of X until Y.
In any case, no change needed then. I may just find it out myself at some point.
We also have to drop |
I changed the docs to state that we provide it but users should move to |
🟨 CI finished in 9h 55m: Pass: 98%/433 | Total: 8d 10h | Avg: 28m 06s | Max: 1h 08m | Hits: 52%/41576
|
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
CUB | |
+/- | Thrust |
CUDA Experimental | |
pycuda | |
CUDA C Core Library |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
+/- | CUB |
+/- | Thrust |
+/- | CUDA Experimental |
+/- | pycuda |
+/- | CUDA C Core Library |
🏃 Runner counts (total jobs: 433)
# | Runner |
---|---|
320 | linux-amd64-cpu16 |
62 | linux-amd64-gpu-v100-latest-1 |
28 | linux-arm64-cpu16 |
23 | windows-amd64-cpu16 |
🟩 CI finished in 12h 19m: Pass: 100%/433 | Total: 8d 11h | Avg: 28m 08s | Max: 1h 08m | Hits: 52%/41576
|
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
CUB | |
+/- | Thrust |
CUDA Experimental | |
pycuda | |
CUDA C Core Library |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
+/- | CUB |
+/- | Thrust |
+/- | CUDA Experimental |
+/- | pycuda |
+/- | CUDA C Core Library |
🏃 Runner counts (total jobs: 433)
# | Runner |
---|---|
320 | linux-amd64-cpu16 |
62 | linux-amd64-gpu-v100-latest-1 |
28 | linux-arm64-cpu16 |
23 | windows-amd64-cpu16 |
Co-authored-by: Bernhard Manfred Gruber <[email protected]>
🟩 CI finished in 4h 21m: Pass: 100%/433 | Total: 2d 17h | Avg: 9m 04s | Max: 1h 29m | Hits: 94%/41576
|
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
CUB | |
+/- | Thrust |
CUDA Experimental | |
pycuda | |
CUDA C Core Library |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
+/- | CUB |
+/- | Thrust |
+/- | CUDA Experimental |
+/- | pycuda |
+/- | CUDA C Core Library |
🏃 Runner counts (total jobs: 433)
# | Runner |
---|---|
320 | linux-amd64-cpu16 |
62 | linux-amd64-gpu-v100-latest-1 |
28 | linux-arm64-cpu16 |
23 | windows-amd64-cpu16 |
We previously moved them back to proper class definitions, as using alias declarations broke CTAD.
Thanks to @bernhardmgruber who realized that instead of making them an alias we can just pull them in and be done with it.