feature: script to toggle clang-format on/off, clang-format exercises and examples #1748
Conversation
johnbowen42
changed the title
| RAJA::ReduceMinLoc<reduce_policy, int> kernel_minloc( | ||
| std::numeric_limits<int>::max(), -1); | ||
| RAJA::ReduceMaxLoc<reduce_policy, int> kernel_maxloc( | ||
| std::numeric_limits<int>::min(), -1); |
There was a problem hiding this comment.
I'm not sure if this an improvement, what do others think? @LLNL/raja-core ?
| > | ||
| > | ||
| >; | ||
| using kernel_pol = RAJA::KernelPolicy<RAJA::statement::HipKernelFixed< |
There was a problem hiding this comment.
Changes like this (and throughout) are not good. It's not consistent and hinders readability. The script should turn off clang-format for RAJA::kernel policies in all tests, examples, etc.
There was a problem hiding this comment.
I just didn't apply the script to the benchmarks subdirectory
There was a problem hiding this comment.
I think it should not be applied to exercises, examples, and possibly tests (I need to look at those) based on what I am seeing.
could you clarify if there are any issues in the examples of exercises directories? I didn't apply the script to the benchmarks directory |
| // using Pol = KernelPolicy< | ||
| // For<1, RAJA::seq_exec>, | ||
| // For<0, RAJA::omp_target_parallel_for_exec<1>, Lambda<0> > | ||
| // >; | ||
| using Pol = KernelPolicy< | ||
| Collapse<omp_target_parallel_collapse_exec, ArgList<0,1>, Lambda<0> > >; | ||
| // clang-format on | ||
| Collapse<omp_target_parallel_collapse_exec, ArgList<0, 1>, Lambda<0>>>; |
There was a problem hiding this comment.
Here's an example of what I don't like. Putting all the closing '>' on the same line makes it harder to parse visually. I prefer that the indentation pattern that we follow consistently for the most part, where opening '<' and 'closing '>' are vertically aligned in execution policies.
There was a problem hiding this comment.
the closing > were all on the same line before clang format was applied, in this case
| @@ -140,20 +143,21 @@ int main(int RAJA_UNUSED_ARG(argc), char** RAJA_UNUSED_ARG(argv[])) | |||
|
|
|||
| *atomic_pi = 0.0; | |||
|
|
|||
| // clang-format off | |||
There was a problem hiding this comment.
Why did the script insert the clangt-format off comment here, but not at line 129 above?
This is a refactoring PR that uses a script to insert annotations in cpp source code. It inserts annotations before and after templated function calls or declarations inside the
examplesandexercisessubdirectories, as these tend to have the most complex examples of such calls and declarations.see also #1731