-
Notifications
You must be signed in to change notification settings - Fork 790
[SYCL] Don't use boost/mp11 for properties filtering #16076
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
[SYCL] Don't use boost/mp11 for properties filtering #16076
Conversation
2a01067
to
03d7051
Compare
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.
Overall, I think it looks good! Very clean! 🧹
std::array<int, N> indexes{}; | ||
int num_matched = 0; | ||
int idx = 0; | ||
(((predicate<property_tys>::value ? indexes[num_matched++] = idx++ : idx++), |
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.
For the sake of mental association, I would add another set of parenthesis:
(((predicate<property_tys>::value ? indexes[num_matched++] = idx++ : idx++), | |
(((predicate<property_tys>::value ? (indexes[num_matched++] = idx++) : idx++), |
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'll make a note of that and will implement in a separate PR with final touches after major refactorings are all finished.
const properties<properties_type_list<property_tys...>> &props) { | ||
return filter_properties_impl<predicate, property_tys...>::apply(props); | ||
} | ||
|
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.
The naming scheme used in this patch does not seem to follow the surrounding code.
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.
Discussed offline - I'm following the style in properties.hpp
because I'll be moving this new code into a single source and removing the surrounding code in near future. Also, we're already very inconsistent in different places with properties.
We want to get rid of it for the upstreaming purposes.