Move to clang-format 15 (#621)

The current version requires clang-format-9. This now getting hard to get. This commit moves it to the clang-format-15, which is the latest in 22.04. Also, updates clang-tidy to 15 as well.
microsoft · Jul 18, 2023 · 9d44660 · 9d44660
1 parent cdfedd8
commit 9d44660
Show file tree

Hide file tree

Showing 20 changed files with 289 additions and 353 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -400,12 +400,14 @@ jobs:
 
   # Job to run clang-format and report errors
   format:
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-22.04
     # We don't need to do the build for this job, but we need to configure it to get the clang-format target
     steps:
     - uses: actions/checkout@v3
     - name: Install clang-tidy and clang-format
-      run: sudo apt install clang-tidy-9 clang-format-9
+      run: |
+        sudo apt update
+        sudo apt install clang-tidy-15 clang-format-15
     - name: Configure CMake
       run: cmake -B ${{github.workspace}}/build -DSNMALLOC_USE_CXX17=ON
     # Run the clang-format check and error if it generates a diff
@@ -417,7 +419,7 @@ jobs:
         git diff --exit-code
     - name: Run clang-tidy
       run: |
-        clang-tidy-9  src/snmalloc/override/malloc.cc  -header-filter="`pwd`/*" -warnings-as-errors='*' -export-fixes=tidy.fail -- -std=c++17 -mcx16 -DSNMALLOC_PLATFORM_HAS_GETENTROPY=0
+        clang-tidy-15  src/snmalloc/override/malloc.cc  -header-filter="`pwd`/*" -warnings-as-errors='*' -export-fixes=tidy.fail -- -std=c++17 -mcx16 -DSNMALLOC_PLATFORM_HAS_GETENTROPY=0
         if [ -f tidy.fail ] ; then
           cat tidy.fail
           exit 1

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -150,7 +150,7 @@ function(clangformat_targets)
   # tool.  It does not work with older versions as AfterCaseLabel is not supported
   # in earlier versions.
   find_program(CLANG_FORMAT NAMES
-    clang-format90 clang-format-9)
+    clang-format150 clang-format-15)
 
   # If we've found a clang-format tool, generate a target for it, otherwise emit
   # a warning.

diff --git a/src/snmalloc/aal/aal_concept.h b/src/snmalloc/aal/aal_concept.h
@@ -14,87 +14,79 @@ namespace snmalloc
    * machine word size, and an upper bound on the address space size
    */
   template<typename AAL>
-  concept IsAAL_static_members = requires()
-  {
-    typename std::integral_constant<uint64_t, AAL::aal_features>;
-    typename std::integral_constant<int, AAL::aal_name>;
-    typename std::integral_constant<std::size_t, AAL::bits>;
-    typename std::integral_constant<std::size_t, AAL::address_bits>;
-  };
+  concept IsAAL_static_members =
+    requires() {
+      typename std::integral_constant<uint64_t, AAL::aal_features>;
+      typename std::integral_constant<int, AAL::aal_name>;
+      typename std::integral_constant<std::size_t, AAL::bits>;
+      typename std::integral_constant<std::size_t, AAL::address_bits>;
+    };
 
   /**
    * AALs provide a prefetch operation.
    */
   template<typename AAL>
-  concept IsAAL_prefetch = requires(void* ptr)
-  {
-    {
-      AAL::prefetch(ptr)
-    }
-    noexcept->ConceptSame<void>;
-  };
+  concept IsAAL_prefetch = requires(void* ptr) {
+                             {
+                               AAL::prefetch(ptr)
+                               } noexcept -> ConceptSame<void>;
+                           };
 
   /**
    * AALs provide a notion of high-precision timing.
    */
   template<typename AAL>
-  concept IsAAL_tick = requires()
-  {
-    {
-      AAL::tick()
-    }
-    noexcept->ConceptSame<uint64_t>;
-  };
+  concept IsAAL_tick = requires() {
+                         {
+                           AAL::tick()
+                           } noexcept -> ConceptSame<uint64_t>;
+                       };
 
   template<typename AAL>
   concept IsAAL_capptr_methods =
-    requires(capptr::Chunk<void> auth, capptr::AllocFull<void> ret, size_t sz)
-  {
-    /**
-     * Produce a pointer with reduced authority from a more privilged pointer.
-     * The resulting pointer will have base at auth's address and length of
-     * exactly sz.  auth+sz must not exceed auth's limit.
-     */
-    {
-      AAL::template capptr_bound<void, capptr::bounds::Chunk>(auth, sz)
-    }
-    noexcept->ConceptSame<capptr::Chunk<void>>;
+    requires(capptr::Chunk<void> auth, capptr::AllocFull<void> ret, size_t sz) {
+      /**
+       * Produce a pointer with reduced authority from a more privilged pointer.
+       * The resulting pointer will have base at auth's address and length of
+       * exactly sz.  auth+sz must not exceed auth's limit.
+       */
+      {
+        AAL::template capptr_bound<void, capptr::bounds::Chunk>(auth, sz)
+        } noexcept -> ConceptSame<capptr::Chunk<void>>;
 
-    /**
-     * "Amplify" by copying the address of one pointer into one of higher
-     * privilege.  The resulting pointer differs from auth only in address.
-     */
-    {
-      AAL::capptr_rebound(auth, ret)
-    }
-    noexcept->ConceptSame<capptr::Chunk<void>>;
+      /**
+       * "Amplify" by copying the address of one pointer into one of higher
+       * privilege.  The resulting pointer differs from auth only in address.
+       */
+      {
+        AAL::capptr_rebound(auth, ret)
+        } noexcept -> ConceptSame<capptr::Chunk<void>>;
 
-    /**
-     * Round up an allocation size to a size this architecture can represent.
-     * While there may also, in general, be alignment requirements for
-     * representability, in snmalloc so far we have not had reason to consider
-     * these explicitly: when we use our...
-     *
-     * - sizeclass machinery (for user-facing data), we assume that all
-     *   sizeclasses describe architecturally representable aligned-and-sized
-     *   regions
-     *
-     * - Range machinery (for internal meta-data), we always choose NAPOT
-     *   regions big enough for the requested size (returning space above the
-     *   allocation within such regions for use as smaller NAPOT regions).
-     *
-     * That is, capptr_size_round is not needed on the user-facing fast paths,
-     * merely internally for bootstrap and metadata management.
-     */
-    {
-      AAL::capptr_size_round(sz)
-    }
-    noexcept->ConceptSame<size_t>;
-  };
+      /**
+       * Round up an allocation size to a size this architecture can represent.
+       * While there may also, in general, be alignment requirements for
+       * representability, in snmalloc so far we have not had reason to consider
+       * these explicitly: when we use our...
+       *
+       * - sizeclass machinery (for user-facing data), we assume that all
+       *   sizeclasses describe architecturally representable aligned-and-sized
+       *   regions
+       *
+       * - Range machinery (for internal meta-data), we always choose NAPOT
+       *   regions big enough for the requested size (returning space above the
+       *   allocation within such regions for use as smaller NAPOT regions).
+       *
+       * That is, capptr_size_round is not needed on the user-facing fast paths,
+       * merely internally for bootstrap and metadata management.
+       */
+      {
+        AAL::capptr_size_round(sz)
+        } noexcept -> ConceptSame<size_t>;
+    };
 
   template<typename AAL>
-  concept IsAAL = IsAAL_static_members<AAL>&& IsAAL_prefetch<AAL>&&
-    IsAAL_tick<AAL>&& IsAAL_capptr_methods<AAL>;
+  concept IsAAL = IsAAL_static_members<AAL> && IsAAL_prefetch<AAL> &&
+    IsAAL_tick<AAL> && IsAAL_capptr_methods<AAL>;
 
 } // namespace snmalloc
 #endif
diff --git a/src/snmalloc/backend/fixedglobalconfig.h b/src/snmalloc/backend/fixedglobalconfig.h
@@ -63,13 +63,11 @@ namespace snmalloc
      * C++, and not just its initializer fragment, to initialize a non-prefix
      * subset of the flags (in any order, at that).
      */
-    static constexpr Flags Options = []() constexpr
-    {
+    static constexpr Flags Options = []() constexpr {
       Flags opts = {};
       opts.HasDomesticate = true;
       return opts;
-    }
-    ();
+    }();
 
     // This needs to be a forward reference as the
     // thread local state will need to know about this.

diff --git a/src/snmalloc/ds/aba.h b/src/snmalloc/ds/aba.h
@@ -71,9 +71,10 @@ namespace snmalloc
         error("Only one inflight ABA operation at a time is allowed.");
       operation_in_flight = true;
 #  endif
-      return Cmp{{independent.ptr.load(std::memory_order_relaxed),
-                  independent.aba.load(std::memory_order_relaxed)},
-                 this};
+      return Cmp{
+        {independent.ptr.load(std::memory_order_relaxed),
+         independent.aba.load(std::memory_order_relaxed)},
+        this};
     }
 
     struct Cmp

diff --git a/src/snmalloc/ds_core/helpers.h b/src/snmalloc/ds_core/helpers.h
@@ -324,7 +324,7 @@ namespace snmalloc
       }
       std::array<char, 20> buf{{0}};
       const char digits[] = "0123456789";
-      for (long i = long(buf.size() - 1); i >= 0; i--)
+      for (long i = static_cast<long>(buf.size() - 1); i >= 0; i--)
       {
         buf[static_cast<size_t>(i)] = digits[s % 10];
         s /= 10;
@@ -356,7 +356,7 @@ namespace snmalloc
       const char hexdigits[] = "0123456789abcdef";
       // Length of string including null terminator
       static_assert(sizeof(hexdigits) == 0x11);
-      for (long i = long(buf.size() - 1); i >= 0; i--)
+      for (long i = static_cast<long>(buf.size() - 1); i >= 0; i--)
       {
         buf[static_cast<size_t>(i)] = hexdigits[s & 0xf];
         s >>= 4;

diff --git a/src/snmalloc/ds_core/mitigations.h b/src/snmalloc/ds_core/mitigations.h
@@ -247,10 +247,10 @@ namespace snmalloc
      */
     full_checks + cheri_checks + clear_meta - freelist_forward_edge -
       pal_enforce_access :
-    /**
-     * clear_meta is important on CHERI to avoid leaking capabilities.
-     */
-    sanity_checks + cheri_checks + clear_meta;
+     /**
+      * clear_meta is important on CHERI to avoid leaking capabilities.
+      */
+     sanity_checks + cheri_checks + clear_meta;
 #else
     CHECK_CLIENT ? full_checks : no_checks;
 #endif

diff --git a/src/snmalloc/ds_core/redblacktree.h b/src/snmalloc/ds_core/redblacktree.h
@@ -17,11 +17,10 @@ namespace snmalloc
    * ID.
    */
   template<typename Rep>
-  concept RBRepTypes = requires()
-  {
-    typename Rep::Handle;
-    typename Rep::Contents;
-  };
+  concept RBRepTypes = requires() {
+                         typename Rep::Handle;
+                         typename Rep::Contents;
+                       };
 
   /**
    * The representation must define operations on the holder and contents
@@ -41,50 +40,36 @@ namespace snmalloc
    */
   template<typename Rep>
   concept RBRepMethods =
-    requires(typename Rep::Handle hp, typename Rep::Contents k, bool b)
-  {
-    {
-      Rep::get(hp)
-    }
-    ->ConceptSame<typename Rep::Contents>;
-    {
-      Rep::set(hp, k)
-    }
-    ->ConceptSame<void>;
-    {
-      Rep::is_red(k)
-    }
-    ->ConceptSame<bool>;
-    {
-      Rep::set_red(k, b)
-    }
-    ->ConceptSame<void>;
-    {
-      Rep::ref(b, k)
-    }
-    ->ConceptSame<typename Rep::Handle>;
-    {
-      Rep::null
-    }
-    ->ConceptSameModRef<const typename Rep::Contents>;
-    {
-      typename Rep::Handle
+    requires(typename Rep::Handle hp, typename Rep::Contents k, bool b) {
+      {
+        Rep::get(hp)
+        } -> ConceptSame<typename Rep::Contents>;
+      {
+        Rep::set(hp, k)
+        } -> ConceptSame<void>;
+      {
+        Rep::is_red(k)
+        } -> ConceptSame<bool>;
       {
-        const_cast<
+        Rep::set_red(k, b)
+        } -> ConceptSame<void>;
+      {
+        Rep::ref(b, k)
+        } -> ConceptSame<typename Rep::Handle>;
+      {
+        Rep::null
+        } -> ConceptSameModRef<const typename Rep::Contents>;
+      {
+        typename Rep::Handle{const_cast<
           std::remove_const_t<std::remove_reference_t<decltype(Rep::root)>>*>(
-          &Rep::root)
-      }
-    }
-    ->ConceptSame<typename Rep::Handle>;
-  };
+          &Rep::root)}
+        } -> ConceptSame<typename Rep::Handle>;
+    };
 
   template<typename Rep>
   concept RBRep = //
-    RBRepTypes<Rep> //
-      && RBRepMethods<Rep> //
-        && ConceptSame<
-          decltype(Rep::null),
-          std::add_const_t<typename Rep::Contents>>;
+    RBRepTypes<Rep> && RBRepMethods<Rep> &&
+    ConceptSame<decltype(Rep::null), std::add_const_t<typename Rep::Contents>>;
 #endif
 
   /**
@@ -275,7 +260,7 @@ namespace snmalloc
       std::array<RBStep, 128> path;
       size_t length = 0;
 
-      RBPath(typename Rep::Handle root) : path{}
+      RBPath(typename Rep::Handle root)
       {
         path[0].set(root, false);
         length = 1;
@@ -490,8 +475,7 @@ namespace snmalloc
        */
       path.move(true);
       while (path.move(false))
-      {
-      }
+      {}
 
       K curr = path.curr();
 
@@ -510,8 +494,8 @@ namespace snmalloc
         // If we had a left child, replace ourselves with the extracted value
         // from above
         Rep::set_red(curr, Rep::is_red(splice));
-        get_dir(true, curr) = K(get_dir(true, splice));
-        get_dir(false, curr) = K(get_dir(false, splice));
+        get_dir(true, curr) = K{get_dir(true, splice)};
+        get_dir(false, curr) = K{get_dir(false, splice)};
         splice = curr;
         path.fixup();
       }
@@ -742,8 +726,7 @@ namespace snmalloc
 
       auto path = get_root_path();
       while (path.move(true))
-      {
-      }
+      {}
 
       K result = path.curr();
 

diff --git a/src/snmalloc/global/memcpy.h b/src/snmalloc/global/memcpy.h
@@ -192,7 +192,8 @@ namespace snmalloc
      * It's not entirely clear what we would do if this were not the case.
      * Best not think too hard about it now.
      */
-    static_assert(alignof(void*) == sizeof(void*));
+    static_assert(
+      alignof(void*) == sizeof(void*)); // NOLINT(misc-redundant-expression)
 
     static constexpr size_t LargestRegisterSize = 16;