-
Notifications
You must be signed in to change notification settings - Fork 4.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Better integrate with std::source_location #2973
Conversation
Signed-off-by: Stephan Lachnit <[email protected]>
…ce_location Signed-off-by: Stephan Lachnit <[email protected]>
Signed-off-by: Stephan Lachnit <[email protected]>
Thanks @stephanlachnit . All new features go to the v2.x branch, and since it already contains source_location integration I will have to pass. |
Can you maybe reconsider this? This is not really a new "feature", just a single new constructor for convenience for the exact same feature that already exists. If the |
@gabime I now removed the The PR now only adds a single symbol if |
Not sure I understand why a new implementation of the SPDLOG_LOGGER_CALL macro benefits library users. |
The main point of having void custom_log(Level level, std::string_view message, const std::source_lcation& loc = std::source_location::current()) {
spdlog_logger_->log(loc, static_cast<spdlog::level::level_enum>(level), message);
} We can of course also create If you want I can drop the use of |
This PR aims to better integrate spdlog v1.x with
std::source_location
. Effectively closes #1959.The main idea is to allow constructing
spdlog::source_loc
fromstd::source_location
. This approach is significantly simpler compared to #2667 and still works as one would expect:spdlog::log(std::source_location::current(), spdlog::level::info, "test");
The neat part is that thanks to
std::source_location
beingconstexpr
, it has the same runtime performance as the macro solution. The only caveat is thatstd::source_location
usesstd::uint_least32_t
instead ofint
for the line number - I adaptedspdlog::source_loc
to use this as well, which might result in some compiler/linter warnings when upgrading (though they are trivial to fix by changing the cast type).