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

fix: workaround NVCC parse failure in cast_op #4893

Merged
merged 3 commits into from
Oct 21, 2023

Conversation

Flamefire
Copy link
Contributor

There is a bug in some CUDA versions (observed in CUDA 12.1 and 11.7 w/ GCC 12.2), that makes cast_op fail to compile:
cast.h:45:120: error: expected template-name before ‘<’ token

Defining the nested type as an alias and using it allows this to work without any change in semantics.

Fixes #4606

The alternative using a static_cast or similar fails due to ambiguity with the const Foo& and Foo& operators (one of the tests)

Suggested changelog entry:

Improve compatibility with the nvcc compiler (especially CUDA 12.1/12.2)

Flamefire and others added 2 commits October 20, 2023 11:05
There is a bug in some CUDA versions (observed in CUDA 12.1 and 11.7 w/ GCC 12.2),
that makes `cast_op` fail to compile:
  `cast.h:45:120: error: expected template-name before ‘<’ token`

Defining the nested type as an alias and using it allows this to work
without any change in semantics.

Fixes pybind#4606
@@ -42,13 +42,15 @@ using make_caster = type_caster<intrinsic_t<type>>;
// Shortcut for calling a caster's `cast_op_type` cast operator for casting a type_caster to a T
template <typename T>
typename make_caster<T>::template cast_op_type<T> cast_op(make_caster<T> &caster) {
return caster.operator typename make_caster<T>::template cast_op_type<T>();
using result_t = typename make_caster<T>::template cast_op_type<T>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please add a comment at the end of the line, e.g.

// See PR #4893.

Also below.

In case someone later is considering inlining result_t again, they can easily know why it's here.

@rwgk rwgk merged commit 3414c56 into pybind:master Oct 21, 2023
84 checks passed
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Oct 21, 2023
@Flamefire Flamefire deleted the cuda-compat branch October 22, 2023 08:19
@correaa
Copy link

correaa commented Oct 31, 2023

If someone has this problem and cannot update pybind11 for any reason, note that, to me at least, this doesn't happen with GCC 11 as host compiler. e.g. cmake .. -DCMAKE_CUDA_HOST_COMPILER=g++-11 ...

@correaa
Copy link

correaa commented Nov 14, 2023

not sure, how can one check if this PR went into a tagged version (not master) of pybind11 ?

@rwgk
Copy link
Collaborator

rwgk commented Nov 14, 2023

not sure, how can one check if this PR went into a tagged version (not master) of pybind11 ?

There was no release of any kind in the time after this PR was merged.

@Flamefire
Copy link
Contributor Author

not sure, how can one check if this PR went into a tagged version (not master) of pybind11 ?

Github offers a nice way: Open the merge commit found in the PR (message like " merged commit 3414c56 into pybind:master") and Github will show the branches and tags this is contained in between the commit message and author(s). Right now it only shows the master branch but once a tag is out, that would be shown too unless the tag is from a different branch.

In the meantime you can pick this PR as a patch and apply it manully by appending .diff or .patch to the commit or PR URL, e.g. https://github.com/pybind/pybind11/pull/4893.patch

@henryiii henryiii changed the title Workaround NVCC parse failure in cast_op fix: workaround NVCC parse failure in cast_op Nov 15, 2023
@henryiii henryiii removed the needs changelog Possibly needs a changelog entry label Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG]: Cuda 12.1: error: expected template-name before ‘<’ token
4 participants