-
Notifications
You must be signed in to change notification settings - Fork 83
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 -Wc++11-narrowing
warnings
#856
Fix -Wc++11-narrowing
warnings
#856
Conversation
Newer versions of `clang` make it error by default, which affect clang-based implementations (like DPCPP).
Note for reviewers: the main change is in It is possible to preserve the helper in form of a function, by doing something like: template <typename T>
inline constexpr auto get_init_value_helper(int x) {
return T{ static_cast<typename get_element_type<T>::type>(x) };
} Where |
@@ -48,11 +48,11 @@ constexpr int default_val = 20; | |||
|
|||
template <typename T, int case_num> | |||
constexpr sycl::specialization_id<T> spec_const( | |||
user_def_types::get_init_value_helper<T>(default_val)); | |||
user_def_types::init_value_helper<T>::get(default_val)); |
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.
What is the value of having struct
instead of generic functions like
user_def_types::init_value_helper<T>::get(default_val)); | |
user_def_types::get_init_value<T>(default_val)); |
?
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.
@keryell, I think @AlexeySachkov is seeking for discussion of this change in this comment: #856 (comment)
However, I do like the new name better, because with the old name one might think that function is returning some kind of helper instead of an init value. Let me know about the preference, I can rework the PR accordingly.
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 do not like the word helper
which does not help the understanding. :-)
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.
What is the value of having struct instead of generic functions like
Yes, that's a good point. We can still have it as a function, but we can also rename it along the way, I will update the PR
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.
Refactored back to function, removed helper
from the name in 6211bfc
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.
Thanks for simplification!
Newer versions of
clang
make it error by default, which affect clang-based implementations (like DPCPP).