-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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 casts to void* #4275
Fix casts to void* #4275
Conversation
Actually, hold on confused whether this or #4274 is correct. This is slightly different: std::is_void is equivalent to std::remove_cv_t not std::remove_cvref_t. Which one is actually appropiate here? Which cast should be used for void pointer and references @rwgk @lalaland ? |
If we go by testing, all tests pass with 4 different variants now, from that viewpoint any of those is fine. The best way to answer the question above is to create a full set of tests (c v ref), see what compilers actually accept, then make the tests work the way we want them to. (In this case that's probably only a very small effort.) From an elegance viewpoint, |
References to void are illegal, so std::remove_cv_t and std::remove_cvref_t are equivalent. So std::is_void is preferred. |
@@ -1179,11 +1179,9 @@ enable_if_t<cast_is_temporary_value_reference<T>::value, T> cast_safe(object &&) | |||
pybind11_fail("Internal error: cast_safe fallback invoked"); | |||
} | |||
template <typename T> | |||
enable_if_t<std::is_same<void, intrinsic_t<T>>::value, void> cast_safe(object &&) {} | |||
enable_if_t<std::is_void<T>::value, void> cast_safe(object &&) {} |
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.
Yeah, is std::is_void is more elegant.
Yes, as noted in #3861, references to void are not allowed in C++, and |
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.
* Fix casts to void* * Improve tests * style: pre-commit fixes * remove c style cast Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Aaron Gokaslan <[email protected]>
Description
The current code is broken for safe casts to void* as those are incorrectly converted to casts to void. This fixes the problem.
Fixes #4117, needed for #4125
This is a reopened version of #4273 with some minor test changes. Closes #4117.
Suggested changelog entry: