Fix #2034 #2037
Fix #2034 #2037
Conversation
|
@dkavolis thank you! this patch fixed errors I started getting from clang 12 and c++20 after updating to latest v1.x version. |
include/spdlog/common.h
Outdated
| template<class T> | ||
| struct is_convertible_to_wformat_string : std::is_convertible<T, fmt::wformat_string<>> | ||
| {}; | ||
| using is_convertible_to_wformat_string = is_convertible_to_basic_format_string<T, wchar_t>; |
There was a problem hiding this comment.
Does it compile if SPDLOG_WCHAR_TO_UTF8_SUPPORT (and hence is_convertible_to_wformat_string ) are not defined ? Or does SFINAE allows it?
Anyway I think it is cleaner just to remove the all those is_convertible_to_wformat_string definitions completely (lines 134-147) and do something like:
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>::value>
{};There was a problem hiding this comment.
is_convertible_to_wformat_string is defined the same way as is_convertible_to_wstring_view but a quick search in the repo showed no uses of either outside of logger::log SFINAE check if msg is convertible to a string view or format string. I used is_convertible_to_wstring_view as an example for is_convertible_to_wformat_string since they are both very similar. Might be cleaner to remove both in this case.
|
Also removed the SFINAE check for string view conversion in |
|
Thanks @dkavolis |
|
@dkavolis I added CI to test c++20 but it fails here: https://travis-ci.com/github/gabime/spdlog/jobs/530329470 Could you please take a look please? (I tried fixing with wrapping with fmt::runtime the problematic line but it didn't help) |
|
Seems like an issue with clang, the error happens when using FMT_CONSTEXPR_CHAR_TRAITS
FMT_INLINE
basic_string_view(const Char* s) : data_(s) {
if (detail::const_check(std::is_same<Char, char>::value &&
!detail::is_constant_evaluated()))
size_ = std::strlen(reinterpret_cast<const char*>(s));
else
size_ = std::char_traits<Char>::length(s);
}But the only way this can happen is if either Can it be due to the old libstdc++ version? GCC is only version 7.5.0 and |
|
So I will try replace the ci to gcc and see what happens Update: |
For some reason clang doesn't like a disabled constructor in
std::is_convertibleso conversion to format strings has to be replaced by conversion to equivalent string views as infmt.This time also tested with Clang 12.0.1 C++20. Might be a good idea to add C++20 to CI.
Fixes #2034