MSVC errors when importing both fmt and std modules

[matt77hias]

MSVC has an open bug that prevents a.o. importing both fmt and std modules in some use cases (i.e. depends which functionality one uses from the std) due to the mixing of std includes inside fmt itself and using import std.

For example:

error C2572: 'std::enable_if': redefinition of default argument: parameter 1

It is possible to work around this by replacing std includes inside fmt with import std instead, and including the C headers for the few types and functions defined outside the std namespace (because these are not exported from the std module):

#include <limits.h>
#include <stdint.h>
#include <stdio.h>
#include <time.h>

Maybe something worth considering with an optional feature macro?

[matt77hias]

tl;dr

I created a new fork with all changes (in reverse chronological order) required to use (i.e. not just build a static lib) MSVC + C++ modules + fmt:

• Resolved warning C4127: conditional expression is constant
  • Independent of C++ modules.
  • Trivial to bring back to fmt.
• Resolved warning C4996: 'fileno': The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name: _fileno.
  • Independent of C++ modules.
  • Trivial to bring back to fmt.
• Worked around error C2039: 'vformat': is not a member of 'fmt'
  • (Existing?) MSVC bug
• Worked around error C2039: 'vformat': is not a member of 'fmt'
  • (Existing?) MSVC bug
• Worked around error C2572: 'std::enable_if': redefinition of default argument: parameter 1
  • Open MSVC bug
• Resolved FMT_USE_CONSTEVAL for MSVC
  * Seems a valid fmt bug.
• Worked around error C2129: static function 'errno_t gmtime_s(tm *const ,const time_t *const )' declared but not defined
  • Open MSVC bug

New MSVC bug

[phprus]

Resolved warning C4127: conditional expression is constant
Independent of C++ modules.
Trivial to bring back to fmt.

Use const_check, like this:

https://github.com/fmtlib/fmt/blob/4e8640ed90ac8751d4a8ca500b893cc8c4bb9668/include/fmt/base.h#L2210C7-L2210C18

[matt77hias]

Note that defined(__cpp_if_constexpr) is used throughout the code as well, but not for just guarding the constexpr part of if constexpr (which looks like hell ;-)), though.

[vitaut]

Resolved warning C4127: conditional expression is constant

A PR for the conditional expression is constant warning is welcome (using const_check).

Resolved FMT_USE_CONSTEVAL for MSVC

What is the problem that you are trying to solve with this change?

Maybe something worth considering with an optional feature macro?

I would wait until the std module support is more stable.

[matt77hias]

A PR for the conditional expression is constant warning is welcome (using const_check).

Done

What is the problem that you are trying to solve with this change?

This is not something that is apparent when fmt is built as a static library and be done with it. Upon using this version my own wrappers (in particular the consteval constructor of fmt::basic_format_string< C, ArgTs... >) started to break because consteval support for MSVC is disabled. Note that both consteval and std::is_constant_evaluated work just fine for MSVC. Maybe the former broke at some point, but the latter has always worked for me. FWIIW my FMT_MSC_VERSION is currently 1939.

[vitaut]

AFAICS FMT_USE_CONSTEVAL is set correctly on MSVC: https://www.godbolt.org/z/s8WT53PY9.

[matt77hias]

My mistake. import std does not import the feature macros. There is a missing <version> include that should have been part of the mixing of std imports and includes bucket (i.e. 3da01e3).

• Added missing include for feature macro testing

[matt77hias]

I would wait until the std module support is more stable.

Imho, given the potential reduced build time import std (C++23) could potentially have over std includes (of which some are included more than once), it could be useful if fmt provided a feature macro to prefer one over the other. I don't see this as a temporary hack but something that has the potential of becoming the default path in the long run.

Though, the below specific snippet is a hack for sure and not something I suggest using ;-)

#define static
#include <time.h>
#undef static

[ShifftC]

We maintain an internal fork of fmt that utilizes std modules with a feature macro.

The error "'vformat': is not a member of 'fmt'" is due to the mixed usage of FMT_BEGIN_EXPORT and namespaces in format.h, which gets closed early by } // namespace detail.
In our fork we replaced all instances of FMT_BEGIN_EXPORT and FMT_BEGIN_END with FMT_EXPORT to prevent issues like this and force to export only whats necessary.

I also can't reproduce the error with time.h, simply including the header (when using std module) works. The error usually appears when importing time.h as a header-unit as there is a bug where static inline functions can't be called.

[matt77hias]

Good catch. This fixes both

Error	C2039	'vformat': is not a member of 'fmt'	fmt	C:\Users\...\fmt\fmt\fmt\format.h	4399
Error	C2661	'fmt::v10::vformat': no overloaded function takes 2 arguments	fmt	C:\Users\...\fmt\fmt\fmt\format-inl.h	153

I also can't reproduce the error with time.h, simply including the header (when using std module) works. The error usually appears when importing time.h as a header-unit as there is a bug where static inline functions can't be called.

Fwiiw in my case, the error is logged upon building module implementation units that use std::chrono::system_clock::time_point in a formatted string (e.g., {:%H:%M:%S}).

[vitaut]

given the potential reduced build time import std (C++23) could potentially have over std includes (of which some are included more than once), it could be useful if fmt provided a feature macro to prefer one over the other.

Fair enough. Could you submit a PR to do this (possibly in a subset of {fmt} for starters)?

[matt77hias]

Could you submit a PR to do this?

#3928

[vitaut]

With #3928 merged, I guess this can be closed now. The fileno warning is unrelated to modules but a PR to fix it would be welcome.

[matt77hias]

a PR to fix it would be welcome.

#3930

[matt77hias]

I now also modularized some Windows specific code requiring wchar_t formatting, and narrowed it down to the below:

import fmt;

auto main()
    -> int
{
    auto str = fmt::format(L"{}", 1u);
	
    return 0;
}

1>C:\Users\...\fmt\format.h(3565,1): fatal  error C1001: Internal compiler error.
1>(compiler file 'msc1.cpp', line 1587)
1> To work around this problem, try simplifying or changing the program near the locations listed above.
1>If possible please provide a repro here: https://developercommunity.visualstudio.com
1>Please choose the Technical Support command on the Visual C++
1> Help menu, or open the Technical Support help file for more information
1>INTERNAL COMPILER ERROR in 'C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.39.33519\bin\HostX64\x64\CL.exe'
1>    Please choose the Technical Support command on the Visual C++
1>    Help menu, or open the Technical Support help file for more information

@ShifftC do you happen to have a specific workaround (or do you only use char formatting)? 🤞

My experience so far with all the internal compiler errors for C++ modules that I encountered, is that there are workarounds. Unfortunately, it requires stripping every use case down to a few LOCs in order to isolate the actual problem.

[ShifftC]

If have a fork with some fixes for msvc and clang, including an ice, still trying to get it to work with gcc but that might just be my setup. Not sure if it's the same ice, currently not home so I can't test it.

[matt77hias]

Tried it for the above example. Unfortunately, still the same ice.

Created a ticket.

[ShifftC]

Compiling the example before #3928 gives 'fmt::v10::detail::convert_float': no matching overloaded function found instead of an ice for me. Our internal fork just fails to compile without an error 🙃. We convert everything to char before internal usage.

I have also done some testing because of the error with time.h, turns out including time.h in the global module fragment of the module using chrono formatting also 'fixes' it, at least for std::chrono::time_point and std::tm, formatting std::chrono::duration also gives an ice.

[matt77hias]

Upon some further trial and error, it turns out that

• with the #define FMT_IMPORT_STD in fmt.cc:
  • char formatting works
  • wchar_t formatting results in the aforementioned internal compiler error
• without the #define FMT_IMPORT_STD in fmt.cc:
  • char formatting works
  • wchar_t formatting works