feature: script to toggle clang-format on/off, clang-format exercises and examples#1748
feature: script to toggle clang-format on/off, clang-format exercises and examples#1748johnbowen42 wants to merge 7 commits intodevelopfrom
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.
rhornung67
left a comment
rhornung67
left a comment
There was a problem hiding this comment.
This still needs work to filter out formatting of nested template code sections, especially involving RAJA::kernel.
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< | ||
| 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
|
|
||
| *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?
3 participants
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