Skip to content
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

Use 'SPDLOG_FMT_RUNTIME' to fix compilation error throwed MSVC and fmt 9.1.x #2517

Merged
merged 2 commits into from
Oct 19, 2022

Conversation

YunchengLiu
Copy link
Contributor

This is the first time I submit a PR to the open source community. I try to submit code and instructions according to the rules I have learned, but there may be inappropriate places to do it. Please also point out my problems actively, I will try to do it as soon as possible. Learn and correct.

fmt 9.1.x added compile-time checks for wide strings, so when compiled with MSVC and this or later version of fmt(9.1.x), codes belows will throw a compilation error #(#2512).

# if defined(_MSC_VER) && defined(SPDLOG_WCHAR_FILENAMES) // for some reason msvc doesn't allow fmt::runtime(..) with wchar here
return fmt::format(fmt_filename, now_tm);
# else
return fmt::format(SPDLOG_FMT_RUNTIME(fmt_filename), now_tm);
# endif

When compiled with SPDLOG_WCHAR_FILENAMES with a newer verision of fmt(9.1.x) using MSVC, SPDLOG_FMT_RUNTIME will solve the problem.

#    if defined(_MSC_VER) && defined(SPDLOG_WCHAR_FILENAMES) // for some reason msvc doesn't allow fmt::runtime(..) with wchar here
#       if FMT_VERSION >= 90101
            // fmt 9.1.x added compile-time checks for wide strings
            // so when compiled with MSVC and this or later version of fmt
            // here should use 'SPDLOG_FMT_RUNTIME' to enable 'fmt::runtime(...)' for wchar to avoid compile error
            return fmt::format(SPDLOG_FMT_RUNTIME(fmt_filename), now_tm);
#       else
            return fmt::format(fmt_filename, now_tm);
#       endif
#    else
        return fmt::format(SPDLOG_FMT_RUNTIME(fmt_filename), now_tm);
#    endif

@gabime
Copy link
Owner

gabime commented Oct 17, 2022

Thanks. Seems this all can be reduced to

// msvc doesn't allow fmt::runtime(..) with wchar, with fmtlib versions < 9.1.0
#if defined(SPDLOG_WCHAR_FILENAMES) && defined(_MSC_VER) && FMT_VERSION < 90101
   return fmt::format(fmt_filename, now_tm);
#else 
  return fmt::format(SPDLOG_FMT_RUNTIME(fmt_filename), now_tm);
#endif

…VC compiler and wchar filename support macros
@YunchengLiu
Copy link
Contributor Author

Thanks. I have simplified the implement code.

@YunchengLiu
Copy link
Contributor Author

@gabime Hi, I found the ci test failed at the code below.

REQUIRE(record->NumStrings == 1);

It seems that this is not due to my PR, probably.

I want to know what should I do next, thanks.

@gabime gabime merged commit 3c0e036 into gabime:v1.x Oct 19, 2022
@gabime
Copy link
Owner

gabime commented Oct 19, 2022

Thanks @YunchengLiu . Merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants