[source_loc] support for owning strings

[timblechmann]

source_loc does not own the strings for file name and function name. this is a perfectly reasonable default in most use cases where this information is filled by the preprocessor.

however i'm running into a funny situation when trying to re-direct qt's logging to spdlog and using multithreaded logging:

• the qml engine uses heap-allocated strings for file/function name and fill QMessageLogContext with raw const char* pointers (compare https://github.com/qt/qtdeclarative/blob/abb9cde13d6d0f503a8a6aad12f4361cd3848c79/src/qml/qml/qqmlengine.cpp#L1580-L1605)
• i directly map QMessageLogContext onto source_loc
• in multi-thread spdlog we run into use-after-free issues (since the worker thread accesses the strings that are freed by the other thread)

---

i've been meditating a bit about this. it's a bit of an edge case, but it would be great if spdlog could provide a second api similar to source_loc, that uses owning std::string to allow applications to fill a source_loc from source locations other than c/c++

[tt4g]

First, unless you are using spdlog::async_logger, logs are output synchronously, so the difference between std::string and char * does not matter.

IMO.
source_loc is used to access the __FILE__ and __LINE__ macros.
These macros are provided to hold the location information of the source code from which spdlog logging functions are called.
The copy cost incurred by replacing the compile-time constant char * with std::string cannot be ignored.

As far as I can see QMessageLogContext is an object to transfer log data generated elsewhere to another location.
The location of that log data is not the spdlog logging function call location.
Therefore, you should not use source_log, but should output the log message as additional information to the log forwarded from another location.

[timblechmann]

@tt4g, the analysis is basically the same conclusion that i came to: it would require a separate API in order to support source_loc that cannot be filled from compile-time constants (not an uncommon use case when working with embedded scripting languages). in a "good old c/c++ codebase", this is not an issue

[tt4g]

spdlog uses the fmt library to format log messages.
Since fmt can format user-defined types (https://fmt.dev/9.1.0/api.html#formatting-user-defined-types), You can define template <> struct fmt::formatter<QMessageLogContext> and print QMessageLogContext as the log argument.
e.g. spdlog::info("Qt source info {}, context);

[gabime]

spdlog uses the fmt library to format log messages.
Since fmt can format user-defined types (https://fmt.dev/9.1.0/api.html#formatting-user-defined-types), You can define template <> struct fmt::formatter and print QMessageLogContext as the log argument.
e.g. spdlog::info("Qt source info {}, context);

Right, other ways would require significant modifications to the API.

The fmt formatting is done in the log call site so it would work even if the QMessageLogContext get destroyed before the the async worker thread processes the message. See here for simple example for user defined types.

[timblechmann]

formatting a QMessageLogContext would be the low-hanging fruit, but it prevents using formatting patterns for the file location: https://github.com/gabime/spdlog/wiki/3.-Custom-formatting#pattern-flags or am i missing something?

Right, other ways would require significant modifications to the API.

yes, this is more a feature request in order to ease the integration of spdlog with source location provided from a vm

[gabime]

https://github.com/gabime/spdlog/wiki/3.-Custom-formatting#pattern-flags or am i missing something?

Custom formatter would execute in the async worker thread, so it could be done but care is needed in that regard.

[timblechmann]

Custom formatter would execute in the async worker thread, so it could be done but care is needed in that regard.

from my reading of the code, the filename formatters like %@ directly access source_loc 🤔 ... how can i point this to a different object with a custom formatter?

---

that said: i wonder if the async formatters could ensure that the ownership of the strings that source_loc point to? there are a few other edge cases apart from using VMs, e.g. if __FILE__ / __LINE__ / std::source_location point to memory that's unmapped before the async worker thread accesse them (thinking about dlclose and friends)

[gabime]

from my reading of the code, the filename formatters like %@ directly access source_loc 🤔 ... how can i point this to a different object with a custom formatter?

With the custom formatter you would need to implement some kind of mechanism for this.I am pretty sure it doesn't worth it. Easiest would be implementing user defined type.

[timblechmann]

@gabime, as this ticket is closed as "completed", is there any change that i need to cherry-pick? i've been working on a patch to async_msg to take ownership of these strings, so i wonder if that's obsolete now

[gabime]

It was closed because there is no simple fix for this and the need for this feature is rare to be worth it.

[timblechmann]

would you accept a patch that enables this feature via a preprocessor define?

[gabime]

I prefer not to add complexity to the codebase for such a rare feature.