Update clang-format version

[vpirogov]

We are stuck at clang-format-11, which is quite old, because of behavior differences with newer version. The intent of this PR is to update version in CI and address formatting differences that arise.

[theComputeKid]

Well.. that is not good..

[theComputeKid]

Clang-format 14 is widely available and also agrees with 11 if you want a less disruptive move.

[vpirogov]

Clang-format 14 is widely available and also agrees with 11 if you want a less disruptive move.

Thanks, @theComputeKid. I'll check whether we can adjust settings in .clang-format to avoid massive diff.

[vpirogov]

I'm lost on why clang-tidy-18 wants to do this:

--- a/examples/graph/sycl_getting_started.cpp
+++ b/examples/graph/sycl_getting_started.cpp
@@ -251,9 +251,9 @@ void sycl_getting_started_tutorial(dnnl::engine::kind ekind) {
     //[Define sycl queue]
     sycl::queue q = (ekind == engine::kind::gpu)
             ? sycl::queue(
-                    sycl::gpu_selector_v, sycl::property::queue::in_order {})
+                      sycl::gpu_selector_v, sycl::property::queue::in_order {})
             : sycl::queue(
-                    sycl::cpu_selector_v, sycl::property::queue::in_order {});
+                      sycl::cpu_selector_v, sycl::property::queue::in_order {});
     //[Define sycl queue]
 
     /// Create a #dnnl::engine based on SYCL device and context. Also,

[rjoursler]

I'm lost on why clang-tidy-18 wants to do this:

To me, this just looks like a bug. From the documentation for AlignAfterOpenBracket, we use DontAlign which states:

BAS_DontAlign (in configuration: DontAlign) Don’t align, instead use ContinuationIndentWidth, e.g.:

The ContinuationIndentWidth we set is 8 and Clang-18 is instead indenting by 10.

It looks like clang-tidy-17 works as expected though.

Going by this issue, it looks like there may be a few issues with the DontAlign setting aligning when it shouldn't.

[vpirogov]

clang-format-14 has AllowShortFunctionsOnASingleLine: Inline broken. Here's sample diff:

diff --git a/src/common/engine_impl.hpp b/src/common/engine_impl.hpp
index e42b8fa27f..ad6e26cb74 100644
--- a/src/common/engine_impl.hpp
+++ b/src/common/engine_impl.hpp
@@ -56,11 +56,17 @@ public:
     }

 #ifdef ONEDNN_BUILD_GRAPH
-    void *get_allocator() const { return (void *)(&allocator_); };
-    void set_allocator(graph::allocator_t *alloc) { allocator_ = *alloc; }
+    void *get_allocator() const {
+        return (void *)(&allocator_);
+    };
+    void set_allocator(graph::allocator_t *alloc) {
+        allocator_ = *alloc;
+    }
 #endif

-    virtual status_t init() { return status::success; }
+    virtual status_t init() {
+        return status::success;
+    }

     virtual status_t create_memory_storage(memory_storage_t **storage,
             engine_t *engine, unsigned flags, size_t size, void *handle) const {

Upd: This is a bug llvm/llvm-project#54901

[vpirogov]

I've experimented with several versions so far and every one has some quirks. In depth analysis of some changes reported below indicates either bugs or behavior changes of existing functionality. I was not able to find knobs to reduce produced diff.

Here's the data to illustrate the issue we are facing:

version	# files changed	diff
14.0.6	159	clang-format-14.diff.txt
17.0.6	140	clang-format-17.diff.txt
18.1.2	426	clang-format-18.diff.txt

[vpirogov]

clang-format-17 hates pure virtual function declarations for some reason:

diff --git a/src/common/engine.hpp b/src/common/engine.hpp
index 65cdce5d5b..abe84715e3 100644
--- a/src/common/engine.hpp
+++ b/src/common/engine.hpp
@@ -113,23 +113,27 @@ struct dnnl_engine : public dnnl::impl::c_compatible {
      * a NULL-terminated list */
     virtual const dnnl::impl::impl_list_item_t *get_reorder_implementation_list(
             const dnnl::impl::memory_desc_t *src_md,
-            const dnnl::impl::memory_desc_t *dst_md) const = 0;
+            const dnnl::impl::memory_desc_t *dst_md) const
+            = 0;

This specific change can be reverted with BreakBeforeBinaryOperators: NonAssignment, but it leads to massive changes elsewhere.