Improvements for logging with formatting (#522)

* Improvements for logging with formatting

* Adding comments and tests

* Apply clang-format

* Ci fixes

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

CMakeLists.txt

@@ -24,6 +24,9 @@ else()
     list(APPEND CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake)
     list(APPEND CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake/test)
 
+    # Useful for testing with address sanitizer:
+    # set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address -g")
+
     if(CHOWDSP_ENABLE_TESTING)
         enable_testing()
         add_subdirectory(tests)

modules/common/chowdsp_data_structures/Allocators/chowdsp_STLArenaAllocator.h

@@ -0,0 +1,52 @@
+#pragma once
+
+namespace chowdsp
+{
+/** A (mostly) STL-conforming allocator backed by a memory arena of your choosing. */
+template <class T, typename ArenaType>
+struct STLArenaAllocator
+{
+    using value_type = T;
+
+    ArenaType& arena;
+
+    explicit STLArenaAllocator (ArenaType& a) noexcept : arena (a)
+    {
+    }
+
+    template <class U>
+    explicit STLArenaAllocator (const STLArenaAllocator<U, ArenaType>& other) noexcept
+        : arena (other.arena)
+    {
+    }
+
+    template <typename U>
+    struct rebind
+    {
+        using other = STLArenaAllocator<U, ArenaType>;
+    };
+
+    T* allocate (std::size_t n)
+    {
+        return static_cast<T*> (arena.allocate_bytes (n, alignof (T)));
+    }
+
+    void deallocate (T*, std::size_t) const
+    {
+        // no-op...
+        // memory will be re-claimed when the arena is cleared.
+    }
+};
+
+template <class T, class U, typename Arena>
+constexpr bool operator== (const STLArenaAllocator<T, Arena>& x, const STLArenaAllocator<U, Arena>& y) noexcept
+{
+    return &x.arena == &y.arena;
+}
+
+template <class T, class U, typename Arena>
+constexpr bool operator!= (const STLArenaAllocator<T, Arena>& x, const STLArenaAllocator<U, Arena>& y) noexcept
+{
+    return ! (x == y);
+}
+} // namespace chowdsp

modules/common/chowdsp_data_structures/chowdsp_data_structures.h

@@ -39,6 +39,7 @@ BEGIN_JUCE_MODULE_DECLARATION
 
 #include "Allocators/chowdsp_ArenaAllocator.h"
 #include "Allocators/chowdsp_ChainedArenaAllocator.h"
+#include "Allocators/chowdsp_STLArenaAllocator.h"
 
 #include "Structures/chowdsp_BucketArray.h"
 #include "Structures/chowdsp_AbstractTree.h"

modules/common/chowdsp_logging/Loggers/chowdsp_BaseLogger.cpp

@@ -0,0 +1,17 @@
+#include "chowdsp_BaseLogger.h"
+
+namespace chowdsp
+{
+BaseLogger* BaseLogger::global_logger = nullptr;
+
+void set_global_logger (BaseLogger* logger)
+{
+    BaseLogger::global_logger = logger;
+    juce::Logger::setCurrentLogger (logger);
+}
+
+BaseLogger* get_global_logger()
+{
+    return BaseLogger::global_logger;
+}
+} // namespace chowdsp

modules/common/chowdsp_logging/Loggers/chowdsp_BaseLogger.h

@@ -6,6 +6,9 @@ struct BaseLogger : juce::Logger
 {
     spdlog::logger internal_logger { "chowdsp_log" };
     spdlog::sink_ptr console_sink {};
+    Broadcaster<void (const juce::String&)> onLogMessage {};
+    ChainedArenaAllocator arena { 8192 };
+    static BaseLogger* global_logger;
 
     BaseLogger()
     {
@@ -20,6 +23,14 @@ struct BaseLogger : juce::Logger
     void logMessage (const juce::String& message) override
     {
         internal_logger.info (message.toStdString());
+        onLogMessage (message);
+        arena.clear();
     }
 };
+
+/** Set's a (static) logger to be used globally. */
+void set_global_logger (BaseLogger*);
+
+/** Returns the global logger, or nullptr if none has been set. */
+BaseLogger* get_global_logger();
 } // namespace chowdsp

modules/common/chowdsp_logging/Loggers/chowdsp_FormatHelpers.h

@@ -0,0 +1,65 @@
+#pragma once
+
+/** Custom formatter for juce::String */
+template <>
+struct fmt::formatter<juce::String> : formatter<std::string>
+{
+    static auto format (const juce::String& str, format_context& ctx) -> decltype (ctx.out())
+    {
+        return fmt::format_to (ctx.out(), "{}", str.toStdString());
+    }
+};
+
+/** Custom formatter for std::span */
+template <typename T>
+struct fmt::formatter<nonstd::span<T>> : formatter<std::string>
+{
+    static auto format (nonstd::span<const T> span, format_context& ctx) -> decltype (ctx.out())
+    {
+        return fmt::format_to (ctx.out(), "{{{}}}", fmt::join (span, ","));
+    }
+};
+
+namespace chowdsp
+{
+/** Implementation of fmt::vformat using a memory arena. */
+template <typename ArenaType>
+std::string_view vformat (ArenaType& arena, fmt::string_view format_str, fmt::format_args args)
+{
+    using FormatAllocator = STLArenaAllocator<char, ArenaType>;
+    FormatAllocator alloc { arena };
+    fmt::basic_memory_buffer<char, 1, FormatAllocator> buffer { alloc };
+    fmt::vformat_to (std::back_inserter (buffer), format_str, args);
+    return { buffer.data(), buffer.size() };
+}
+
+/** Implementation of fmt::format using a memory arena. */
+template <typename ArenaType, typename... Args>
+std::string_view format (ArenaType& arena, fmt::string_view format_str, const Args&... args)
+{
+    return vformat (arena, format_str, fmt::make_format_args (args...));
+}
+
+/** Implementation of fmt::format using the global logger's memory arena. */
+template <typename... Args>
+std::string_view format (fmt::string_view format_str, const Args&... args)
+{
+    auto* global_logger = get_global_logger();
+    if (global_logger == nullptr)
+    {
+        // make sure you've set up the global logger before calling this!
+        jassertfalse;
+        return {};
+    }
+
+    // If this is being called from multiple threads, we might need to synchronize the arena here?
+    return format (global_logger->arena, format_str, std::forward<const Args&> (args)...);
+}
+
+/** A formatted wrapped for juce::Logger::writeToLog(). */
+template <typename... Args>
+void log (fmt::string_view format_str, const Args&... args)
+{
+    juce::Logger::writeToLog (toString (format (format_str, std::forward<const Args&> (args)...)));
+}
+} // namespace chowdsp

modules/common/chowdsp_logging/Loggers/chowdsp_LogFileHelpers.cpp

@@ -79,10 +79,10 @@ namespace LogFileHelpers
     {
         juce::Logger::writeToLog (toString (signal == 0 ? exitString : crashString));
 
-        if (auto* logger = dynamic_cast<BaseLogger*> (juce::Logger::getCurrentLogger()))
+        if (auto* logger = get_global_logger())
             flushLogger (logger);
 
-        juce::Logger::setCurrentLogger (nullptr);
+        set_global_logger (nullptr);
     }
 
     static juce::File getSystemLogFileFolder()

modules/common/chowdsp_logging/Loggers/chowdsp_Logger.cpp

@@ -30,7 +30,7 @@ Logger::Logger (const LogFileParams& loggerParams,
     logger.internal_logger.sinks().push_back (file_sink);
     logger.internal_logger.info ("Starting log file: " + log_file.getFullPathName().toStdString());
 
-    juce::Logger::setCurrentLogger (&logger);
+    set_global_logger (&logger);
 
     juce::SystemStats::setApplicationCrashHandler (signalHandler);
 

modules/common/chowdsp_logging/Loggers/chowdsp_Logger.h

@@ -16,7 +16,7 @@ class Logger : private juce::Timer
 
     const LogFileParams params;
 
-private:
+protected:
     void timerCallback() override;
 
     CrashLogHelpers::CrashLogAnalysisCallback crashLogAnalysisCallback = &CrashLogHelpers::defaultCrashLogAnalyzer;
@@ -26,6 +26,7 @@ class Logger : private juce::Timer
 
     BaseLogger logger;
 
+private:
     JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (Logger)
 };
 } // namespace chowdsp

modules/common/chowdsp_logging/chowdsp_logging.cpp

@@ -1,5 +1,6 @@
 #include "chowdsp_logging.h"
 
+#include "Loggers/chowdsp_BaseLogger.cpp"
 #include "Loggers/chowdsp_LogFileHelpers.cpp"
 #include "Loggers/chowdsp_CrashLogHelpers.cpp"
 #include "Loggers/chowdsp_Logger.cpp"

modules/common/chowdsp_logging/chowdsp_logging.h

@@ -8,7 +8,7 @@ BEGIN_JUCE_MODULE_DECLARATION
    version:       2.2.0
    name:          ChowDSP Logging
    description:   ChowDSP Logging module
-   dependencies:  chowdsp_core, chowdsp_data_structures
+   dependencies:  chowdsp_core, chowdsp_data_structures, chowdsp_listeners
 
    website:       https://ccrma.stanford.edu/~jatin/chowdsp
    license:       BSD 3-Clause
@@ -22,6 +22,7 @@ BEGIN_JUCE_MODULE_DECLARATION
 
 #include <chowdsp_core/chowdsp_core.h>
 #include <chowdsp_data_structures/chowdsp_data_structures.h>
+#include <chowdsp_listeners/chowdsp_listeners.h>
 
 JUCE_BEGIN_IGNORE_WARNINGS_GCC_LIKE ("-Wswitch-enum",
                                      "-Wfloat-equal",
@@ -40,6 +41,7 @@ JUCE_END_IGNORE_WARNINGS_GCC_LIKE
 #include "Loggers/chowdsp_LogFileHelpers.h"
 #include "Loggers/chowdsp_CrashLogHelpers.h"
 #include "Loggers/chowdsp_Logger.h"
+#include "Loggers/chowdsp_FormatHelpers.h"
 
 // legacy
 #include "Loggers/chowdsp_PluginLogger.h"

tests/common_tests/chowdsp_data_structures_test/CMakeLists.txt

@@ -16,6 +16,7 @@ target_sources(chowdsp_data_structures_test
         TupleHelpersTest.cpp
         VectorHelpersTest.cpp
         SmallMapTest.cpp
+        STLArenaAllocatorTest.cpp
 )
 
 target_compile_features(chowdsp_data_structures_test PRIVATE cxx_std_20)

tests/common_tests/chowdsp_data_structures_test/STLArenaAllocatorTest.cpp

@@ -0,0 +1,27 @@
+#include <CatchUtils.h>
+#include <chowdsp_data_structures/chowdsp_data_structures.h>
+
+TEST_CASE ("STL Arena Allocator Test", "[common][data-structures]")
+{
+    using Arena = chowdsp::ArenaAllocator<>;
+    Arena arena { 128 };
+
+    using Alloc = chowdsp::STLArenaAllocator<int, Arena>;
+    Alloc alloc { arena };
+
+    using custom_vector = std::vector<int, Alloc>;
+    custom_vector vec { { 1, 2, 3, 4 }, alloc };
+    REQUIRE (vec.size() == 4);
+    REQUIRE (vec.front() == 1);
+    REQUIRE (vec.back() == 4);
+
+    vec.push_back (5);
+    REQUIRE (vec.size() == 5);
+    REQUIRE (vec.back() == 5);
+
+    vec.erase (vec.begin());
+    REQUIRE (vec.size() == 4);
+    vec.insert (vec.begin(), 0);
+    REQUIRE (vec.size() == 5);
+    REQUIRE (vec.front() == 0);
+}

tests/common_tests/chowdsp_logging_test/CMakeLists.txt

@@ -2,4 +2,5 @@ setup_catch_lib_test(chowdsp_logging_test common_tests_lib)
 
 target_sources(chowdsp_logging_test PRIVATE
     PluginLoggerTest.cpp
+    CustomFormattingTest.cpp
 )

tests/common_tests/chowdsp_logging_test/CustomFormattingTest.cpp

@@ -0,0 +1,21 @@
+#include <chowdsp_logging/chowdsp_logging.h>
+#include <CatchUtils.h>
+
+TEST_CASE ("Custom Formatting Test", "[common][logs]")
+{
+    chowdsp::ArenaAllocator<> arena { 2048 };
+
+    SECTION ("juce::String")
+    {
+        juce::String str { "This is a JUCE string!" };
+        const auto format_result = chowdsp::format (arena, "{}", str);
+        REQUIRE (str == chowdsp::toString (format_result));
+    }
+
+    SECTION ("std::span")
+    {
+        std::vector vec { 0.0f, 1.0f, 2.0f, 3.0f, 4.0f };
+        const auto format_result = chowdsp::format (arena, "{}", nonstd::span { vec });
+        REQUIRE (format_result == "{0,1,2,3,4}");
+    }
+}

tests/common_tests/chowdsp_logging_test/PluginLoggerTest.cpp

@@ -26,6 +26,7 @@ TEMPLATE_TEST_CASE ("Plugin Logger Test", "[common][logs]", chowdsp::PluginLogge
         {
             Logger logger { logFileSubDir, logFileNameRoot };
             juce::Logger::writeToLog (testLogString);
+            chowdsp::log ("The magic number is: {}", 18);
 
             logFile = logger.getLogFile();
         }
@@ -34,6 +35,8 @@ TEMPLATE_TEST_CASE ("Plugin Logger Test", "[common][logs]", chowdsp::PluginLogge
 
         auto logString = logFile.loadFileAsString();
         REQUIRE_MESSAGE (logString.contains (testLogString), "Test log string was not found in the log file!");
+        if (std::is_same_v<Logger, chowdsp::Logger>)
+            REQUIRE_MESSAGE (logString.contains ("The magic number is: 18"), "Test log string was not found in the log file!");
         REQUIRE_MESSAGE (! logString.contains (testNonLogString), "Test non-log string WAS found in the log file!");
     }