Fixes for logging and fuzzy search (#499)

* Fixes for logging and fuzzy search * More work on logger tests * Removing old jasserts * Add comments for arena allocator * Apply clang-format * Fixing SONAR issues --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Chowdhury-DSP · Feb 23, 2024 · b1ac3eb · b1ac3eb
1 parent 7cd4e66
commit b1ac3eb
Show file tree

Hide file tree

Showing 12 changed files with 101 additions and 76 deletions.
diff --git a/modules/common/chowdsp_data_structures/Allocators/chowdsp_ArenaAllocator.h b/modules/common/chowdsp_data_structures/Allocators/chowdsp_ArenaAllocator.h
@@ -42,7 +42,10 @@ class ArenaAllocator
     /** Returns the number of bytes currently being used */
     [[nodiscard]] size_t get_bytes_used() const noexcept { return bytes_used; }
 
-    /** Allocates a given number of bytes */
+    /**
+     * Allocates a given number of bytes.
+     * The returned memory will be un-initialized, so be sure to clear it manually if needed.
+     */
     void* allocate_bytes (size_t num_bytes, size_t alignment = 1) noexcept
     {
         auto* pointer = juce::snapPointerToAlignment (raw_data.data() + bytes_used, alignment);
@@ -55,7 +58,10 @@ class ArenaAllocator
         return pointer;
     }
 
-    /** Allocates space for some number of objects of type T */
+    /**
+     * Allocates space for some number of objects of type T
+     * The returned memory will be un-initialized, so be sure to clear it manually if needed.
+     */
     template <typename T, typename IntType>
     T* allocate (IntType num_Ts, size_t alignment = alignof (T)) noexcept
     {

diff --git a/modules/common/chowdsp_logging/Loggers/chowdsp_CrashLogHelpers.cpp b/modules/common/chowdsp_logging/Loggers/chowdsp_CrashLogHelpers.cpp
@@ -2,6 +2,7 @@
 
 namespace chowdsp::CrashLogHelpers
 {
+// LCOV_EXCL_START
 void defaultCrashLogAnalyzer (const juce::File& logFile)
 {
 #if JUCE_MODULE_AVAILABLE_juce_gui_basics
@@ -34,6 +35,7 @@ void defaultCrashLogAnalyzer (const juce::File& logFile)
     jassertfalse; // Implement your own!
 #endif
 }
+// LCOV_EXCL_END
 
 constexpr std::string_view crashString = "Plugin crashing!!!";
 constexpr std::string_view crashExaminedString = "The crash in this log file is now being examined!";

diff --git a/modules/common/chowdsp_logging/Loggers/chowdsp_LogFileHelpers.cpp b/modules/common/chowdsp_logging/Loggers/chowdsp_LogFileHelpers.cpp
@@ -70,13 +70,36 @@ namespace LogFileHelpers
             {
                 logger->internal_logger.flush();
             }
+            // LCOV_EXCL_START
             catch ([[maybe_unused]] const spdlog::spdlog_ex& ex)
             {
                 jassertfalse;
             }
+            // LCOV_EXCL_END
         }
 
         juce::Logger::setCurrentLogger (nullptr);
     }
+
+    static juce::File getSystemLogFileFolder()
+    {
+#if JUCE_MAC
+        return { "~/Library/Logs" };
+#else
+        return juce::File::getSpecialLocation (juce::File::userApplicationDataDirectory);
+#endif
+    }
+
 } // namespace LogFileHelpers
+
+juce::File LogFileParams::getLogFile (const LogFileParams& params)
+{
+    return LogFileHelpers::getSystemLogFileFolder()
+        .getChildFile (params.logFileSubDir)
+        .getChildFile (params.logFileNameRoot
+                       + juce::Time::getCurrentTime()
+                             .formatted ("%Y-%m-%d_%H-%M-%S"))
+        .withFileExtension (params.logFileExtension)
+        .getNonexistentSibling();
+}
 } // namespace chowdsp
diff --git a/modules/common/chowdsp_logging/Loggers/chowdsp_LogFileHelpers.h b/modules/common/chowdsp_logging/Loggers/chowdsp_LogFileHelpers.h
@@ -11,6 +11,8 @@ struct LogFileParams
     juce::String logFileNameRoot {};
     juce::String logFileExtension = ".log";
     size_t maxNumLogFiles = 50;
+
+    static juce::File getLogFile (const LogFileParams&);
 };
 
 #ifndef DOXYGEN

diff --git a/modules/common/chowdsp_logging/Loggers/chowdsp_Logger.cpp b/modules/common/chowdsp_logging/Loggers/chowdsp_Logger.cpp
@@ -2,21 +2,18 @@
 
 namespace chowdsp
 {
-Logger::Logger (const juce::String& logFileSubDir, const juce::String& logFileNameRoot)
-    : Logger (LogFileParams { logFileSubDir, logFileNameRoot })
+Logger::Logger (const juce::String& logFileSubDir,
+                const juce::String& logFileNameRoot,
+                CrashLogHelpers::CrashLogAnalysisCallback&& callback)
+    : Logger (LogFileParams { logFileSubDir, logFileNameRoot },
+              std::move (callback))
 {
 }
 
-static juce::File getSystemLogFileFolder()
-{
-#if JUCE_MAC
-    return juce::File ("~/Library/Logs");
-#else
-    return juce::File::getSpecialLocation (juce::File::userApplicationDataDirectory);
-#endif
-}
-
-Logger::Logger (const LogFileParams& loggerParams) : params (loggerParams)
+Logger::Logger (const LogFileParams& loggerParams,
+                CrashLogHelpers::CrashLogAnalysisCallback&& callback)
+    : params (loggerParams),
+      crashLogAnalysisCallback (std::move (callback))
 {
     using namespace LogFileHelpers;
     using namespace CrashLogHelpers;
@@ -25,13 +22,7 @@ Logger::Logger (const LogFileParams& loggerParams) : params (loggerParams)
     pruneOldLogFiles (pastLogFiles, params);
     checkLogFilesForCrashes (pastLogFiles, crashLogAnalysisCallback);
 
-    log_file = getSystemLogFileFolder()
-                   .getChildFile (params.logFileSubDir)
-                   .getChildFile (params.logFileNameRoot
-                                  + juce::Time::getCurrentTime()
-                                        .formatted ("%Y-%m-%d_%H-%M-%S"))
-                   .withFileExtension (params.logFileExtension)
-                   .getNonexistentSibling();
+    log_file = LogFileParams::getLogFile (params);
     log_file.create();
 
     file_sink = std::make_shared<spdlog::sinks::basic_file_sink_mt> (log_file.getFullPathName().toStdString(),

diff --git a/modules/common/chowdsp_logging/Loggers/chowdsp_Logger.h b/modules/common/chowdsp_logging/Loggers/chowdsp_Logger.h
@@ -5,16 +5,20 @@ namespace chowdsp
 class Logger
 {
 public:
-    Logger (const juce::String& logFileSubDir, const juce::String& logFileNameRoot);
-    explicit Logger (const LogFileParams& loggerParams);
+    Logger (const juce::String& logFileSubDir,
+            const juce::String& logFileNameRoot,
+            CrashLogHelpers::CrashLogAnalysisCallback&& callback = &CrashLogHelpers::defaultCrashLogAnalyzer);
+    explicit Logger (const LogFileParams& loggerParams,
+                     CrashLogHelpers::CrashLogAnalysisCallback&& callback = &CrashLogHelpers::defaultCrashLogAnalyzer);
     ~Logger();
 
     [[nodiscard]] const juce::File& getLogFile() const { return log_file; }
 
-    LogFileParams params;
-    CrashLogHelpers::CrashLogAnalysisCallback crashLogAnalysisCallback = &CrashLogHelpers::defaultCrashLogAnalyzer;
+    const LogFileParams params;
 
 private:
+    CrashLogHelpers::CrashLogAnalysisCallback crashLogAnalysisCallback = &CrashLogHelpers::defaultCrashLogAnalyzer;
+
     juce::File log_file {};
     spdlog::sink_ptr file_sink {};
 

diff --git a/modules/common/chowdsp_logging/Loggers/chowdsp_PluginLogger.cpp b/modules/common/chowdsp_logging/Loggers/chowdsp_PluginLogger.cpp
@@ -2,13 +2,17 @@
 
 namespace chowdsp
 {
-PluginLogger::PluginLogger (const juce::String& logFileSubDir, const juce::String& logFileNameRoot)
-    : PluginLogger (LogFileParams { logFileSubDir, logFileNameRoot })
+PluginLogger::PluginLogger (const juce::String& logFileSubDir,
+                            const juce::String& logFileNameRoot,
+                            CrashLogHelpers::CrashLogAnalysisCallback&& callback)
+    : PluginLogger (LogFileParams { logFileSubDir, logFileNameRoot },
+                    std::move (callback))
 {
 }
 
-PluginLogger::PluginLogger (LogFileParams loggerParams)
-    : params (std::move (loggerParams))
+PluginLogger::PluginLogger (LogFileParams loggerParams, CrashLogHelpers::CrashLogAnalysisCallback&& callback)
+    : params (std::move (loggerParams)),
+      crashLogAnalysisCallback (std::move (callback))
 {
     using namespace LogFileHelpers;
     using namespace CrashLogHelpers;

diff --git a/modules/common/chowdsp_logging/Loggers/chowdsp_PluginLogger.h b/modules/common/chowdsp_logging/Loggers/chowdsp_PluginLogger.h
@@ -12,16 +12,18 @@ namespace chowdsp
 class PluginLogger
 {
 public:
-    PluginLogger (const juce::String& logFileSubDir, const juce::String& logFileNameRoot);
-    explicit PluginLogger (LogFileParams loggerParams);
+    PluginLogger (const juce::String& logFileSubDir,
+                  const juce::String& logFileNameRoot,
+                  CrashLogHelpers::CrashLogAnalysisCallback&& callback = &CrashLogHelpers::defaultCrashLogAnalyzer);
+    explicit PluginLogger (LogFileParams loggerParams,
+                           CrashLogHelpers::CrashLogAnalysisCallback&& callback = &CrashLogHelpers::defaultCrashLogAnalyzer);
     ~PluginLogger();
 
     [[nodiscard]] const juce::File& getLogFile() const { return fileLogger->getLogFile(); }
 
-    CrashLogHelpers::CrashLogAnalysisCallback crashLogAnalysisCallback = &CrashLogHelpers::defaultCrashLogAnalyzer;
-
 private:
     const LogFileParams params {};
+    const CrashLogHelpers::CrashLogAnalysisCallback crashLogAnalysisCallback;
 
     std::unique_ptr<juce::FileLogger> fileLogger {};
 

diff --git a/modules/common/chowdsp_logging/chowdsp_logging.h b/modules/common/chowdsp_logging/chowdsp_logging.h
@@ -24,7 +24,10 @@ BEGIN_JUCE_MODULE_DECLARATION
 #include <chowdsp_data_structures/chowdsp_data_structures.h>
 
 JUCE_BEGIN_IGNORE_WARNINGS_GCC_LIKE ("-Wswitch-enum",
-                                     "-Wfloat-equal")
+                                     "-Wfloat-equal",
+                                     "-Wextra-semi",
+                                     "-Wc++20-compat",
+                                     "-Wlanguage-extension-token")
 
 #include "third_party/spdlog/include/spdlog/spdlog.h"
 #include "third_party/spdlog/include/spdlog/sinks/stdout_color_sinks.h"

diff --git a/modules/plugin/chowdsp_fuzzy_search/Search/chowdsp_SearchDatabase.h b/modules/plugin/chowdsp_fuzzy_search/Search/chowdsp_SearchDatabase.h
@@ -72,17 +72,6 @@ class SearchDatabase
 
         TempResult() = default;
 
-        TempResult (float _score, int _entryIndex)
-        {
-            set (_score, _entryIndex);
-        }
-
-        void set (float newScore, int newEntryIndex)
-        {
-            score = newScore;
-            entryIndex = newEntryIndex;
-        }
-
         bool operator<(const TempResult& other) const
         {
             return score > other.score; // reversed to sort high score on top
@@ -184,13 +173,13 @@ class SearchDatabase
     {
         if (pass < 1)
         {
-            // set
             for (size_t i = 0; i < tempResults.size(); ++i)
             {
                 const Entry& e = entries[i];
                 int bestIndex = 0;
                 const float currScore = scoreEntry (e, bestIndex, perWordScores);
-                tempResults[i].set (currScore, (int) i); // also sets the index
+                tempResults[i].score = currScore;
+                tempResults[i].entryIndex = static_cast<int> (i);
                 tempResultsOrderPenalty[i].bestIndex = bestIndex;
                 tempResultsOrderPenalty[i].misses = 0;
             }
@@ -363,8 +352,14 @@ class SearchDatabase
 
         // 1. loop over each word in query
         auto perWordScores = nonstd::span { searchArena.allocate<float> (wordStorage.getWordCount()), wordStorage.getWordCount() };
+        std::fill (perWordScores.begin(), perWordScores.end(), 0.0f);
+
         auto tempResults = nonstd::span { searchArena.allocate<TempResult> (entries.size()), entries.size() };
+        std::fill (tempResults.begin(), tempResults.end(), TempResult {});
+
         auto tempResultsOrderPenalty = nonstd::span { searchArena.allocate<TempResultOrderPenalty> (entries.size()), entries.size() };
+        std::fill (tempResultsOrderPenalty.begin(), tempResultsOrderPenalty.end(), TempResultOrderPenalty {});
+
         for (const auto [qi, queryWord] : enumerate (queryWords))
         {
             // 2. score every word against this query-word

diff --git a/modules/plugin/chowdsp_fuzzy_search/Search/chowdsp_SearchHelpers.h b/modules/plugin/chowdsp_fuzzy_search/Search/chowdsp_SearchHelpers.h
@@ -184,6 +184,7 @@ inline nonstd::span<std::string_view> splitString (std::string_view s, ArenaAllo
     if (stringSplitData == nullptr)
         return {};
     auto ss = nonstd::span { stringSplitData, s.size() };
+    std::fill (ss.begin(), ss.end(), std::string_view {});
     size_t ss_index = 0;
 
     size_t left = 0;

diff --git a/tests/common_tests/chowdsp_logging_test/PluginLoggerTest.cpp b/tests/common_tests/chowdsp_logging_test/PluginLoggerTest.cpp
@@ -64,7 +64,6 @@ TEMPLATE_TEST_CASE ("Plugin Logger Test", "[common][logs]", chowdsp::PluginLogge
         logsDirectory.deleteRecursively();
     }
 
-#if ! JUCE_LINUX
     SECTION ("Crash Log Test")
     {
         juce::File logFile;
@@ -77,43 +76,36 @@ TEMPLATE_TEST_CASE ("Plugin Logger Test", "[common][logs]", chowdsp::PluginLogge
             JUCE_END_IGNORE_WARNINGS_GCC_LIKE
         }
 
-        auto logString = logFile.loadFileAsString();
-        REQUIRE_MESSAGE (logString.contains ("Interrupt signal received!"), "Interrupt signal string not found in log!");
-        REQUIRE_MESSAGE (logString.contains ("Stack Trace:"), "Stack trace string not found in log!");
-        REQUIRE_MESSAGE (logString.contains ("Plugin crashing!!!"), "Plugin crashing string not found in log!");
-        REQUIRE_MESSAGE (! logString.contains ("Exiting gracefully"), "Exit string string WAS found in the log file!");
+        const auto testLogFileHasCrash = [] (const juce::File& logFileWithCrash)
+        {
+            auto logString = logFileWithCrash.loadFileAsString();
+            REQUIRE_MESSAGE (logString.contains ("Interrupt signal received!"), "Interrupt signal string not found in log!");
+            REQUIRE_MESSAGE (logString.contains ("Stack Trace:"), "Stack trace string not found in log!");
+            REQUIRE_MESSAGE (logString.contains ("Plugin crashing!!!"), "Plugin crashing string not found in log!");
+            REQUIRE_MESSAGE (! logString.contains ("Exiting gracefully"), "Exit string string WAS found in the log file!");
+        };
+
+        testLogFileHasCrash (logFile);
+
+        int callbacksCount = 0;
+        const auto crashLogCallback = [&callbacksCount, testLogFileHasCrash] (const juce::File& crashLogFile)
+        {
+            callbacksCount++;
+            testLogFileHasCrash (crashLogFile);
+        };
 
         // the first logger we create after a crash should report the crash...
         {
-            auto prevNumTopLevelWindows = juce::TopLevelWindow::getNumTopLevelWindows();
-            Logger logger { logFileSubDir, logFileNameRoot };
-            juce::MessageManager::getInstance()->runDispatchLoopUntil (100);
-
-            auto newNumTopLevelWindows = juce::TopLevelWindow::getNumTopLevelWindows();
-            REQUIRE_MESSAGE (newNumTopLevelWindows == prevNumTopLevelWindows + 1, "AlertWindow not created!");
-
-            // Linux on CI doesn't like trying to open the log file!
-            for (int i = 0; i < newNumTopLevelWindows; ++i)
-            {
-                if (auto* alertWindow = dynamic_cast<juce::AlertWindow*> (juce::TopLevelWindow::getTopLevelWindow (i)))
-                {
-                    alertWindow->triggerButtonClick ("Show Log File");
-                    juce::MessageManager::getInstance()->runDispatchLoopUntil (100);
-                    break;
-                }
-            }
+            Logger logger { logFileSubDir, logFileNameRoot, crashLogCallback };
+            REQUIRE (callbacksCount == 1);
         }
 
         // the next logger should not!
         {
-            auto prevNumTopLevelWindows = juce::TopLevelWindow::getNumTopLevelWindows();
-            Logger logger { logFileSubDir, logFileNameRoot };
-
-            auto newNumTopLevelWindows = juce::TopLevelWindow::getNumTopLevelWindows();
-            REQUIRE_MESSAGE (newNumTopLevelWindows == prevNumTopLevelWindows, "AlertWindow was incorrectly created!");
+            Logger logger { logFileSubDir, logFileNameRoot, crashLogCallback };
+            REQUIRE (callbacksCount == 1);
         }
     }
-#endif
 
     // clean up after ourselves...
     auto logsDirectory = juce::FileLogger::getSystemLogFileFolder().getChildFile (logFileSubDir);