[clang-format] Reformat due to transition from v9.0.0 to v19.1.0 and update of style configuration
Description
This PR is a preparation step for issue #580. Since we will move from clang-format v9.0.0 to v19.1.0, this PR reformats the code according to v19.1.0. This PR is separated from the CI enabling (PR #595) in order to have a more informative git blame.
There are some changes I want to discuss.
-
src/lapack/backends/cusolver/cusolver_lapack.cppThe following changes are suggested byclang-format:
Because it looks too funny, I added- queue.submit([&](sycl::handler &cgh) { - onemkl_cusolver_host_task(cgh, queue, [=](CusolverScopedContextHandler &sc) { - auto handle = sc.get_handle(queue); - cusolverStatus_t err; - CUSOLVER_ERROR_FUNC_T_SYNC(func_name, func, err, handle, m, n, scratch_size); - }); - }).wait(); + queue + .submit([&](sycl::handler &cgh) { + onemkl_cusolver_host_task(cgh, queue, [=](CusolverScopedContextHandler &sc) { + auto handle = sc.get_handle(queue); + cusolverStatus_t err; + CUSOLVER_ERROR_FUNC_T_SYNC(func_name, func, err, handle, m, n, scratch_size); + }); + }) + .wait();// clang-format off/onfor all the similar suggestions in this file. -
tests/unit_tests/dft/include/{compute_out_of_place,compute_inplace}.hppAlthough I don't like the following formatting personally, it is probably still fine. I can turn the formatting off if there are objections.- EXPECT_TRUE(check_equal_strided<domain == oneapi::mkl::dft::domain::REAL>( - bwd_ptr + backward_distance * i, out_host_ref.data() + ref_distance * i, sizes, - strides_bwd_cpy, abs_error_margin, rel_error_margin, std::cout)); + EXPECT_TRUE(check_equal_strided < domain == + oneapi::mkl::dft::domain::REAL > + (bwd_ptr + backward_distance * i, + out_host_ref.data() + ref_distance * i, sizes, strides_bwd_cpy, + abs_error_margin, rel_error_margin, std::cout));
Checklist
All Submissions
- Do all unit tests pass locally? Only code reformatting. The CPU tests in CI passed.
- [X] Have you formatted the code using clang-format?
The diff looks good to me overall.
Regarding the changes in src/lapack/backends/cusolver/cusolver_lapack.cpp, could we try to change the clang-format configuration file to get a good enough formatting without // clang-format off/on?
Looking at the repository's _clang-format and the documentation there are a bunch of deprecated configurations that we should update:
-
Standard: Cpp11->Standard: c++17(orStandard: Latest) -
AllowAllConstructorInitializersOnNextLineandConstructorInitializerAllOnOneLineOrOnePerLinecan be replaced withPackConstructorInitializers: Never -
AlwaysBreakAfterDefinitionReturnTypecan be removed -
AlwaysBreakAfterReturnType->BreakAfterReturnType: Automatic -
AlwaysBreakTemplateDeclarations->BreakTemplateDeclarations: Yes -
KeepEmptyLinesAtTheStartOfBlocks->KeepEmptyLines: AtEndOfFile: true AtStartOfBlock: false AtStartOfFile: false -
SpaceInEmptyParentheses->SpacesInParens: Never
Maybe this will be enough to fix the issue. If not we could maybe add the configuration PenaltyIndentedWhitespace: 500 for instance.
I agree with @Rbiessy that we should try to improve the configuration file rather than using // clang-format off. This PR is a good opportunity to improve the config file.
It looks like this is just how clang-format handles method chaining:
queue
.submit([&](sycl::handler& cgh) {
// body
});
})
.wait();
I didn't find an option for this unfortunately. One workaround is to write queue.submit() and queue.wait() separately, but this is against the purpose of using automatic formatting. I think we just let clang-format do its job...
The style configuration file is updated in the commit ff81d95. Most changes and clean-up are straightforward. However, one significant change is DerivePointerAlignment: false. The description of this option is
If
true, analyze the formatted file for the most common alignment of&and*. Pointer and reference alignment styles are going to be updated according to the preferences found in the file.PointerAlignmentis then used only as fallback.
Because the alignment style is according to the preferences found in the file, the current code base has both left and right alignments, for example, https://github.com/oneapi-src/oneMKL/blob/6923d402d5bccba9ae1966062bc5a277fc74776c/examples/blas/run_time_dispatching/level3/gemm_usm.cpp#L63 https://github.com/oneapi-src/oneMKL/blob/6923d402d5bccba9ae1966062bc5a277fc74776c/examples/blas/compile_time_dispatching/level3/gemm_usm_mklcpu_cublas.cpp#L64 I am open to either left or right alignment, but we must choose one in my opinion.
I didn't mean to approve on behalf of the other teams, I don't know of way to change this in GitHub so I have just requested for their approval again!
@oneapi-src/onemkl-rng-write, @oneapi-src/onemkl-sparse-write, @oneapi-src/onemkl-dft-write I plan to merge the PR on 10/22 EOD Pacific time if there are no objections.