[ds_core] simplify MessageBuilder construction (#713)

* [ds_core] simplify MessageBuilder construction * remove special handling of zero case * select latest xcode
microsoft · Dec 18, 2024 · 141cdd9 · 141cdd9
1 parent 6952683
commit 141cdd9
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 45 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -130,7 +130,10 @@ jobs:
             # 2to3 exists.  As a quick hack, delete it first.  This should be
             # removed once the homebrew install is fixed.
       run: "rm -f /usr/local/bin/2to3 ; brew update && brew install ninja"
-
+    - name: Pick the latest XCode in runner
+      uses: maxim-lobanov/setup-xcode@v1
+      with:
+        xcode-version: latest-stable
     - name: Configure CMake
       run: cmake -B ${{github.workspace}}/build -DCMAKE_BUILD_TYPE=${{matrix.build-type}} -G Ninja ${{ matrix.cmake-flags }} ${{ matrix.extra-cmake-flags }}
       # Build with a nice ninja status line

diff --git a/src/snmalloc/ds_core/helpers.h b/src/snmalloc/ds_core/helpers.h
@@ -1,11 +1,10 @@
 #pragma once
 
 #include "bits.h"
+#include "snmalloc/ds_core/defines.h"
 
 #include <array>
-#include <atomic>
 #include <cstddef>
-#include <tuple>
 #include <type_traits>
 
 namespace snmalloc
@@ -156,26 +155,6 @@ namespace snmalloc
      */
     size_t insert = 0;
 
-    /**
-     * Add argument `i` from the tuple `args` to the output.  This is
-     * implemented recursively because the different tuple elements can have
-     * different types and so the code for dispatching will depend on the type
-     * at the index.  The compiler will lower this to a jump table in optimised
-     * builds.
-     */
-    template<size_t I, typename... Args>
-    void add_tuple_arg(size_t i, const std::tuple<Args...>& args)
-    {
-      if (i == I)
-      {
-        append(std::get<I>(args));
-      }
-      else if constexpr (I != 0)
-      {
-        add_tuple_arg<I - 1>(i, args);
-      }
-    }
-
     /**
      * Append a single character into the buffer.  This is the single primitive
      * operation permitted on the buffer and performs bounds checks to ensure
@@ -361,43 +340,41 @@ namespace snmalloc
       append(static_cast<unsigned long long>(x));
     }
 
-  public:
     /**
-     * Constructor.  Takes a format string and the arguments to output.
+     * Append with format string and arguments. Compiler
+     * is able to optimize the recursion into loops.
      */
-    template<typename... Args>
-    SNMALLOC_FAST_PATH MessageBuilder(const char* fmt, Args... args)
+    template<typename Head, typename... Tail>
+    SNMALLOC_FAST_PATH_INLINE void
+    append(const char* fmt, Head&& head, Tail&&... tail)
     {
-      buffer[SafeLength] = 0;
-      size_t arg = 0;
-      auto args_tuple = std::forward_as_tuple(args...);
-      for (const char* s = fmt; *s != 0; ++s)
+      for (;;)
       {
-        if (s[0] == '{' && s[1] == '}')
+        if (fmt[0] == '\0')
         {
-          add_tuple_arg<sizeof...(Args) - 1>(arg++, args_tuple);
-          ++s;
+          error("Internal error: format string missing `{}`!");
         }
-        else
+
+        if (fmt[0] == '{' && fmt[1] == '}')
         {
-          append_char(*s);
+          append(std::forward<Head>(head));
+          return append(fmt + 2, std::forward<Tail>(tail)...);
         }
+
+        append_char(*fmt);
+        fmt++;
       }
-      append_char('\0');
     }
 
+  public:
     /**
-     * Constructor for trivial format strings (no arguments).  This exists to
-     * allow `MessageBuilder` to be used with macros without special casing
-     * the single-argument version.
+     * Constructor.  Takes a format string and the arguments to output.
      */
-    SNMALLOC_FAST_PATH MessageBuilder(const char* fmt)
+    template<typename... Args>
+    SNMALLOC_FAST_PATH MessageBuilder(const char* fmt, Args&&... args)
     {
       buffer[SafeLength] = 0;
-      for (const char* s = fmt; *s != 0; ++s)
-      {
-        append_char(*s);
-      }
+      append(fmt, std::forward<Args>(args)...);
       append_char('\0');
     }