[SYCL] Don't use boost/mp11 for properties filtering #16076
Conversation
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
aelovikov-intel
force-pushed
the
properties-filtering
branch
from
2a01067 to
03d7051
Compare
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
steffenlarsen
left a comment
steffenlarsen
left a comment
There was a problem hiding this comment.
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.
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.
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.
The naming scheme used in this patch does not seem to follow the surrounding code.
There was a problem hiding this comment.
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.
2 participants
We want to get rid of it for the upstreaming purposes.