From e2b759b19df7ff34262ac40f4a8734854b61eb75 Mon Sep 17 00:00:00 2001 From: odygrd Date: Sun, 16 Feb 2025 03:29:14 +0000 Subject: [PATCH 1/2] minor DeferredFormatCodec, DirectFormatCodec improvements --- include/quill/DeferredFormatCodec.h | 53 +++++++++++++++++++++++++---- include/quill/DirectFormatCodec.h | 22 +++++++----- 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/include/quill/DeferredFormatCodec.h b/include/quill/DeferredFormatCodec.h index 5da2ff3e..a02eed4b 100644 --- a/include/quill/DeferredFormatCodec.h +++ b/include/quill/DeferredFormatCodec.h @@ -11,8 +11,12 @@ #include "quill/core/DynamicFormatArgStore.h" #include "quill/core/InlinedVector.h" +#include +#include #include +#include #include +#include QUILL_BEGIN_NAMESPACE @@ -81,8 +85,6 @@ QUILL_BEGIN_NAMESPACE template struct DeferredFormatCodec { - // Checks like this Will fail for types with private ctors and friends declarations - // static_assert(std::is_copy_constructible_v, "T must be copy constructible"); static constexpr bool is_trivially_copyable = std::is_trivially_copyable_v; static size_t compute_encoded_size(detail::SizeCacheVector&, T const&) noexcept @@ -93,11 +95,12 @@ struct DeferredFormatCodec } else { - return sizeof(T) + alignof(T); + // If it’s misaligned, the worst-case scenario is when the pointer is off by one byte from an alignment boundary + return sizeof(T) + alignof(T) - 1; } } - static void encode(std::byte*& buffer, detail::SizeCacheVector const&, uint32_t&, T const& arg) noexcept + static void encode(std::byte*& buffer, detail::SizeCacheVector const&, uint32_t&, T const& arg) { if constexpr (is_trivially_copyable) { @@ -108,7 +111,7 @@ struct DeferredFormatCodec { auto aligned_ptr = align_pointer(buffer, alignof(T)); new (static_cast(aligned_ptr)) T(arg); - buffer += sizeof(T) + alignof(T); + buffer += sizeof(T) + alignof(T) - 1; } } @@ -116,6 +119,7 @@ struct DeferredFormatCodec { if constexpr (is_trivially_copyable) { + static_assert(is_default_constructible::value, "T is not default-constructible!"); T arg; // Cast to void* to silence compiler warning about private members @@ -130,6 +134,7 @@ struct DeferredFormatCodec auto* tmp = reinterpret_cast(aligned_ptr); // Take a copy + static_assert(is_copy_constructible::value, "T is not copy-constructible!"); T arg{*tmp}; if constexpr (!std::is_trivially_destructible_v) @@ -138,17 +143,53 @@ struct DeferredFormatCodec tmp->~T(); } - buffer += sizeof(T) + alignof(T); + buffer += sizeof(T) + alignof(T) - 1; return arg; } } static void decode_and_store_arg(std::byte*& buffer, DynamicFormatArgStore* args_store) { + static_assert(is_move_constructible::value, "T is not move-constructible!"); args_store->push_back(decode_arg(buffer)); } private: + // These trait implementations will take the friend declaration into account + + // Default constructible check + template + struct is_default_constructible : std::false_type + { + }; + + template + struct is_default_constructible> : std::true_type + { + }; + + // Copy constructible check: tests if we can call U(const U&) + template + struct is_copy_constructible : std::false_type + { + }; + + template + struct is_copy_constructible()))>> : std::true_type + { + }; + + // Move constructible check: tests if we can call U(U&&) + template + struct is_move_constructible : std::false_type + { + }; + + template + struct is_move_constructible()))>> : std::true_type + { + }; + static std::byte* align_pointer(void* pointer, size_t alignment) noexcept { return reinterpret_cast((reinterpret_cast(pointer) + (alignment - 1ul)) & diff --git a/include/quill/DirectFormatCodec.h b/include/quill/DirectFormatCodec.h index 8f8963e1..eb623635 100644 --- a/include/quill/DirectFormatCodec.h +++ b/include/quill/DirectFormatCodec.h @@ -12,7 +12,10 @@ #include "quill/core/DynamicFormatArgStore.h" #include "quill/core/InlinedVector.h" +#include +#include #include +#include QUILL_BEGIN_NAMESPACE @@ -83,20 +86,21 @@ QUILL_BEGIN_NAMESPACE template struct DirectFormatCodec { - static size_t compute_encoded_size(quill::detail::SizeCacheVector& conditional_arg_size_cache, T const& arg) noexcept + static size_t compute_encoded_size(quill::detail::SizeCacheVector& conditional_arg_size_cache, T const& arg) { - return sizeof(uint32_t) + - conditional_arg_size_cache.push_back(static_cast(fmtquill::formatted_size("{}", arg))); + // The computed size includes the size of the length field + return sizeof(uint32_t) + + conditional_arg_size_cache.push_back(static_cast(fmtquill::formatted_size("{}", arg))); } static void encode(std::byte*& buffer, quill::detail::SizeCacheVector const& conditional_arg_size_cache, - uint32_t& conditional_arg_size_cache_index, T const& arg) noexcept + uint32_t& conditional_arg_size_cache_index, T const& arg) { - uint32_t const len = conditional_arg_size_cache[conditional_arg_size_cache_index++]; - std::memcpy(buffer, &len, sizeof(len)); - buffer += sizeof(len); - fmtquill::format_to_n(reinterpret_cast(buffer), len, "{}", arg); - buffer += len; + uint32_t const len = conditional_arg_size_cache[conditional_arg_size_cache_index++]; + std::memcpy(buffer, &len, sizeof(len)); + buffer += sizeof(len); + fmtquill::format_to_n(reinterpret_cast(buffer), len, "{}", arg); + buffer += len; } static std::string_view decode_arg(std::byte*& buffer) From feb454d9e03ee1525858aa061547aa1a50a473d0 Mon Sep 17 00:00:00 2001 From: odygrd Date: Sun, 16 Feb 2025 04:03:17 +0000 Subject: [PATCH 2/2] minor DeferredFormatCodec, DirectFormatCodec improvements --- include/quill/DeferredFormatCodec.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/quill/DeferredFormatCodec.h b/include/quill/DeferredFormatCodec.h index a02eed4b..f91e72c3 100644 --- a/include/quill/DeferredFormatCodec.h +++ b/include/quill/DeferredFormatCodec.h @@ -131,7 +131,7 @@ struct DeferredFormatCodec else { auto aligned_ptr = align_pointer(buffer, alignof(T)); - auto* tmp = reinterpret_cast(aligned_ptr); + auto* tmp = std::launder(reinterpret_cast(aligned_ptr)); // Take a copy static_assert(is_copy_constructible::value, "T is not copy-constructible!");