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

Make wchar_t overloads usable in module #2260

Merged

Conversation

DanielaE
Copy link
Contributor

Bring detail::find() into scope.

@DanielaE
Copy link
Contributor Author

This might be a defect in msvc's modules implementation, but it unlocks a big chunk of my module-related compile checks.

Code like this compiles now:

import fmt;
#include <chrono>

enum class check_color {red, green, blue};
// insert appropriate formatter here

int main() {
  const auto Now = std::chrono::system_clock::now();
  auto wresult = fmt::format(L"{}", check_color::blue);
  fmt::wformat_args args = fmt::make_format_args<fmt::wformat_context>(L"hi", wresult, Now);
  wresult = fmt::vformat(L"{},{},{:%Y-%m-%d %H:%M:%S}", args);
}

Basically, I could use modularized {fmt} in production with a pre-production version of msvc 🎉

@vitaut
Copy link
Contributor

vitaut commented Apr 28, 2021

For future reference could you post an error that you get without this change?

@DanielaE
Copy link
Contributor Author

format.h(3039): error C3861: 'find': identifier not found (compiling source file check.cpp)
format.h(3845): note: see reference to function template instantiation 'void fmt::v7::detail::parse_format_string<false,wchar_t,fmt::v7::detail::format_handler<iterator,Char,fmt::v7::basic_format_context<fmt::v7::detail::buffer_appender<wchar_t>,Char>>&>(fmt::v7::basic_string_view<Char>,Handler)' being compiled
        with
        [
            Char=wchar_t,
            Handler=fmt::v7::detail::format_handler<iterator,wchar_t,fmt::v7::basic_format_context<fmt::v7::detail::buffer_appender<wchar_t>,wchar_t>> &
        ] (compiling source file check.cpp)
format.h(3890): note: see reference to function template instantiation 'void fmt::v7::detail::vformat_to<T>(fmt::v7::detail::buffer<T> &,fmt::v7::basic_string_view<wchar_t>,fmt::v7::basic_format_args<fmt::v7::basic_format_context<fmt::v7::detail::buffer_appender<T>,wchar_t>>,fmt::v7::detail::locale_ref)' being compiled
        with
        [
            T=wchar_t
        ] (compiling source file check.cpp)
check.cpp(144): note: see reference to function template instantiation 'fmt::v7::detail::buffer_appender<wchar_t> fmt::v7::format_to<wchar_t[3],int,500,wchar_t>(fmt::v7::basic_memory_buffer<wchar_t,500,std::allocator<wchar_t>> &,const S (&),int &&)' being compiled
        with
        [
            S=wchar_t [3]
        ]

@DanielaE
Copy link
Contributor Author

This might be another defect in msvc's modules implementation. There are other problems with names not found when there are partial or total class template specifications involved. Here it's a similar situation but with function templates. May be the compiler mistakenly thinks ADL is involved even though it should be in a template specialization context resurrected from the BMI.

At least the proposed change is innocent.

@DanielaE
Copy link
Contributor Author

This is most likely the same problem as with detail::compile_string_to_view() not being found from FMT_STRING_IMPL, i.e. the reason of the failure to compile to_wstring(T).

@@ -2992,6 +2992,7 @@ FMT_CONSTEXPR const Char* parse_replacement_field(const Char* begin,
template <bool IS_CONSTEXPR, typename Char, typename Handler>
FMT_CONSTEXPR_DECL FMT_INLINE void parse_format_string(
basic_string_view<Char> format_str, Handler&& handler) {
using detail::find;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a short comment saying that this is a workaround for an MSVC bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

Bring ''detail::find()' into scope.
@DanielaE DanielaE force-pushed the feature/unlock-wide-overloads-in-module branch from 632cf45 to 30d4474 Compare April 28, 2021 16:29
@vitaut vitaut merged commit 342973b into fmtlib:master Apr 28, 2021
@vitaut
Copy link
Contributor

vitaut commented Apr 28, 2021

Thank you

@DanielaE DanielaE deleted the feature/unlock-wide-overloads-in-module branch April 28, 2021 17:23
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