spdlog icon indicating copy to clipboard operation
spdlog copied to clipboard

Using std::source_location.

Open gian21391 opened this issue 3 years ago • 5 comments

It would be great to have support for std::source_location without using the macro version when compiling for C++ 20 (related issue at #1823).

I drafted an idea on how to implement that.

The only issue with this implementation is that it would introduce a small breaking change:

spdlog::info(1)

would not compile anymore. However, that does not work with fmt anyway.

gian21391 avatar Mar 07 '23 14:03 gian21391

Thanks. To add this fully means duplicating all of this for all levels (trace(), debug(), warn(), error(), critical()) in spdlog.h AND in logger.h class declarations.

We need to find a way to achieve this without bloating the code. Not sure how..

gabime avatar Mar 08 '23 18:03 gabime

I have now extended the implementation to all the other logging levels and managed to reintroduce support for code like spdlog::info(1). Unfortunately, I don't think it's possible to extend this to the logger class due to the impossibility of having a default parameter (or a parameter in general) after a parameter pack in functions and I don't see yet how the same work-around could be applied there. For that, I guess we have to wait until support for non-terminal parameter pack is added to the standard.

gian21391 avatar Mar 09 '23 14:03 gian21391

Hello all, I found a workaround for passing source_location on variadic info, debug,... functions. I wrap the format_string and added source_location as second ctor parameter that solves the issue. Example:

// common.h
template<typename T>
struct format_string_wrapper
{
    format_string_wrapper(const char* fs, spdlog::details::source_location loc = spdlog::details::source_location::current())
        : format_string_{fs}
        , loc_{loc}
    {}
    operator T()
    {
        return format_string_;
    }
    T format_string_;
    spdlog::details::source_location loc_;
};

template<typename... Args>
using format_string_t = format_string_wrapper<fmt::format_string<Args...>>;

//logger.h
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.format_string_), std::forward<Args>(args)...);
}

mguludag avatar Mar 26 '23 19:03 mguludag

Just wanted to second the approach @mguludag posted — I've been using something similar for a few years in various codebases at work. I actually found this pull request when searching around to see if anyone else had come up with this idea. I have a very basic implementation here that I've used for some personal projects.

Unfortunately a defaulted source_location argument after a function parameter pack doesn't work; the function signature is never matched unless the parameter pack contains 0 items or only a source_location (which defeats the purpose).

An alternate approach is a "double call" syntax, something like handle.warning()("message: {}", msg);. warning would be a function that captures log context (potentially re-usable and/or overridable) and returns a callable context object; the context when called does the actual logging. I don't think this looks as clean so it's not the direction I went. (Note that you wouldn't want to pass the format string in the context parens, as then users are likely to forget to add the second required parens when logging a non-formatted string.)

JadeMatrix avatar Aug 05 '23 23:08 JadeMatrix

Just wanted to second the approach @mguludag posted — I've been using something similar for a few years in various codebases at work. I actually found this pull request when searching around to see if anyone else had come up with this idea. I have a very basic implementation here that I've used for some personal projects.

Unfortunately a defaulted source_location argument after a function parameter pack doesn't work; the function signature is never matched unless the parameter pack contains 0 items or only a source_location (which defeats the purpose).

An alternate approach is a "double call" syntax, something like handle.warning()("message: {}", msg);. warning would be a function that captures log context (potentially re-usable and/or overridable) and returns a callable context object; the context when called does the actual logging. I don't think this looks as clean so it's not the direction I went. (Note that you wouldn't want to pass the format string in the context parens, as then users are likely to forget to add the second required parens when logging a non-formatted string.)

I implemented source_location into spdlog but I removed extra default source_location parameters from spdlog's templated non format_str log functions. Using log functions with format_string the source_location feature works fine. https://github.com/gabime/spdlog/pull/2690

mguludag avatar Aug 06 '23 16:08 mguludag