From 30ee6904011cdfc57c7f7b72997aed9cb15a74df Mon Sep 17 00:00:00 2001
From: Joe Burzinski <tridacnid@gmail.com>
Date: Sun, 31 May 2020 13:15:40 -0500
Subject: [PATCH 1/3] Add support for FMT_STRING compile time checking. Add
 negative compilation unit test for compilers and c++ standard that support
 relaxed constexpr.

---
 CMakeLists.txt                      |   1 +
 include/spdlog/logger.h             | 184 ++++++++++++----------------
 include/spdlog/spdlog.h             | 131 ++++++--------------
 tests/CMakeLists.txt                |  26 ++++
 tests/test_compilation_failures.cpp |  14 +++
 tests/test_misc.cpp                 |  35 +++++-
 6 files changed, 193 insertions(+), 198 deletions(-)
 create mode 100644 tests/test_compilation_failures.cpp

diff --git a/CMakeLists.txt b/CMakeLists.txt
index f8941df3..7b792cab 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -71,6 +71,7 @@ option(SPDLOG_BUILD_EXAMPLE_HO "Build header only example" OFF)
 # testing options
 option(SPDLOG_BUILD_TESTS "Build tests" OFF)
 option(SPDLOG_BUILD_TESTS_HO "Build tests using the header only version" OFF)
+option(SPDLOG_BUILD_FAILING_TESTS "Add unit test targets that test for compilation failures" OFF)
 
 # bench options
 option(SPDLOG_BUILD_BENCH "Build benchmarks (Requires https://github.com/google/benchmark.git to be installed)" OFF)
diff --git a/include/spdlog/logger.h b/include/spdlog/logger.h
index 67f808ba..ed44f526 100644
--- a/include/spdlog/logger.h
+++ b/include/spdlog/logger.h
@@ -73,78 +73,74 @@ public:
 
     void swap(spdlog::logger &other) SPDLOG_NOEXCEPT;
 
-    template<typename... Args>
-    void log(source_loc loc, level::level_enum lvl, string_view_t fmt, const Args &... args)
+    // FormatString is a type derived from fmt::compile_string
+    template<typename FormatString, typename std::enable_if<std::is_base_of<fmt::compile_string, FormatString>::value, int>::type = 0,
+        typename... Args>
+    void log(source_loc loc, level::level_enum lvl, FormatString &&fmt, Args &&... args)
     {
-        bool log_enabled = should_log(lvl);
-        bool traceback_enabled = tracer_.enabled();
-        if (!log_enabled && !traceback_enabled)
-        {
-            return;
-        }
-        SPDLOG_TRY
-        {
-            memory_buf_t buf;
-            fmt::format_to(buf, fmt, args...);
-            details::log_msg log_msg(loc, name_, lvl, string_view_t(buf.data(), buf.size()));
-            log_it_(log_msg, log_enabled, traceback_enabled);
-        }
-        SPDLOG_LOGGER_CATCH()
+        log_(loc, lvl, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
     }
 
+    // FormatString is NOT a type derived from fmt::compile_string but is a string_view_t or can be implicitly converted to one
     template<typename... Args>
-    void log(level::level_enum lvl, string_view_t fmt, const Args &... args)
+    void log(source_loc loc, level::level_enum lvl, string_view_t fmt, Args &&... args)
     {
-        log(source_loc{}, lvl, fmt, args...);
+        log_(loc, lvl, fmt, std::forward<Args>(args)...);
     }
 
-    template<typename... Args>
-    void trace(string_view_t fmt, const Args &... args)
+    template<typename FormatString, typename... Args>
+    void log(level::level_enum lvl, FormatString &&fmt, Args &&... args)
     {
-        log(level::trace, fmt, args...);
+        log(source_loc{}, lvl, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
     }
 
-    template<typename... Args>
-    void debug(string_view_t fmt, const Args &... args)
+    template<typename FormatString, typename... Args>
+    void trace(FormatString &&fmt, Args &&... args)
     {
-        log(level::debug, fmt, args...);
+        log(level::trace, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
     }
 
-    template<typename... Args>
-    void info(string_view_t fmt, const Args &... args)
+    template<typename FormatString, typename... Args>
+    void debug(FormatString &&fmt, Args &&... args)
     {
-        log(level::info, fmt, args...);
+        log(level::debug, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
     }
 
-    template<typename... Args>
-    void warn(string_view_t fmt, const Args &... args)
+    template<typename FormatString, typename... Args>
+    void info(FormatString &&fmt, Args &&... args)
     {
-        log(level::warn, fmt, args...);
+        log(level::info, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
     }
 
-    template<typename... Args>
-    void error(string_view_t fmt, const Args &... args)
+    template<typename FormatString, typename... Args>
+    void warn(FormatString &&fmt, Args &&... args)
     {
-        log(level::err, fmt, args...);
+        log(level::warn, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
     }
 
-    template<typename... Args>
-    void critical(string_view_t fmt, const Args &... args)
+    template<typename FormatString, typename... Args>
+    void error(FormatString &&fmt, Args &&... args)
     {
-        log(level::critical, fmt, args...);
+        log(level::err, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
+    }
+
+    template<typename FormatString, typename... Args>
+    void critical(FormatString &&fmt, Args &&... args)
+    {
+        log(level::critical, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
     }
 
     template<typename T>
-    void log(level::level_enum lvl, const T &msg)
+    void log(level::level_enum lvl, T &&msg)
     {
-        log(source_loc{}, lvl, msg);
+        log(source_loc{}, lvl, std::forward<T>(msg));
     }
 
     // T can be statically converted to string_view
-    template<class T, typename std::enable_if<std::is_convertible<const T &, spdlog::string_view_t>::value, T>::type * = nullptr>
-    void log(source_loc loc, level::level_enum lvl, const T &msg)
+    template<class T, typename std::enable_if<std::is_convertible<T &&, spdlog::string_view_t>::value, T>::type * = nullptr>
+    void log(source_loc loc, level::level_enum lvl, T &&msg)
     {
-        log(loc, lvl, string_view_t{msg});
+        log(loc, lvl, string_view_t{std::forward<T>(msg)});
     }
 
     void log(log_clock::time_point log_time, source_loc loc, level::level_enum lvl, string_view_t msg)
@@ -179,48 +175,48 @@ public:
     }
 
     // T cannot be statically converted to string_view or wstring_view
-    template<class T, typename std::enable_if<!std::is_convertible<const T &, spdlog::string_view_t>::value &&
-                                                  !is_convertible_to_wstring_view<const T &>::value,
-                          T>::type * = nullptr>
-    void log(source_loc loc, level::level_enum lvl, const T &msg)
+    template<class T,
+        typename std::enable_if<!std::is_convertible<T &&, spdlog::string_view_t>::value && !is_convertible_to_wstring_view<T &&>::value,
+            T>::type * = nullptr>
+    void log(source_loc loc, level::level_enum lvl, T &&msg)
     {
-        log(loc, lvl, "{}", msg);
+        log(loc, lvl, "{}", std::forward<T>(msg));
     }
 
     template<typename T>
-    void trace(const T &msg)
+    void trace(T &&msg)
     {
-        log(level::trace, msg);
+        log(level::trace, std::forward<T>(msg));
     }
 
     template<typename T>
-    void debug(const T &msg)
+    void debug(T &&msg)
     {
-        log(level::debug, msg);
+        log(level::debug, std::forward<T>(msg));
     }
 
     template<typename T>
-    void info(const T &msg)
+    void info(T &&msg)
     {
-        log(level::info, msg);
+        log(level::info, std::forward<T>(msg));
     }
 
     template<typename T>
-    void warn(const T &msg)
+    void warn(T &&msg)
     {
-        log(level::warn, msg);
+        log(level::warn, std::forward<T>(msg));
     }
 
     template<typename T>
-    void error(const T &msg)
+    void error(T &&msg)
     {
-        log(level::err, msg);
+        log(level::err, std::forward<T>(msg));
     }
 
     template<typename T>
-    void critical(const T &msg)
+    void critical(T &&msg)
     {
-        log(level::critical, msg);
+        log(level::critical, std::forward<T>(msg));
     }
 
 #ifdef SPDLOG_WCHAR_TO_UTF8_SUPPORT
@@ -229,7 +225,7 @@ public:
 #else
 
     template<typename... Args>
-    void log(source_loc loc, level::level_enum lvl, wstring_view_t fmt, const Args &... args)
+    void log(source_loc loc, level::level_enum lvl, wstring_view_t fmt, Args &&... args)
     {
         bool log_enabled = should_log(lvl);
         bool traceback_enabled = tracer_.enabled();
@@ -241,7 +237,7 @@ public:
         {
             // format to wmemory_buffer and convert to utf8
             fmt::wmemory_buffer wbuf;
-            fmt::format_to(wbuf, fmt, args...);
+            fmt::format_to(wbuf, fmt, std::forward<Args>(args)...);
 
             memory_buf_t buf;
             details::os::wstr_to_utf8buf(wstring_view_t(wbuf.data(), wbuf.size()), buf);
@@ -251,51 +247,9 @@ public:
         SPDLOG_LOGGER_CATCH()
     }
 
-    template<typename... Args>
-    void log(level::level_enum lvl, wstring_view_t fmt, const Args &... args)
-    {
-        log(source_loc{}, lvl, fmt, args...);
-    }
-
-    template<typename... Args>
-    void trace(wstring_view_t fmt, const Args &... args)
-    {
-        log(level::trace, fmt, args...);
-    }
-
-    template<typename... Args>
-    void debug(wstring_view_t fmt, const Args &... args)
-    {
-        log(level::debug, fmt, args...);
-    }
-
-    template<typename... Args>
-    void info(wstring_view_t fmt, const Args &... args)
-    {
-        log(level::info, fmt, args...);
-    }
-
-    template<typename... Args>
-    void warn(wstring_view_t fmt, const Args &... args)
-    {
-        log(level::warn, fmt, args...);
-    }
-
-    template<typename... Args>
-    void error(wstring_view_t fmt, const Args &... args)
-    {
-        log(level::err, fmt, args...);
-    }
-
-    template<typename... Args>
-    void critical(wstring_view_t fmt, const Args &... args)
-    {
-        log(level::critical, fmt, args...);
-    }
-
     // T can be statically converted to wstring_view
-    template<class T, typename std::enable_if<is_convertible_to_wstring_view<const T &>::value, T>::type * = nullptr>
-    void log(source_loc loc, level::level_enum lvl, const T &msg)
+    template<class T, typename std::enable_if<is_convertible_to_wstring_view<T &&>::value, T>::type * = nullptr>
+    void log(source_loc loc, level::level_enum lvl, T &&msg)
     {
         bool log_enabled = should_log(lvl);
         bool traceback_enabled = tracer_.enabled();
@@ -307,7 +261,7 @@ public:
         SPDLOG_TRY
         {
             memory_buf_t buf;
-            details::os::wstr_to_utf8buf(msg, buf);
+            details::os::wstr_to_utf8buf(std::forward<T>(msg), buf);
             details::log_msg log_msg(loc, name_, lvl, string_view_t(buf.data(), buf.size()));
             log_it_(log_msg, log_enabled, traceback_enabled);
         }
@@ -370,6 +324,26 @@ protected:
     err_handler custom_err_handler_{nullptr};
     details::backtracer tracer_;
 
+    // common implementation for after templated public api has been resolved
+    template<typename FormatString, typename... Args>
+    void log_(source_loc loc, level::level_enum lvl, FormatString &&fmt, Args &&... args)
+    {
+        bool log_enabled = should_log(lvl);
+        bool traceback_enabled = tracer_.enabled();
+        if (!log_enabled && !traceback_enabled)
+        {
+            return;
+        }
+        SPDLOG_TRY
+        {
+            memory_buf_t buf;
+            fmt::format_to(buf, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
+            details::log_msg log_msg(loc, name_, lvl, string_view_t(buf.data(), buf.size()));
+            log_it_(log_msg, log_enabled, traceback_enabled);
+        }
+        SPDLOG_LOGGER_CATCH()
+    }
+
     // log the given message (if the given log level is high enough),
     // and save backtrace (if backtrace is enabled).
     void log_it_(const details::log_msg &log_msg, bool log_enabled, bool traceback_enabled);
diff --git a/include/spdlog/spdlog.h b/include/spdlog/spdlog.h
index 55de6676..1407e7c0 100644
--- a/include/spdlog/spdlog.h
+++ b/include/spdlog/spdlog.h
@@ -121,153 +121,102 @@ SPDLOG_API spdlog::logger *default_logger_raw();
 
 SPDLOG_API void set_default_logger(std::shared_ptr<spdlog::logger> default_logger);
 
-template<typename... Args>
-inline void log(source_loc source, level::level_enum lvl, string_view_t fmt, const Args &... args)
+template<typename FormatString, typename... Args>
+inline void log(source_loc source, level::level_enum lvl, FormatString &&fmt, Args &&... args)
 {
-    default_logger_raw()->log(source, lvl, fmt, args...);
+    default_logger_raw()->log(source, lvl, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
 }
 
-template<typename... Args>
-inline void log(level::level_enum lvl, string_view_t fmt, const Args &... args)
+template<typename FormatString, typename... Args>
+inline void log(level::level_enum lvl, FormatString &&fmt, Args &&... args)
 {
-    default_logger_raw()->log(source_loc{}, lvl, fmt, args...);
+    default_logger_raw()->log(source_loc{}, lvl, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
 }
 
-template<typename... Args>
-inline void trace(string_view_t fmt, const Args &... args)
+template<typename FormatString, typename... Args>
+inline void trace(FormatString &&fmt, Args &&... args)
 {
-    default_logger_raw()->trace(fmt, args...);
+    default_logger_raw()->trace(std::forward<FormatString>(fmt), std::forward<Args>(args)...);
 }
 
-template<typename... Args>
-inline void debug(string_view_t fmt, const Args &... args)
+template<typename FormatString, typename... Args>
+inline void debug(FormatString &&fmt, Args &&... args)
 {
-    default_logger_raw()->debug(fmt, args...);
+    default_logger_raw()->debug(std::forward<FormatString>(fmt), std::forward<Args>(args)...);
 }
 
-template<typename... Args>
-inline void info(string_view_t fmt, const Args &... args)
+template<typename FormatString, typename... Args>
+inline void info(FormatString &&fmt, Args &&... args)
 {
-    default_logger_raw()->info(fmt, args...);
+    default_logger_raw()->info(std::forward<FormatString>(fmt), std::forward<Args>(args)...);
 }
 
-template<typename... Args>
-inline void warn(string_view_t fmt, const Args &... args)
+template<typename FormatString, typename... Args>
+inline void warn(FormatString &&fmt, Args &&... args)
 {
-    default_logger_raw()->warn(fmt, args...);
+    default_logger_raw()->warn(std::forward<FormatString>(fmt), std::forward<Args>(args)...);
 }
 
-template<typename... Args>
-inline void error(string_view_t fmt, const Args &... args)
+template<typename FormatString, typename... Args>
+inline void error(FormatString &&fmt, Args &&... args)
 {
-    default_logger_raw()->error(fmt, args...);
+    default_logger_raw()->error(std::forward<FormatString>(fmt), std::forward<Args>(args)...);
 }
 
-template<typename... Args>
-inline void critical(string_view_t fmt, const Args &... args)
+template<typename FormatString, typename... Args>
+inline void critical(FormatString &&fmt, const Args &... args)
 {
-    default_logger_raw()->critical(fmt, args...);
+    default_logger_raw()->critical(std::forward<FormatString>(fmt), args...);
 }
 
 template<typename T>
-inline void log(source_loc source, level::level_enum lvl, const T &msg)
+inline void log(source_loc source, level::level_enum lvl, T &&msg)
 {
-    default_logger_raw()->log(source, lvl, msg);
+    default_logger_raw()->log(source, lvl, std::forward<T>(msg));
 }
 
 template<typename T>
-inline void log(level::level_enum lvl, const T &msg)
+inline void log(level::level_enum lvl, T &&msg)
 {
-    default_logger_raw()->log(lvl, msg);
+    default_logger_raw()->log(lvl, std::forward<T>(msg));
 }
 
 template<typename T>
-inline void trace(const T &msg)
+inline void trace(T &&msg)
 {
-    default_logger_raw()->trace(msg);
+    default_logger_raw()->trace(std::forward<T>(msg));
 }
 
 template<typename T>
-inline void debug(const T &msg)
+inline void debug(T &&msg)
 {
-    default_logger_raw()->debug(msg);
+    default_logger_raw()->debug(std::forward<T>(msg));
 }
 
 template<typename T>
-inline void info(const T &msg)
+inline void info(T &&msg)
 {
-    default_logger_raw()->info(msg);
+    default_logger_raw()->info(std::forward<T>(msg));
 }
 
 template<typename T>
-inline void warn(const T &msg)
+inline void warn(T &&msg)
 {
-    default_logger_raw()->warn(msg);
+    default_logger_raw()->warn(std::forward<T>(msg));
 }
 
 template<typename T>
-inline void error(const T &msg)
+inline void error(T &&msg)
 {
-    default_logger_raw()->error(msg);
+    default_logger_raw()->error(std::forward<T>(msg));
 }
 
 template<typename T>
-inline void critical(const T &msg)
+inline void critical(T &&msg)
 {
-    default_logger_raw()->critical(msg);
+    default_logger_raw()->critical(std::forward<T>(msg));
 }
 
-#ifdef SPDLOG_WCHAR_TO_UTF8_SUPPORT
-template<typename... Args>
-inline void log(source_loc source, level::level_enum lvl, wstring_view_t fmt, const Args &... args)
-{
-    default_logger_raw()->log(source, lvl, fmt, args...);
-}
-
-template<typename... Args>
-inline void log(level::level_enum lvl, wstring_view_t fmt, const Args &... args)
-{
-    default_logger_raw()->log(lvl, fmt, args...);
-}
-
-template<typename... Args>
-inline void trace(wstring_view_t fmt, const Args &... args)
-{
-    default_logger_raw()->trace(fmt, args...);
-}
-
-template<typename... Args>
-inline void debug(wstring_view_t fmt, const Args &... args)
-{
-    default_logger_raw()->debug(fmt, args...);
-}
-
-template<typename... Args>
-inline void info(wstring_view_t fmt, const Args &... args)
-{
-    default_logger_raw()->info(fmt, args...);
-}
-
-template<typename... Args>
-inline void warn(wstring_view_t fmt, const Args &... args)
-{
-    default_logger_raw()->warn(fmt, args...);
-}
-
-template<typename... Args>
-inline void error(wstring_view_t fmt, const Args &... args)
-{
-    default_logger_raw()->error(fmt, args...);
-}
-
-template<typename... Args>
-inline void critical(wstring_view_t fmt, const Args &... args)
-{
-    default_logger_raw()->critical(fmt, args...);
-}
-
-#endif // SPDLOG_WCHAR_TO_UTF8_SUPPORT
-
 } // namespace spdlog
 
 //
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 2dd4958b..6cb0c44d 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -68,3 +68,29 @@ endif()
 if(SPDLOG_BUILD_TESTS_HO OR SPDLOG_BUILD_ALL)
     spdlog_prepare_test(spdlog-utests-ho spdlog::spdlog_header_only)
 endif()
+
+# Set up compilation failure test case (only available if compiler supports relaxed constexpr and c++ standard is > 11)
+set(HAVE_CXX_RELAXED_CONSTEXPR)
+if(CMAKE_CXX_STANDARD GREATER 11) # If we're in c++11 mode we don't have relaxed constexpr, even if our compiler is new enough.
+    list(FIND CMAKE_CXX_COMPILE_FEATURES "cxx_relaxed_constexpr" HAVE_CXX_RELAXED_CONSTEXPR)
+endif()
+if(HAVE_CXX_RELAXED_CONSTEXPR AND (SPDLOG_BUILD_FAILING_TESTS OR SPDLOG_BUILD_ALL))
+    message(STATUS "Enabling negative compilation unit test target")
+    set(SPDLOG_FAIL_COMPILATION_TARGET spdlog_fail_compilation_utests)
+    add_executable(${SPDLOG_FAIL_COMPILATION_TARGET}
+        test_compilation_failures.cpp
+        main.cpp)
+    spdlog_enable_warnings(${SPDLOG_FAIL_COMPILATION_TARGET})
+    target_link_libraries(${SPDLOG_FAIL_COMPILATION_TARGET} PRIVATE spdlog::spdlog)
+    set_target_properties(${SPDLOG_FAIL_COMPILATION_TARGET} PROPERTIES
+        EXCLUDE_FROM_ALL TRUE
+        EXCLUDE_FROM_DEFAULT_BUILD TRUE)
+    add_test(NAME ${SPDLOG_FAIL_COMPILATION_TARGET}
+        COMMAND ${CMAKE_COMMAND} --build . --target ${SPDLOG_FAIL_COMPILATION_TARGET} --config $<CONFIG>
+        WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
+    set_tests_properties(${SPDLOG_FAIL_COMPILATION_TARGET} PROPERTIES PASS_REGULAR_EXPRESSION "invalid type specifier")
+    
+    add_custom_target(${SPDLOG_FAIL_COMPILATION_TARGET}_run_target
+        COMMAND ${CMAKE_CTEST_COMMAND} -R ${SPDLOG_FAIL_COMPILATION_TARGET} --output-on-failure
+        COMMENT "Running tests that fail to compile.")
+endif()
diff --git a/tests/test_compilation_failures.cpp b/tests/test_compilation_failures.cpp
new file mode 100644
index 00000000..640adbdb
--- /dev/null
+++ b/tests/test_compilation_failures.cpp
@@ -0,0 +1,14 @@
+#include "includes.h"
+
+TEST_CASE("{fmt} FMT_STRING functionality preserved (negative test)", "[fmt][fail][fail compilation]")
+{
+    std::ostringstream oss;
+    auto oss_sink = std::make_shared<spdlog::sinks::ostream_sink_mt>(oss);
+
+    spdlog::set_default_logger(std::make_shared<spdlog::logger>("oss", oss_sink));
+    spdlog::default_logger()->set_level(spdlog::level::trace);
+
+    spdlog::info(FMT_STRING("The best part of {{fmt}} is the compile time checking: {:d}"), "I shouldn't compile");
+    // This should never be able to compile, so running is a failure.
+    FAIL("This test case isn't meant to compile, let alone run.");
+}
diff --git a/tests/test_misc.cpp b/tests/test_misc.cpp
index 1d67e879..cf43f6f4 100644
--- a/tests/test_misc.cpp
+++ b/tests/test_misc.cpp
@@ -3,7 +3,7 @@
 #include "spdlog/fmt/bin_to_hex.h"
 
 template<class T>
-std::string log_info(const T &what, spdlog::level::level_enum logger_level = spdlog::level::info)
+std::string log_info(T &&what, spdlog::level::level_enum logger_level = spdlog::level::info)
 {
 
     std::ostringstream oss;
@@ -12,7 +12,7 @@ std::string log_info(const T &what, spdlog::level::level_enum logger_level = spd
     spdlog::logger oss_logger("oss", oss_sink);
     oss_logger.set_level(logger_level);
     oss_logger.set_pattern("%v");
-    oss_logger.info(what);
+    oss_logger.info(std::forward<T>(what));
 
     return oss.str().substr(0, oss.str().length() - strlen(spdlog::details::os::default_eol));
 }
@@ -269,3 +269,34 @@ TEST_CASE("default logger API", "[default logger]")
     spdlog::drop_all();
     spdlog::set_pattern("%v");
 }
+
+TEST_CASE("{fmt} FMT_STRING functionality preserved (positive test)", "[fmt]")
+{
+    std::ostringstream oss;
+    auto oss_sink = std::make_shared<spdlog::sinks::ostream_sink_mt>(oss);
+
+    spdlog::set_default_logger(std::make_shared<spdlog::logger>("oss", oss_sink));
+    spdlog::default_logger()->set_level(spdlog::level::trace);
+    spdlog::set_pattern("%v");
+
+    const std::string expected_output_all(
+        std::string("The best part of {fmt} is the compile time checking: 42") + std::string(spdlog::details::os::default_eol));
+    spdlog::trace(FMT_STRING("The best part of {{fmt}} is the compile time checking: {:d}"), 42);
+    REQUIRE(oss.str() == expected_output_all);
+    oss.str("");
+    spdlog::debug(FMT_STRING("The best part of {{fmt}} is the compile time checking: {:d}"), 42);
+    REQUIRE(oss.str() == expected_output_all);
+    oss.str("");
+    spdlog::info(FMT_STRING("The best part of {{fmt}} is the compile time checking: {:d}"), 42);
+    REQUIRE(oss.str() == expected_output_all);
+    oss.str("");
+    spdlog::warn(FMT_STRING("The best part of {{fmt}} is the compile time checking: {:d}"), 42);
+    REQUIRE(oss.str() == expected_output_all);
+    oss.str("");
+    spdlog::error(FMT_STRING("The best part of {{fmt}} is the compile time checking: {:d}"), 42);
+    REQUIRE(oss.str() == expected_output_all);
+    oss.str("");
+    spdlog::critical(FMT_STRING("The best part of {{fmt}} is the compile time checking: {:d}"), 42);
+    REQUIRE(oss.str() == expected_output_all);
+    oss.str("");
+}

From 3041faffab1bf0aba46cbcf51b697336d0e846fa Mon Sep 17 00:00:00 2001
From: Joe Burzinski <tridacnid@gmail.com>
Date: Tue, 2 Jun 2020 20:30:25 -0500
Subject: [PATCH 2/3] Address code review comments: revert perfect forwarding
 on places that didn't need it, remove negative compilation unit test.

---
 CMakeLists.txt                      |  1 -
 include/spdlog/logger.h             | 94 ++++++++++++++---------------
 include/spdlog/spdlog.h             | 60 +++++++++---------
 tests/CMakeLists.txt                | 26 --------
 tests/test_compilation_failures.cpp | 14 -----
 tests/test_misc.cpp                 | 35 +----------
 6 files changed, 79 insertions(+), 151 deletions(-)
 delete mode 100644 tests/test_compilation_failures.cpp

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 7b792cab..f8941df3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -71,7 +71,6 @@ option(SPDLOG_BUILD_EXAMPLE_HO "Build header only example" OFF)
 # testing options
 option(SPDLOG_BUILD_TESTS "Build tests" OFF)
 option(SPDLOG_BUILD_TESTS_HO "Build tests using the header only version" OFF)
-option(SPDLOG_BUILD_FAILING_TESTS "Add unit test targets that test for compilation failures" OFF)
 
 # bench options
 option(SPDLOG_BUILD_BENCH "Build benchmarks (Requires https://github.com/google/benchmark.git to be installed)" OFF)
diff --git a/include/spdlog/logger.h b/include/spdlog/logger.h
index ed44f526..6cf99e13 100644
--- a/include/spdlog/logger.h
+++ b/include/spdlog/logger.h
@@ -76,71 +76,71 @@ public:
     // FormatString is a type derived from fmt::compile_string
     template<typename FormatString, typename std::enable_if<std::is_base_of<fmt::compile_string, FormatString>::value, int>::type = 0,
         typename... Args>
-    void log(source_loc loc, level::level_enum lvl, FormatString &&fmt, Args &&... args)
+    void log(source_loc loc, level::level_enum lvl, FormatString &&fmt, const Args &... args)
     {
-        log_(loc, lvl, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
+        log_(loc, lvl, std::forward<FormatString>(fmt), args...);
     }
 
     // FormatString is NOT a type derived from fmt::compile_string but is a string_view_t or can be implicitly converted to one
     template<typename... Args>
-    void log(source_loc loc, level::level_enum lvl, string_view_t fmt, Args &&... args)
+    void log(source_loc loc, level::level_enum lvl, string_view_t fmt, const Args &... args)
     {
-        log_(loc, lvl, fmt, std::forward<Args>(args)...);
+        log_(loc, lvl, fmt, args...);
     }
 
     template<typename FormatString, typename... Args>
-    void log(level::level_enum lvl, FormatString &&fmt, Args &&... args)
+    void log(level::level_enum lvl, FormatString &&fmt, const Args &... args)
     {
-        log(source_loc{}, lvl, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
+        log(source_loc{}, lvl, std::forward<FormatString>(fmt), args...);
     }
 
     template<typename FormatString, typename... Args>
-    void trace(FormatString &&fmt, Args &&... args)
+    void trace(FormatString &&fmt, const Args &... args)
     {
-        log(level::trace, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
+        log(level::trace, std::forward<FormatString>(fmt), args...);
     }
 
     template<typename FormatString, typename... Args>
-    void debug(FormatString &&fmt, Args &&... args)
+    void debug(FormatString &&fmt, const Args &... args)
     {
-        log(level::debug, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
+        log(level::debug, std::forward<FormatString>(fmt), args...);
     }
 
     template<typename FormatString, typename... Args>
-    void info(FormatString &&fmt, Args &&... args)
+    void info(FormatString &&fmt, const Args &... args)
     {
-        log(level::info, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
+        log(level::info, std::forward<FormatString>(fmt), args...);
     }
 
     template<typename FormatString, typename... Args>
-    void warn(FormatString &&fmt, Args &&... args)
+    void warn(FormatString &&fmt, const Args &... args)
     {
-        log(level::warn, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
+        log(level::warn, std::forward<FormatString>(fmt), args...);
     }
 
     template<typename FormatString, typename... Args>
-    void error(FormatString &&fmt, Args &&... args)
+    void error(FormatString &&fmt, const Args &... args)
     {
-        log(level::err, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
+        log(level::err, std::forward<FormatString>(fmt), args...);
     }
 
     template<typename FormatString, typename... Args>
-    void critical(FormatString &&fmt, Args &&... args)
+    void critical(FormatString &&fmt, const Args &... args)
     {
-        log(level::critical, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
+        log(level::critical, std::forward<FormatString>(fmt), args...);
     }
 
     template<typename T>
-    void log(level::level_enum lvl, T &&msg)
+    void log(level::level_enum lvl, const T &msg)
     {
-        log(source_loc{}, lvl, std::forward<T>(msg));
+        log(source_loc{}, lvl, msg);
     }
 
     // T can be statically converted to string_view
-    template<class T, typename std::enable_if<std::is_convertible<T &&, spdlog::string_view_t>::value, T>::type * = nullptr>
-    void log(source_loc loc, level::level_enum lvl, T &&msg)
+    template<class T, typename std::enable_if<std::is_convertible<const T &, spdlog::string_view_t>::value, T>::type * = nullptr>
+    void log(source_loc loc, level::level_enum lvl, const T &msg)
     {
-        log(loc, lvl, string_view_t{std::forward<T>(msg)});
+        log(loc, lvl, string_view_t{msg});
     }
 
     void log(log_clock::time_point log_time, source_loc loc, level::level_enum lvl, string_view_t msg)
@@ -175,48 +175,48 @@ public:
     }
 
     // T cannot be statically converted to string_view or wstring_view
-    template<class T,
-        typename std::enable_if<!std::is_convertible<T &&, spdlog::string_view_t>::value && !is_convertible_to_wstring_view<T &&>::value,
-            T>::type * = nullptr>
-    void log(source_loc loc, level::level_enum lvl, T &&msg)
+    template<class T, typename std::enable_if<!std::is_convertible<const T &, spdlog::string_view_t>::value &&
+                                                  !is_convertible_to_wstring_view<const T &>::value,
+                          T>::type * = nullptr>
+    void log(source_loc loc, level::level_enum lvl, const T &msg)
     {
-        log(loc, lvl, "{}", std::forward<T>(msg));
+        log(loc, lvl, "{}", msg);
     }
 
     template<typename T>
-    void trace(T &&msg)
+    void trace(const T &msg)
     {
-        log(level::trace, std::forward<T>(msg));
+        log(level::trace, msg);
     }
 
     template<typename T>
-    void debug(T &&msg)
+    void debug(const T &msg)
     {
-        log(level::debug, std::forward<T>(msg));
+        log(level::debug, msg);
     }
 
     template<typename T>
-    void info(T &&msg)
+    void info(const T &msg)
     {
-        log(level::info, std::forward<T>(msg));
+        log(level::info, msg);
     }
 
     template<typename T>
-    void warn(T &&msg)
+    void warn(const T &msg)
     {
-        log(level::warn, std::forward<T>(msg));
+        log(level::warn, msg);
     }
 
     template<typename T>
-    void error(T &&msg)
+    void error(const T &msg)
     {
-        log(level::err, std::forward<T>(msg));
+        log(level::err, msg);
     }
 
     template<typename T>
-    void critical(T &&msg)
+    void critical(const T &msg)
     {
-        log(level::critical, std::forward<T>(msg));
+        log(level::critical, msg);
     }
 
 #ifdef SPDLOG_WCHAR_TO_UTF8_SUPPORT
@@ -225,7 +225,7 @@ public:
 #else
 
     template<typename... Args>
-    void log(source_loc loc, level::level_enum lvl, wstring_view_t fmt, Args &&... args)
+    void log(source_loc loc, level::level_enum lvl, wstring_view_t fmt, const Args &... args)
     {
         bool log_enabled = should_log(lvl);
         bool traceback_enabled = tracer_.enabled();
@@ -237,7 +237,7 @@ public:
         {
             // format to wmemory_buffer and convert to utf8
             fmt::wmemory_buffer wbuf;
-            fmt::format_to(wbuf, fmt, std::forward<Args>(args)...);
+            fmt::format_to(wbuf, fmt, args...);
 
             memory_buf_t buf;
             details::os::wstr_to_utf8buf(wstring_view_t(wbuf.data(), wbuf.size()), buf);
@@ -248,8 +248,8 @@ public:
     }
 
     // T can be statically converted to wstring_view
-    template<class T, typename std::enable_if<is_convertible_to_wstring_view<T &&>::value, T>::type * = nullptr>
-    void log(source_loc loc, level::level_enum lvl, T &&msg)
+    template<class T, typename std::enable_if<is_convertible_to_wstring_view<const T &>::value, T>::type * = nullptr>
+    void log(source_loc loc, level::level_enum lvl, const T &msg)
     {
         bool log_enabled = should_log(lvl);
         bool traceback_enabled = tracer_.enabled();
@@ -261,7 +261,7 @@ public:
         SPDLOG_TRY
         {
             memory_buf_t buf;
-            details::os::wstr_to_utf8buf(std::forward<T>(msg), buf);
+            details::os::wstr_to_utf8buf(msg, buf);
             details::log_msg log_msg(loc, name_, lvl, string_view_t(buf.data(), buf.size()));
             log_it_(log_msg, log_enabled, traceback_enabled);
         }
@@ -326,7 +326,7 @@ protected:
 
     // common implementation for after templated public api has been resolved
     template<typename FormatString, typename... Args>
-    void log_(source_loc loc, level::level_enum lvl, FormatString &&fmt, Args &&... args)
+    void log_(source_loc loc, level::level_enum lvl, FormatString &&fmt, const Args &... args)
     {
         bool log_enabled = should_log(lvl);
         bool traceback_enabled = tracer_.enabled();
@@ -337,7 +337,7 @@ protected:
         SPDLOG_TRY
         {
             memory_buf_t buf;
-            fmt::format_to(buf, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
+            fmt::format_to(buf, std::forward<FormatString>(fmt), args...);
             details::log_msg log_msg(loc, name_, lvl, string_view_t(buf.data(), buf.size()));
             log_it_(log_msg, log_enabled, traceback_enabled);
         }
diff --git a/include/spdlog/spdlog.h b/include/spdlog/spdlog.h
index 1407e7c0..44c0d0d9 100644
--- a/include/spdlog/spdlog.h
+++ b/include/spdlog/spdlog.h
@@ -122,45 +122,45 @@ SPDLOG_API spdlog::logger *default_logger_raw();
 SPDLOG_API void set_default_logger(std::shared_ptr<spdlog::logger> default_logger);
 
 template<typename FormatString, typename... Args>
-inline void log(source_loc source, level::level_enum lvl, FormatString &&fmt, Args &&... args)
+inline void log(source_loc source, level::level_enum lvl, FormatString &&fmt, const Args &... args)
 {
-    default_logger_raw()->log(source, lvl, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
+    default_logger_raw()->log(source, lvl, std::forward<FormatString>(fmt), args...);
 }
 
 template<typename FormatString, typename... Args>
-inline void log(level::level_enum lvl, FormatString &&fmt, Args &&... args)
+inline void log(level::level_enum lvl, FormatString &&fmt, const Args &... args)
 {
-    default_logger_raw()->log(source_loc{}, lvl, std::forward<FormatString>(fmt), std::forward<Args>(args)...);
+    default_logger_raw()->log(source_loc{}, lvl, std::forward<FormatString>(fmt), args...);
 }
 
 template<typename FormatString, typename... Args>
-inline void trace(FormatString &&fmt, Args &&... args)
+inline void trace(FormatString &&fmt, const Args &... args)
 {
-    default_logger_raw()->trace(std::forward<FormatString>(fmt), std::forward<Args>(args)...);
+    default_logger_raw()->trace(std::forward<FormatString>(fmt), args...);
 }
 
 template<typename FormatString, typename... Args>
-inline void debug(FormatString &&fmt, Args &&... args)
+inline void debug(FormatString &&fmt, const Args &... args)
 {
-    default_logger_raw()->debug(std::forward<FormatString>(fmt), std::forward<Args>(args)...);
+    default_logger_raw()->debug(std::forward<FormatString>(fmt), args...);
 }
 
 template<typename FormatString, typename... Args>
-inline void info(FormatString &&fmt, Args &&... args)
+inline void info(FormatString &&fmt, const Args &... args)
 {
-    default_logger_raw()->info(std::forward<FormatString>(fmt), std::forward<Args>(args)...);
+    default_logger_raw()->info(std::forward<FormatString>(fmt), args...);
 }
 
 template<typename FormatString, typename... Args>
-inline void warn(FormatString &&fmt, Args &&... args)
+inline void warn(FormatString &&fmt, const Args &... args)
 {
-    default_logger_raw()->warn(std::forward<FormatString>(fmt), std::forward<Args>(args)...);
+    default_logger_raw()->warn(std::forward<FormatString>(fmt), args...);
 }
 
 template<typename FormatString, typename... Args>
-inline void error(FormatString &&fmt, Args &&... args)
+inline void error(FormatString &&fmt, const Args &... args)
 {
-    default_logger_raw()->error(std::forward<FormatString>(fmt), std::forward<Args>(args)...);
+    default_logger_raw()->error(std::forward<FormatString>(fmt), args...);
 }
 
 template<typename FormatString, typename... Args>
@@ -170,51 +170,51 @@ inline void critical(FormatString &&fmt, const Args &... args)
 }
 
 template<typename T>
-inline void log(source_loc source, level::level_enum lvl, T &&msg)
+inline void log(source_loc source, level::level_enum lvl, const T &msg)
 {
-    default_logger_raw()->log(source, lvl, std::forward<T>(msg));
+    default_logger_raw()->log(source, lvl, msg);
 }
 
 template<typename T>
-inline void log(level::level_enum lvl, T &&msg)
+inline void log(level::level_enum lvl, const T &msg)
 {
-    default_logger_raw()->log(lvl, std::forward<T>(msg));
+    default_logger_raw()->log(lvl, msg);
 }
 
 template<typename T>
-inline void trace(T &&msg)
+inline void trace(const T &msg)
 {
-    default_logger_raw()->trace(std::forward<T>(msg));
+    default_logger_raw()->trace(msg);
 }
 
 template<typename T>
-inline void debug(T &&msg)
+inline void debug(const T &msg)
 {
-    default_logger_raw()->debug(std::forward<T>(msg));
+    default_logger_raw()->debug(msg);
 }
 
 template<typename T>
-inline void info(T &&msg)
+inline void info(const T &msg)
 {
-    default_logger_raw()->info(std::forward<T>(msg));
+    default_logger_raw()->info(msg);
 }
 
 template<typename T>
-inline void warn(T &&msg)
+inline void warn(const T &msg)
 {
-    default_logger_raw()->warn(std::forward<T>(msg));
+    default_logger_raw()->warn(msg);
 }
 
 template<typename T>
-inline void error(T &&msg)
+inline void error(const T &msg)
 {
-    default_logger_raw()->error(std::forward<T>(msg));
+    default_logger_raw()->error(msg);
 }
 
 template<typename T>
-inline void critical(T &&msg)
+inline void critical(const T &msg)
 {
-    default_logger_raw()->critical(std::forward<T>(msg));
+    default_logger_raw()->critical(msg);
 }
 
 } // namespace spdlog
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 6cb0c44d..2dd4958b 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -68,29 +68,3 @@ endif()
 if(SPDLOG_BUILD_TESTS_HO OR SPDLOG_BUILD_ALL)
     spdlog_prepare_test(spdlog-utests-ho spdlog::spdlog_header_only)
 endif()
-
-# Set up compilation failure test case (only available if compiler supports relaxed constexpr and c++ standard is > 11)
-set(HAVE_CXX_RELAXED_CONSTEXPR)
-if(CMAKE_CXX_STANDARD GREATER 11) # If we're in c++11 mode we don't have relaxed constexpr, even if our compiler is new enough.
-    list(FIND CMAKE_CXX_COMPILE_FEATURES "cxx_relaxed_constexpr" HAVE_CXX_RELAXED_CONSTEXPR)
-endif()
-if(HAVE_CXX_RELAXED_CONSTEXPR AND (SPDLOG_BUILD_FAILING_TESTS OR SPDLOG_BUILD_ALL))
-    message(STATUS "Enabling negative compilation unit test target")
-    set(SPDLOG_FAIL_COMPILATION_TARGET spdlog_fail_compilation_utests)
-    add_executable(${SPDLOG_FAIL_COMPILATION_TARGET}
-        test_compilation_failures.cpp
-        main.cpp)
-    spdlog_enable_warnings(${SPDLOG_FAIL_COMPILATION_TARGET})
-    target_link_libraries(${SPDLOG_FAIL_COMPILATION_TARGET} PRIVATE spdlog::spdlog)
-    set_target_properties(${SPDLOG_FAIL_COMPILATION_TARGET} PROPERTIES
-        EXCLUDE_FROM_ALL TRUE
-        EXCLUDE_FROM_DEFAULT_BUILD TRUE)
-    add_test(NAME ${SPDLOG_FAIL_COMPILATION_TARGET}
-        COMMAND ${CMAKE_COMMAND} --build . --target ${SPDLOG_FAIL_COMPILATION_TARGET} --config $<CONFIG>
-        WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
-    set_tests_properties(${SPDLOG_FAIL_COMPILATION_TARGET} PROPERTIES PASS_REGULAR_EXPRESSION "invalid type specifier")
-    
-    add_custom_target(${SPDLOG_FAIL_COMPILATION_TARGET}_run_target
-        COMMAND ${CMAKE_CTEST_COMMAND} -R ${SPDLOG_FAIL_COMPILATION_TARGET} --output-on-failure
-        COMMENT "Running tests that fail to compile.")
-endif()
diff --git a/tests/test_compilation_failures.cpp b/tests/test_compilation_failures.cpp
deleted file mode 100644
index 640adbdb..00000000
--- a/tests/test_compilation_failures.cpp
+++ /dev/null
@@ -1,14 +0,0 @@
-#include "includes.h"
-
-TEST_CASE("{fmt} FMT_STRING functionality preserved (negative test)", "[fmt][fail][fail compilation]")
-{
-    std::ostringstream oss;
-    auto oss_sink = std::make_shared<spdlog::sinks::ostream_sink_mt>(oss);
-
-    spdlog::set_default_logger(std::make_shared<spdlog::logger>("oss", oss_sink));
-    spdlog::default_logger()->set_level(spdlog::level::trace);
-
-    spdlog::info(FMT_STRING("The best part of {{fmt}} is the compile time checking: {:d}"), "I shouldn't compile");
-    // This should never be able to compile, so running is a failure.
-    FAIL("This test case isn't meant to compile, let alone run.");
-}
diff --git a/tests/test_misc.cpp b/tests/test_misc.cpp
index cf43f6f4..1d67e879 100644
--- a/tests/test_misc.cpp
+++ b/tests/test_misc.cpp
@@ -3,7 +3,7 @@
 #include "spdlog/fmt/bin_to_hex.h"
 
 template<class T>
-std::string log_info(T &&what, spdlog::level::level_enum logger_level = spdlog::level::info)
+std::string log_info(const T &what, spdlog::level::level_enum logger_level = spdlog::level::info)
 {
 
     std::ostringstream oss;
@@ -12,7 +12,7 @@ std::string log_info(T &&what, spdlog::level::level_enum logger_level = spdlog::
     spdlog::logger oss_logger("oss", oss_sink);
     oss_logger.set_level(logger_level);
     oss_logger.set_pattern("%v");
-    oss_logger.info(std::forward<T>(what));
+    oss_logger.info(what);
 
     return oss.str().substr(0, oss.str().length() - strlen(spdlog::details::os::default_eol));
 }
@@ -269,34 +269,3 @@ TEST_CASE("default logger API", "[default logger]")
     spdlog::drop_all();
     spdlog::set_pattern("%v");
 }
-
-TEST_CASE("{fmt} FMT_STRING functionality preserved (positive test)", "[fmt]")
-{
-    std::ostringstream oss;
-    auto oss_sink = std::make_shared<spdlog::sinks::ostream_sink_mt>(oss);
-
-    spdlog::set_default_logger(std::make_shared<spdlog::logger>("oss", oss_sink));
-    spdlog::default_logger()->set_level(spdlog::level::trace);
-    spdlog::set_pattern("%v");
-
-    const std::string expected_output_all(
-        std::string("The best part of {fmt} is the compile time checking: 42") + std::string(spdlog::details::os::default_eol));
-    spdlog::trace(FMT_STRING("The best part of {{fmt}} is the compile time checking: {:d}"), 42);
-    REQUIRE(oss.str() == expected_output_all);
-    oss.str("");
-    spdlog::debug(FMT_STRING("The best part of {{fmt}} is the compile time checking: {:d}"), 42);
-    REQUIRE(oss.str() == expected_output_all);
-    oss.str("");
-    spdlog::info(FMT_STRING("The best part of {{fmt}} is the compile time checking: {:d}"), 42);
-    REQUIRE(oss.str() == expected_output_all);
-    oss.str("");
-    spdlog::warn(FMT_STRING("The best part of {{fmt}} is the compile time checking: {:d}"), 42);
-    REQUIRE(oss.str() == expected_output_all);
-    oss.str("");
-    spdlog::error(FMT_STRING("The best part of {{fmt}} is the compile time checking: {:d}"), 42);
-    REQUIRE(oss.str() == expected_output_all);
-    oss.str("");
-    spdlog::critical(FMT_STRING("The best part of {{fmt}} is the compile time checking: {:d}"), 42);
-    REQUIRE(oss.str() == expected_output_all);
-    oss.str("");
-}

From 741b0d6e82d58cf3cca74fffc12630a8b150ca0f Mon Sep 17 00:00:00 2001
From: Joe Burzinski <tridacnid@gmail.com>
Date: Wed, 3 Jun 2020 21:47:48 -0500
Subject: [PATCH 3/3] Address code review comments: remove perfect forwarding
 on FormatString template parameters.

---
 include/spdlog/logger.h | 45 +++++++++++++++++++++--------------------
 include/spdlog/spdlog.h | 32 ++++++++++++++---------------
 2 files changed, 39 insertions(+), 38 deletions(-)

diff --git a/include/spdlog/logger.h b/include/spdlog/logger.h
index 6cf99e13..25e5383e 100644
--- a/include/spdlog/logger.h
+++ b/include/spdlog/logger.h
@@ -74,11 +74,10 @@ public:
     void swap(spdlog::logger &other) SPDLOG_NOEXCEPT;
 
     // FormatString is a type derived from fmt::compile_string
-    template<typename FormatString, typename std::enable_if<std::is_base_of<fmt::compile_string, FormatString>::value, int>::type = 0,
-        typename... Args>
-    void log(source_loc loc, level::level_enum lvl, FormatString &&fmt, const Args &... args)
+    template<typename FormatString, typename std::enable_if<fmt::is_compile_string<FormatString>::value, int>::type = 0, typename... Args>
+    void log(source_loc loc, level::level_enum lvl, const FormatString &fmt, const Args &... args)
     {
-        log_(loc, lvl, std::forward<FormatString>(fmt), args...);
+        log_(loc, lvl, fmt, args...);
     }
 
     // FormatString is NOT a type derived from fmt::compile_string but is a string_view_t or can be implicitly converted to one
@@ -89,45 +88,45 @@ public:
     }
 
     template<typename FormatString, typename... Args>
-    void log(level::level_enum lvl, FormatString &&fmt, const Args &... args)
+    void log(level::level_enum lvl, const FormatString &fmt, const Args &... args)
     {
-        log(source_loc{}, lvl, std::forward<FormatString>(fmt), args...);
+        log(source_loc{}, lvl, fmt, args...);
     }
 
     template<typename FormatString, typename... Args>
-    void trace(FormatString &&fmt, const Args &... args)
+    void trace(const FormatString &fmt, const Args &... args)
     {
-        log(level::trace, std::forward<FormatString>(fmt), args...);
+        log(level::trace, fmt, args...);
     }
 
     template<typename FormatString, typename... Args>
-    void debug(FormatString &&fmt, const Args &... args)
+    void debug(const FormatString &fmt, const Args &... args)
     {
-        log(level::debug, std::forward<FormatString>(fmt), args...);
+        log(level::debug, fmt, args...);
     }
 
     template<typename FormatString, typename... Args>
-    void info(FormatString &&fmt, const Args &... args)
+    void info(const FormatString &fmt, const Args &... args)
     {
-        log(level::info, std::forward<FormatString>(fmt), args...);
+        log(level::info, fmt, args...);
     }
 
     template<typename FormatString, typename... Args>
-    void warn(FormatString &&fmt, const Args &... args)
+    void warn(const FormatString &fmt, const Args &... args)
     {
-        log(level::warn, std::forward<FormatString>(fmt), args...);
+        log(level::warn, fmt, args...);
     }
 
     template<typename FormatString, typename... Args>
-    void error(FormatString &&fmt, const Args &... args)
+    void error(const FormatString &fmt, const Args &... args)
     {
-        log(level::err, std::forward<FormatString>(fmt), args...);
+        log(level::err, fmt, args...);
     }
 
     template<typename FormatString, typename... Args>
-    void critical(FormatString &&fmt, const Args &... args)
+    void critical(const FormatString &fmt, const Args &... args)
     {
-        log(level::critical, std::forward<FormatString>(fmt), args...);
+        log(level::critical, fmt, args...);
     }
 
     template<typename T>
@@ -136,8 +135,10 @@ public:
         log(source_loc{}, lvl, msg);
     }
 
-    // T can be statically converted to string_view
-    template<class T, typename std::enable_if<std::is_convertible<const T &, spdlog::string_view_t>::value, T>::type * = nullptr>
+    // T can be statically converted to string_view and isn't a fmt::compile_string
+    template<class T,
+        typename std::enable_if<std::is_convertible<const T &, spdlog::string_view_t>::value && !fmt::is_compile_string<T>::value, T>::type
+            * = nullptr>
     void log(source_loc loc, level::level_enum lvl, const T &msg)
     {
         log(loc, lvl, string_view_t{msg});
@@ -326,7 +327,7 @@ protected:
 
     // common implementation for after templated public api has been resolved
     template<typename FormatString, typename... Args>
-    void log_(source_loc loc, level::level_enum lvl, FormatString &&fmt, const Args &... args)
+    void log_(source_loc loc, level::level_enum lvl, const FormatString &fmt, const Args &... args)
     {
         bool log_enabled = should_log(lvl);
         bool traceback_enabled = tracer_.enabled();
@@ -337,7 +338,7 @@ protected:
         SPDLOG_TRY
         {
             memory_buf_t buf;
-            fmt::format_to(buf, std::forward<FormatString>(fmt), args...);
+            fmt::format_to(buf, fmt, args...);
             details::log_msg log_msg(loc, name_, lvl, string_view_t(buf.data(), buf.size()));
             log_it_(log_msg, log_enabled, traceback_enabled);
         }
diff --git a/include/spdlog/spdlog.h b/include/spdlog/spdlog.h
index 44c0d0d9..fa219950 100644
--- a/include/spdlog/spdlog.h
+++ b/include/spdlog/spdlog.h
@@ -122,51 +122,51 @@ SPDLOG_API spdlog::logger *default_logger_raw();
 SPDLOG_API void set_default_logger(std::shared_ptr<spdlog::logger> default_logger);
 
 template<typename FormatString, typename... Args>
-inline void log(source_loc source, level::level_enum lvl, FormatString &&fmt, const Args &... args)
+inline void log(source_loc source, level::level_enum lvl, const FormatString &fmt, const Args &... args)
 {
-    default_logger_raw()->log(source, lvl, std::forward<FormatString>(fmt), args...);
+    default_logger_raw()->log(source, lvl, fmt, args...);
 }
 
 template<typename FormatString, typename... Args>
-inline void log(level::level_enum lvl, FormatString &&fmt, const Args &... args)
+inline void log(level::level_enum lvl, const FormatString &fmt, const Args &... args)
 {
-    default_logger_raw()->log(source_loc{}, lvl, std::forward<FormatString>(fmt), args...);
+    default_logger_raw()->log(source_loc{}, lvl, fmt, args...);
 }
 
 template<typename FormatString, typename... Args>
-inline void trace(FormatString &&fmt, const Args &... args)
+inline void trace(const FormatString &fmt, const Args &... args)
 {
-    default_logger_raw()->trace(std::forward<FormatString>(fmt), args...);
+    default_logger_raw()->trace(fmt, args...);
 }
 
 template<typename FormatString, typename... Args>
-inline void debug(FormatString &&fmt, const Args &... args)
+inline void debug(const FormatString &fmt, const Args &... args)
 {
-    default_logger_raw()->debug(std::forward<FormatString>(fmt), args...);
+    default_logger_raw()->debug(fmt, args...);
 }
 
 template<typename FormatString, typename... Args>
-inline void info(FormatString &&fmt, const Args &... args)
+inline void info(const FormatString &fmt, const Args &... args)
 {
-    default_logger_raw()->info(std::forward<FormatString>(fmt), args...);
+    default_logger_raw()->info(fmt, args...);
 }
 
 template<typename FormatString, typename... Args>
-inline void warn(FormatString &&fmt, const Args &... args)
+inline void warn(const FormatString &fmt, const Args &... args)
 {
-    default_logger_raw()->warn(std::forward<FormatString>(fmt), args...);
+    default_logger_raw()->warn(fmt, args...);
 }
 
 template<typename FormatString, typename... Args>
-inline void error(FormatString &&fmt, const Args &... args)
+inline void error(const FormatString &fmt, const Args &... args)
 {
-    default_logger_raw()->error(std::forward<FormatString>(fmt), args...);
+    default_logger_raw()->error(fmt, args...);
 }
 
 template<typename FormatString, typename... Args>
-inline void critical(FormatString &&fmt, const Args &... args)
+inline void critical(const FormatString &fmt, const Args &... args)
 {
-    default_logger_raw()->critical(std::forward<FormatString>(fmt), args...);
+    default_logger_raw()->critical(fmt, args...);
 }
 
 template<typename T>