-
Notifications
You must be signed in to change notification settings - Fork 118
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
Implement remaining ranges iterator concepts and modernize array #627
Conversation
5d254b0
to
65e631c
Compare
{ | ||
template <class _T1, class _T2> | ||
_LIBCUDACXX_NODISCARD_EXT _LIBCUDACXX_HIDE_FROM_ABI _LIBCUDACXX_INLINE_VISIBILITY _LIBCUDACXX_CONSTEXPR_AFTER_CXX11 bool | ||
operator()(const _T1& __lhs, const _T2& __rhs) const noexcept(noexcept(__lhs == __rhs)) |
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.
suggestion: in upstream and standard these functions do not use noexcept specifier. This can have visible side effects:
struct equal_to_1 {
template <class _T1, class _T2>
bool operator()(const _T1& __lhs, const _T2& __rhs) const noexcept(noexcept(__lhs == __rhs)) {
return __lhs == __rhs;
}
};
struct equal_to_2 {
template <class _T1, class _T2>
bool operator()(const _T1& __lhs, const _T2& __rhs) const {
return __lhs == __rhs;
}
};
int main() {
static_assert(noexcept(equal_to_1{}(1, 2)));
static_assert(noexcept(equal_to_2{}(1, 2)));
}
Unless there's a strong reason to deviate from upstream, I wouldn't do that (same for __less
).
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.
Generally, adding noexcept specifiers is allowed by the standard an QoI detial.
My point is that libc++ is not good at it and we should be better, even if we do not care about exceptions on device
6bd91e7
to
a391ed2
Compare
a391ed2
to
93a3462
Compare
This will faciliate internal testing before we decide to export the functionality to users
NOTE: This is currently disabled, as it relies on ranges features such as `ranges::begin` and `ranges::size`
ca66f31
to
dc40f7d
Compare
There were some necessary changes wrt to constexpr evaluation, which we will need going forward with ranges
dc40f7d
to
c38bb4e
Compare
This implements the remaining pieces of the iterator concepts and basic ranges CPO
It cleans up the remaining
<iterator>
infrastructure as well as the predefined iteratorsreverse_iterator
andmove_iterator
which are necessary for testing purposes later on.Finally we also modernize
<array>
which is also necessary for many incoming changes as the current empty array specialization is non-conforming wrt constexpr evaluation.Note this is currenlty based on the nonmodifying_algorithms PR as there were some merge conflicts