-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
Add build step for NVCC and fix a warning #3227
Conversation
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.
Looks great to me, thanks a lot! 🎉
@@ -664,6 +664,19 @@ class serializer | |||
} | |||
} | |||
|
|||
// templates to avoid warnings about useless casts | |||
template <typename NumberType, enable_if_t<std::is_signed<NumberType>::value, int> = 0> | |||
bool is_negative_number(NumberType x) |
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.
template <typename NumberType>
enable_if_t<std::is_signed<NumberType>::value, bool> is_negative_number(NumberType x)
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.
?
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.
Try this. You use legal but verbose and very indirect way for enable_if_t
utilize.
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.
That is a normal way to do it for functions that are already templates. https://en.cppreference.com/w/cpp/types/enable_if
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.
"legal but verbose", Ok
And how about this?
json/include/nlohmann/detail/conversions/from_json.hpp
Lines 421 to 423 in 700b95f
template < typename BasicJsonType, typename Key, typename Value, typename Compare, typename Allocator, | |
typename = enable_if_t < !std::is_constructible < | |
typename BasicJsonType::string_t, Key >::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.
I'm not sure why @theodelrieu used the longer form here compared to the rest of the file, perhaps there was a technical reason. If not, it would make sense to use the value form as in the rest of the file. c5e63fd
This PR adds a CI step for NVCC and fixes a warning reported in #2676.