cosmetic changes from static analysis

benchmarks/hot_path_latency/hot_path_bench.h

@@ -18,7 +18,11 @@
 #include <random>
 #include <thread>
 
-#include <x86intrin.h>
+#if defined(_WIN32)
+  #include <intrin.h>
+#else
+  #include <x86intrin.h>
+#endif
 
 // Instead of sleep
 inline void wait(std::chrono::nanoseconds min, std::chrono::nanoseconds max)

examples/example_trivial.cpp

@@ -56,7 +56,7 @@ int main()
              char_arrays.mid_0);
 
     // Using a dynamic runtime log level
-    std::array<quill::LogLevel, 4> const runtime_log_levels = {
+    std::array<quill::LogLevel, 4> constexpr runtime_log_levels = {
       quill::LogLevel::Debug, quill::LogLevel::Info, quill::LogLevel::Warning, quill::LogLevel::Error};
 
     for (auto const& log_level : runtime_log_levels)
@@ -123,7 +123,7 @@ int main()
   LOG_CRITICAL(logger_1, "This is never logged");
 
   // Get all created loggers
-  std::unordered_map<std::string, quill::Logger*> created_loggers = quill::get_all_loggers();
+  std::unordered_map<std::string, quill::Logger*> const created_loggers = quill::get_all_loggers();
   std::vector<std::string> logger_names;
   for (auto const& elem : created_loggers)
   {

quill/include/quill/Clock.h

@@ -3,6 +3,8 @@
  * Distributed under the MIT License (http://opensource.org/licenses/MIT)
  */
 
+#pragma once
+
 #include "quill/detail/LogManager.h"
 #include "quill/detail/misc/Attributes.h"
 #include "quill/detail/misc/Rdtsc.h"

quill/include/quill/Logger.h

@@ -231,7 +231,7 @@ class alignas(detail::CACHE_LINE_ALIGNED) Logger
     // look up their types to deserialize them
 
     // Note: The metadata variable here is created during program init time,
-    std::byte* const write_begin = write_buffer;
+    std::byte const* const write_begin = write_buffer;
     write_buffer = detail::align_pointer<alignof(detail::Header), std::byte>(write_buffer);
 
     constexpr bool is_printf_format = macro_metadata.is_printf_format();

quill/include/quill/Quill.h

@@ -34,7 +34,7 @@ namespace quill
 /** Version Info **/
 constexpr uint32_t VersionMajor{3};
 constexpr uint32_t VersionMinor{4};
-constexpr uint32_t VersionPatch{1};
+constexpr uint32_t VersionPatch{2};
 constexpr uint32_t Version{VersionMajor * 10000 + VersionMinor * 100 + VersionPatch};
 
 /** forward declarations **/

quill/include/quill/detail/HandlerCollection.h

@@ -78,10 +78,9 @@ class HandlerCollection
       {
         return handler;
       }
-      else
-      {
+
         // recreate this handler
-        if constexpr (std::is_base_of<StreamHandler, THandler>::value)
+        if constexpr (std::is_base_of_v<StreamHandler, THandler>)
         {
           handler = std::make_shared<THandler>(handler_name, std::forward<Args>(args)...);
         }
@@ -92,12 +91,12 @@ class HandlerCollection
 
         search->second = handler;
         return handler;
-      }
+
     }
 
     // if first time add it
     std::shared_ptr<Handler> handler;
-    if constexpr (std::is_base_of<StreamHandler, THandler>::value)
+    if constexpr (std::is_base_of_v<StreamHandler, THandler>)
     {
       handler = std::make_shared<THandler>(handler_name, std::forward<Args>(args)...);
     }
@@ -116,7 +115,7 @@ class HandlerCollection
    * @throws std::runtime_error if the handler does not exist
    * @return a shared_ptr to the handler
    */
-  QUILL_NODISCARD QUILL_ATTRIBUTE_COLD std::shared_ptr<Handler> get_handler(std::string const& handler_name);
+  QUILL_NODISCARD QUILL_ATTRIBUTE_COLD std::shared_ptr<Handler> get_handler(std::string const& handler_name) const;
 
   /**
    * Subscribe a handler to the vector of active handlers so that the backend thread can see it

quill/include/quill/detail/LogManager.h

@@ -103,7 +103,7 @@ class LogManager
                                         std::optional<TimestampClock*> timestamp_clock)
   {
     return _logger_collection.create_logger(
-      logger_name, handlers,
+      logger_name, std::move(handlers),
       timestamp_clock_type.has_value() ? *timestamp_clock_type : _config.default_timestamp_clock_type,
       timestamp_clock.has_value() ? *timestamp_clock : _config.default_custom_timestamp_clock);
   }
@@ -138,7 +138,7 @@ class LogManager
     // get the root logger - this is needed for the logger_details struct, in order to figure out
     // the clock type later on the backend thread
     Logger* default_logger = logger_collection().get_logger(nullptr);
-    LoggerDetails* logger_details = std::addressof(default_logger->_logger_details);
+    LoggerDetails const* logger_details = std::addressof(default_logger->_logger_details);
 
     // Create an atomic variable
     std::atomic<bool> backend_thread_flushed{false};
@@ -155,11 +155,11 @@ class LogManager
 
     detail::ThreadContext* const thread_context =
       _thread_context_collection.local_thread_context<QUILL_QUEUE_TYPE>();
-    size_t const total_size = sizeof(detail::Header) + sizeof(uintptr_t);
+    size_t constexpr total_size = sizeof(detail::Header) + sizeof(uintptr_t);
 
     std::byte* write_buffer =
       thread_context->spsc_queue<QUILL_QUEUE_TYPE>().prepare_write(static_cast<uint32_t>(total_size));
-    std::byte* const write_begin = write_buffer;
+    std::byte const* const write_begin = write_buffer;
 
     write_buffer = detail::align_pointer<alignof(detail::Header), std::byte>(write_buffer);
 
@@ -195,7 +195,7 @@ class LogManager
    * Starts the backend worker thread.
    * This should only be called by the LogManagerSingleton and never directly from here
    */
-  QUILL_ATTRIBUTE_COLD void inline start_backend_worker(bool with_signal_handler,
+  QUILL_ATTRIBUTE_COLD void start_backend_worker(bool with_signal_handler,
                                                         std::initializer_list<int> const& catchable_signals)
   {
     if (with_signal_handler)
@@ -247,7 +247,7 @@ class LogManager
   /**
    * @return true if backend worker has started
    */
-  QUILL_NODISCARD QUILL_ATTRIBUTE_COLD bool backend_worker_is_running() noexcept
+  QUILL_NODISCARD QUILL_ATTRIBUTE_COLD bool backend_worker_is_running() const noexcept
   {
     return _backend_worker.is_running();
   }
@@ -300,7 +300,7 @@ class LogManagerSingleton
    */
   detail::LogManager& log_manager() noexcept { return _log_manager; }
 
-  QUILL_ATTRIBUTE_COLD void inline start_backend_worker(bool with_signal_handler,
+  QUILL_ATTRIBUTE_COLD void start_backend_worker(bool with_signal_handler,
                                                         std::initializer_list<int> const& catchable_signals)
   {
     // protect init to be called only once

quill/include/quill/detail/LoggerCollection.h

@@ -122,7 +122,7 @@ class LoggerCollection
    * This is meant to called before quill:start() and that is checked internally before calling
    * this function.
    */
-  QUILL_ATTRIBUTE_COLD void enable_console_colours() noexcept;
+  QUILL_ATTRIBUTE_COLD void enable_console_colours() const noexcept;
 
   /**
    * Get the root logger pointer

quill/include/quill/detail/Serialize.h

@@ -7,7 +7,7 @@
 
 #ifndef __STDC_WANT_LIB_EXT1__
   #define __STDC_WANT_LIB_EXT1__ 1
-  #include <string.h>
+  #include <cstring>
 #endif
 
 #include "misc/Utilities.h"
@@ -42,9 +42,9 @@ class LoggerDetails;
 template <typename Arg>
 QUILL_NODISCARD constexpr bool is_type_of_c_array()
 {
-  using ArgType = detail::remove_cvref_t<Arg>;
-  return std::is_array<ArgType>::value &&
-    std::is_same<detail::remove_cvref_t<typename std::remove_extent<ArgType>::type>, char>::value;
+  using ArgType = remove_cvref_t<Arg>;
+  return std::is_array_v<ArgType> &&
+    std::is_same_v<remove_cvref_t<std::remove_extent_t<ArgType>>, char>;
 }
 
 template <typename Arg>
@@ -78,7 +78,7 @@ QUILL_NODISCARD constexpr bool is_type_of_wide_string()
 #endif
 
 template <typename Arg>
-QUILL_NODISCARD inline constexpr bool need_call_dtor_for()
+QUILL_NODISCARD constexpr bool need_call_dtor_for()
 {
   using ArgType = detail::remove_cvref_t<Arg>;
 
@@ -94,18 +94,18 @@ QUILL_NODISCARD inline constexpr bool need_call_dtor_for()
     return false;
   }
 
-  return !std::is_trivially_destructible<ArgType>::value;
+  return !std::is_trivially_destructible_v<ArgType>;
 }
 
 template <typename TFormatContext, size_t DestructIdx>
-QUILL_NODISCARD QUILL_ATTRIBUTE_HOT inline std::byte* decode_args(
+QUILL_NODISCARD QUILL_ATTRIBUTE_HOT std::byte* decode_args(
   std::byte* in, std::vector<fmtquill::basic_format_arg<TFormatContext>>&, std::byte**)
 {
   return in;
 }
 
 template <typename TFormatContext, size_t DestructIdx, typename Arg, typename... Args>
-QUILL_NODISCARD QUILL_ATTRIBUTE_HOT inline std::byte* decode_args(
+QUILL_NODISCARD QUILL_ATTRIBUTE_HOT std::byte* decode_args(
   std::byte* in, std::vector<fmtquill::basic_format_arg<TFormatContext>>& args, std::byte** destruct_args)
 {
   using ArgType = detail::remove_cvref_t<Arg>;
@@ -168,12 +168,12 @@ QUILL_NODISCARD QUILL_ATTRIBUTE_HOT inline std::byte* decode_args(
 }
 
 template <size_t DestructIdx>
-QUILL_ATTRIBUTE_HOT inline void destruct_args(std::byte**)
+QUILL_ATTRIBUTE_HOT void destruct_args(std::byte**)
 {
 }
 
 template <size_t DestructIdx, typename Arg, typename... Args>
-QUILL_ATTRIBUTE_HOT inline void destruct_args(std::byte** args)
+QUILL_ATTRIBUTE_HOT void destruct_args(std::byte** args)
 {
   using ArgType = detail::remove_cvref_t<Arg>;
   if constexpr (need_call_dtor_for<Arg>())
@@ -364,7 +364,7 @@ QUILL_NODISCARD QUILL_ATTRIBUTE_HOT std::pair<std::byte*, std::string> format_to
 {
   std::string error;
   constexpr size_t num_dtors = fmtquill::detail::count<need_call_dtor_for<Args>()...>();
-  std::byte* dtor_args[(std::max)(num_dtors, (size_t)1)];
+  std::byte* dtor_args[(std::max)(num_dtors, static_cast<size_t>(1))];
 
   args.clear();
   std::byte* ret = decode_args<fmtquill::format_context, 0, Args...>(data, args, dtor_args);
@@ -397,7 +397,7 @@ QUILL_NODISCARD QUILL_ATTRIBUTE_HOT std::pair<std::byte*, std::string> printf_fo
 {
   std::string error;
   constexpr size_t num_dtors = fmtquill::detail::count<need_call_dtor_for<Args>()...>();
-  std::byte* dtor_args[(std::max)(num_dtors, (size_t)1)];
+  std::byte* dtor_args[(std::max)(num_dtors, static_cast<size_t>(1))];
 
   args.clear();
   std::byte* ret = decode_args<fmtquill::printf_context, 0, Args...>(data, args, dtor_args);
@@ -455,7 +455,7 @@ struct Header
 public:
   Header() = default;
   Header(MetadataFormatFn metadata_and_format_fn, detail::LoggerDetails const* logger_details, uint64_t timestamp)
-    : metadata_and_format_fn(metadata_and_format_fn), logger_details(logger_details), timestamp(timestamp){};
+    : metadata_and_format_fn(metadata_and_format_fn), logger_details(logger_details), timestamp(timestamp){}
 
   MetadataFormatFn metadata_and_format_fn{nullptr};
   detail::LoggerDetails const* logger_details{nullptr};

quill/include/quill/detail/ThreadContext.h

@@ -28,7 +28,7 @@ namespace quill::detail
  * The backend thread reads all existing ThreadContext class instances and pop the events
  * from each thread queue
  */
-class ThreadContext
+class alignas(CACHE_LINE_ALIGNED) ThreadContext
 {
 public:
   /**
@@ -55,22 +55,6 @@ class ThreadContext
   ThreadContext(ThreadContext const&) = delete;
   ThreadContext& operator=(ThreadContext const&) = delete;
 
-  /**
-   * Operator new to align this object to a cache line boundary as we always create it on the heap
-   * This object should always be aligned to a cache line as it contains the SPSC queue as a member
-   * which has cache line alignment requirements
-   * @param i size of object
-   * @return a pointer to the allocated object
-   */
-  void* operator new(size_t i) { return alloc_aligned(i, CACHE_LINE_ALIGNED); }
-
-  /**
-   * Operator delete
-   * @see operator new
-   * @param p pointer to object
-   */
-  void operator delete(void* p) { free_aligned(p); }
-
   /**
    * @return A reference to the backend's thread transit event buffer
    */

quill/include/quill/detail/ThreadContextCollection.h

@@ -54,18 +54,9 @@ class ThreadContextCollection
     ThreadContextWrapper(ThreadContextCollection& thread_context_collection, uint32_t default_queue_capacity,
                          uint32_t initial_transit_event_buffer_capacity, bool huge_pages)
       : _thread_context_collection(thread_context_collection),
-        _thread_context(std::shared_ptr<ThreadContext>(new ThreadContext(
-          queue_type, default_queue_capacity, initial_transit_event_buffer_capacity, huge_pages)))
+        _thread_context(std::make_shared<ThreadContext>(
+          queue_type, default_queue_capacity, initial_transit_event_buffer_capacity, huge_pages))
     {
-      // We can not use std::make_shared above.
-      // Explanation :
-      // ThreadContext has the SPSC queue as a class member which requires a 64 cache byte alignment,
-      // since we are creating this object on the heap this is not guaranteed.
-      // Visual Studio is the only compiler that gives a warning that ThreadContext might not be aligned to 64 bytes,
-      // as it is allocated on the heap, and we should use aligned alloc instead.
-      // The solution to solve this would be to define a custom operator new and operator delete for ThreadContext.
-      // However, when using std::make_shared, the default allocator is used.
-      // This is a problem if the class is supposed to use a non-default allocator like ThreadContext
       _thread_context_collection.register_thread_context(_thread_context);
     }
 
@@ -143,7 +134,7 @@ class ThreadContextCollection
   template <QueueType queue_type>
   QUILL_NODISCARD_ALWAYS_INLINE_HOT ThreadContext* local_thread_context() noexcept
   {
-    static thread_local ThreadContextWrapper<queue_type> thread_context_wrapper{
+    thread_local ThreadContextWrapper<queue_type> thread_context_wrapper{
       *this, _config.default_queue_capacity,
       _config.backend_thread_use_transit_buffer ? _config.backend_thread_initial_transit_event_buffer_capacity : 1,
       _config.enable_huge_pages_hot_path};
@@ -277,7 +268,7 @@ class ThreadContextCollection
   {
     std::lock_guard<std::mutex> const lock(_mutex);
 
-    auto thread_context_it = std::find_if(_thread_contexts.begin(), _thread_contexts.end(),
+    auto const thread_context_it = std::find_if(_thread_contexts.begin(), _thread_contexts.end(),
                                           [thread_context](std::shared_ptr<ThreadContext> const& elem)
                                           { return elem.get() == thread_context; });