Compile-time check bypassed due to overloading. about spdlog

[tt4g]

Thank you for reporting.
Could you send PR?

[gabime]

This is kind of expected. If there are no args spdlog does’t treat the passed string as format string for performance reasons, and bypass the fmt formatting.

[tt4g]

@gabime Should I add a section to the FAQ page (https://github.com/gabime/spdlog/wiki/0.-FAQ) about this design?

[Luffbee]

Is this the desired behavior? It seems this overload can be removed, as there is another overload that can cover the use cases of it:

spdlog/include/spdlog/logger.h

Lines 93 to 97 in 6230201

	// T cannot be statically converted to format string (including string_view/wstring_view)
	template <class T,
	typename std::enable_if<!is_convertible_to_any_format_string<const T &>::value,
	int>::type = 0>
	void log(source_loc loc, level::level_enum lvl, const T &msg) {

Firstly, my apology, I misunderstood the above overload, it is used to handle cases like SPDLOG_INFO(type_can_fmt{}).
The std::string_view fits the requirement of is_convertible_to_any_format_string.

I tried to fix the problem by adding an overload of logger::log_(), which will bypass the fmt formatting if there are no arguments, like the following:

    ...
    void log_(source_loc loc, level::level_enum lvl, string_view_t msg) {
        log_no_fmt_(loc, lvl, msg); 
    }
    // copy from the `log(..., string_view_t)`
    void log_no_fmt_(source_loc loc, level::level_enum lvl, string_view_t msg) {
        bool log_enabled = should_log(lvl);
        bool traceback_enabled = tracer_.enabled();
        if (!log_enabled && !traceback_enabled) {
            return;
        }

        details::log_msg log_msg(loc, name_, lvl, msg);
        log_it_(log_msg, log_enabled, traceback_enabled);
    }

However, it fails to compile with many errors of "error: the value of ‘msg’ is not usable in a constant expression", which is caused by using non-literal strings, e.g.:

// bench/latency.cpp
void bench_c_string(benchmark::State &state, std::shared_ptr<spdlog::logger> logger) {
    const char *msg =
        "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vestibulum pharetra metus cursus "
        "lacus placerat congue. Nulla egestas, mauris a tincidunt tempus, enim lectus volutpat mi, "
        "eu consequat sem "
        "libero nec massa. In dapibus ipsum a diam rhoncus gravida. Etiam non dapibus eros. Donec "
        "fringilla dui sed "
        "augue pretium, nec scelerisque est maximus. Nullam convallis, sem nec blandit maximus, "
        "nisi turpis ornare "
        "nisl, sit amet volutpat neque massa eu odio. Maecenas malesuada quam ex, posuere congue "
        "nibh turpis duis.";

    for (auto _ : state) {
        logger->info(msg);
    }
}

So the problem now is how to correctly detect non-literal strings and dispatch them to log_no_fmt_?
Is there any effort to solve this problem before?

[tt4g]

Use fmt::runtime() for non-literal characters: #2786

---

It is returns runtime object basic_runtime<char> in fmt 9.1.0 (https://fmt.dev/9.1.0/api.html#_CPPv4N3fmt7runtimeE11string_view).

Note fmt::runtime() returns runtime_format_string<> fmt 10 and above (https://fmt.dev/10.0.0/api.html#_CPPv4N3fmt7runtimeE11string_view).

The template function log(source_loc loc, level::level_enum lvl, const T &msg) does not fit because is_convertible_to_any_format_string<const T &>::value == true when using fmt::runtime().

spdlog/include/spdlog/common.h

Lines 219 to 223 in 6230201

	template <class T>
	struct is_convertible_to_any_format_string
	: std::integral_constant<bool,
	is_convertible_to_basic_format_string<T, char>::value ||
	is_convertible_to_basic_format_string<T, wchar_t>::value> {};

spdlog/include/spdlog/common.h

Lines 186 to 201 in 6230201

	template <typename Char>
	#if FMT_VERSION >= 90101
	using fmt_runtime_string = fmt::runtime_format_string<Char>;
	#else
	using fmt_runtime_string = fmt::basic_runtime<Char>;
	#endif
	// clang doesn't like SFINAE disabled constructor in std::is_convertible<> so have to repeat the
	// condition from basic_format_string here, in addition, fmt::basic_runtime<Char> is only
	// convertible to basic_format_string<Char> but not basic_string_view<Char>
	template <class T, class Char = char>
	struct is_convertible_to_basic_format_string
	: std::integral_constant<bool,
	std::is_convertible<T, fmt::basic_string_view<Char>>::value ||
	std::is_same<remove_cvref_t<T>, fmt_runtime_string<Char>>::value> {
	};

[tt4g]

Looking at the logging template function signatures, it seems that it would be very difficult to change the current implementation.

The only way to make the compile time check work with just a literal string would be to directly call log(source_loc loc, level::level_enum lvl, format_string_t<Args...> fmt, Args &&...args) that calls log_(source_loc loc, level::level_enum lvl, string_view_t fmt, Args &&...args).

spdlog/include/spdlog/logger.h

Lines 78 to 81 in 6230201

	template <typename... Args>
	void log(source_loc loc, level::level_enum lvl, format_string_t<Args...> fmt, Args &&...args) {
	log_(loc, lvl, details::to_string_view(fmt), std::forward<Args>(args)...);
	}

Since info(const T &msg) cannot call log(source_loc loc, level::level_enum lvl, format_string_t<Args...> fmt, Args &&...args), try calling log(source_loc loc, level::level_enum lvl, format_string_t<Args...> fmt, Args &&...args) directly.

spdlog/include/spdlog/logger.h

Lines 241 to 244 in 6230201

	template <typename T>
	void info(const T &msg) {
	log(level::info, msg);
	}

[Luffbee]

Since info(const T &msg) cannot call log(source_loc loc, level::level_enum lvl, format_string_t<Args...> fmt, Args &&...args), try calling log(source_loc loc, level::level_enum lvl, format_string_t<Args...> fmt, Args &&...args) directly.

spdlog/include/spdlog/logger.h

Lines 241 to 244 in 6230201

	template <typename T>
	void info(const T &msg) {
	log(level::info, msg);
	}

There is another similar overload can achieve the compile-time check

spdlog/include/spdlog/logger.h

Lines 138 to 141 in 6230201

	template <typename... Args>
	void info(format_string_t<Args...> fmt, Args &&...args) {
	log(level::info, fmt, std::forward<Args>(args)...);
	}

The problem is info("{}") will resolve to the info(const T &msg) overload.
Therefore, I tried to add the !is_convertible_to_any_format_string requirement to info(const T &msg), which makes the info("{}") resolve to info(format_string_t<Args...> fmt, Args &&...args), and compile-time check works for info("{}").

template <class T, 
           typename std::enable_if<!is_convertible_to_any_format_string<const T &>::value, 
                                   int>::type = 0> 
void info(const T &msg) { 
     log(level::info, msg); 
} 

However, this modification makes non-literal strings resolve to the compile-time check overload, too.

std::string s = fmt::format("{}", 123);
// without modification: resolves to `info(const T&msg)`, compile ok
// with modification: resolves to `info(format_string_t<Args...> fmt, Args &&...args)`, compile fail (c++20) due to `s` is not constexpr
spdlog::info(s);

The problem here is there is no way to do different dispatch for constexpr argument and non-constexpr argument before c++23.
With c++23, the if consteval feature seems can help in this problem.

[tt4g]

Yes, this problem cannot be avoided because if consteval is not available in C++11.
If you really want to do a compile-time check when passing only string, you will have to call log(source_loc loc, level::level_enum lvl, format_string_t<Args...> fmt, Args &&...args).

[gabime]

@gabime Should I add a section to the FAQ page (https://github.com/gabime/spdlog/wiki/0.-FAQ) about this design?

If this is actually a FAQ then yes please

[tt4g]

If this is actually a FAQ then yes please

Added: https://github.com/gabime/spdlog/wiki/0.-FAQ#format-string-compile-time-check-does-not-work-when-there-are-no-args