Latest compilers emit a ton of C4996 warnings as errors

[Dan-Albrecht]

Windows Terminal version

main @ 2e91e9c

Windows build number

10.0.22621.0

Other Software

Visual Studio Version 17.8.0 Preview 3.0 [34118.359.main]

msbuild -version
MSBuild version 17.8.0-preview-23431-02+3c910ba83 for .NET Framework
17.8.0.43102

Steps to reproduce

• Open OpenConsole solution
• Set to x64
• F5

Expected Behavior

No errors

Actual Behavior

1,123 C4996 warnings as errors. e.g.:

Error	C4996	'stdext::checked_array_iterator<_Ty*>': warning STL4043: stdext::checked_array_iterator, stdext::unchecked_array_iterator, and related factory functions are non-Standard extensions and will be removed in the future. 
std::span (since C++20) and gsl::span can be used instead. 
You can define _SILENCE_STDEXT_ARR_ITERS_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to suppress this warning.	
fmt	C:\git\terminal\oss\fmt\include\fmt\format.h	359

[lhecker]

Related: fmtlib/fmt#3540
We currently use 7.1.3 (2 years old? wat). In the 9 versions since then (currently 10.1.1) we've missed:

• compile-time format string checks
• faster format string compilation
• "{}"_cf instead of FMT_COMPILE("{}")
• smaller PCH
• way faster float and hex formatting
• way faster format_to
• ...

[zadjii-msft]

Yep we should probably just update that. Thanks!

[DHowett]

Related: #15855

I am committing to not updating our checked-in dependencies until we have a Governance-clean pipeline for doing so :)

[lhecker]

BTW I just noticed: Happy round-issue-number to you!

It turns out that the upgrade is a significant pain in the butt, because fmt largely removed support for wchar_t. Things that don't work anymore:

• Passing wstring parameters to string formattings and vice versa
• Implicitly casting hstring to wstring_view

I started working on this in https://github.com/microsoft/terminal/tree/dev/lhecker/16000-fmt-update

[DHowett]

I am uncomfortable with the premier C++ formatting library removing support for an entire platform like that. Considering that Windows predominantly uses wide strings, that makes it nearly unfit for purpose.

Do you happen to have any links to docs about them explicitly dropping support?

[lhecker]

Hey! I said "largely removed", not "dropped". 😄 And I meant it in the context of our project which uses char -> wchar_t conversions quite a bit. Maybe I should've worded it better, like "because fmt removed parts that we took a significant dependency on".

fmt now matches the behavior of std::format which doesn't provide a builtin implicit conversion either:

error C2338: static_assert failed: 'Cannot format an argument. To make type T formattable, provide a formatter<T> specialization. See N4950 [format.arg.store]/2 and [formatter.requirements].'

The same is true for winrt::hstring which also fails with the above message with std::format.

I don't know which commit(s) removed either of the two features, because there's no commit in fmt/xchar.h's history that has a fitting title, there's no issue with one either, and I'm not sure what to look for when reading the massive format-inl.h file.
But it fits the C++ standard behavior so I don't think we're missing out on anything.

[lhecker]

I finished fixing all issues and as it turns out, the wchar_t / char thing was fairly minor. The biggest problem was fixing all those

format(RS_(...), ...)

which now need to be

format(fmt::runtime(RS_(...)), ...)

because it now does compile time format checks even without FMT_COMPILE (great!), but that doesn't work for hstring parameters because fmt defines its own fmt::string_view class for pre-C++17 compatibility. fmt::string_view has an implicit constructor for std::string_view, but an hstring is only implicitly convertible to an std::string_view.
Me and my C++ compiler have spent a lot of time today staring out of the window, while the "door stuck" soundtrack played in the back of my head.