Thanks for the PR! The checks are failing in the following tests (you can see this by clicking on Details next to the failing checks, then "View more details on Azure Pipelines" - we split the x86 tests across 8 machines or "shards", and clicking on Run Tests under "x86 1" will display one shard's failures. Clicking on the back arrow next to "Jobs in run BLAH" and then clicking on the Tests tab will allow you to see failure logs with compiler errors).

tests/std/tests/P0355R7_calendars_and_time_zones_formatting
tests/std/tests/P0355R7_calendars_and_time_zones_time_zones
tests/std/tests/P0355R7_calendars_and_time_zones_zoned_time

One of the errors, emitted by Clang, is:

In file included from C:\a\1\s\tests\std\tests\P0355R7_calendars_and_time_zones_formatting\test.cpp:5:
D:\build\out\inc\chrono(5970,62): error: implicit instantiation of undefined template 'std::array<char, 4>'
                    _Stream << _STD put_time<_CharT>(&_Time, _Fmt_string(_Spec).data());
                                                             ^

That's defined here:

These are the only mentions of array in <chrono> which is not directly including <array> (and was dragging it in through <format>):

Thus, either <chrono> needs to include <array> (and won't receive any throughput benefit), or _Fmt_string needs to be reworked to not use array (which would be preferable, I think).

Please let us know if you need any help with resolving these failures (we're also available on the STL Discord, linked at the top of the README). To validate a fix, you should run the STL's test suite locally (instructions are in the README, we can also help with getting started there) - at a minimum, the 3 failing test directories here. (Running all of the tests/std/tests/P0355R7_* tests for C++20 <chrono> and all of the tests/std/tests/P0645R10_* tests for <format> might be a good idea and should be very fast. Running the entire test suite can take 30 minutes or more depending on how many cores your PC has, and is probably not necessary for a targeted change.) The rationale (which I'll explain in the Contribution Guide that I need to write up) is to run tests locally before pushing changes to PRs - this helps save time for contributors and reviewers, and avoids unnecessary load on the CI infrastructure (getting the CI machines to run tests is convenient, but it's a shared resource that we've been given a generous-but-monthly-finite-limit, so we ask contributors to verify that tests are locally passing before pushing commits that trigger CI test runs).

Thank you for the explanation.

I think adding <array> to the <chrono> would be a plausible idea if you ok with that. At least for now.
Every header should include its dependencies explicitly, not relying on the others.

Sorry for the local testing absence, just didn't think that such tiny change could lead to such a big chaos🙄

barcharcraz

Now that we don't have to deal with array's iterators not being T*s we can use _STD begin() and _STD end() here.

miscco

I would rather avoid calls to begin and end and simply use the explicit bounds

I guess you have to reach some consensus here, I can't change it back and forth 😎

Oh sorry, I did not see that @barcharcraz suggested those.

Usually we try to avoid adding those function calls as they do impair debug performance. Lets wait for feedback from the maintainers

Oh sorry, I did not see that @barcharcraz suggested those.

Usually we try to avoid adding those function calls as they do impair debug performance. Lets wait for feedback from the maintainers

I don't have super strong thoughts on this. Debug experience isn't a huge deal here as the functions in question don't appear in the callstack above user functions. I'm willing to keep things explicit though.

_STD begin(_Buffer) definitely isn't necessary

Someone please, what's wrong with Format Validation here?
It was ok until latest commit, where only _STD begin was removed nothing else touched.

P.S. How to run this Code Format Validation test locally? Because running all tests take way too long.

Someone please, what's wrong with Format Validation here?

clang-format. If you click on details you will see a diff of what clang-format says it should be

diff --git a/stl/inc/format b/stl/inc/format
index cafb5b2..e8d3977 100644
--- a/stl/inc/format
+++ b/stl/inc/format
@@ -1871,8 +1871,7 @@ template <class _CharT, class _OutputIt>
 _NODISCARD _OutputIt _Fmt_write(_OutputIt _Out, const void* const _Value) {
     // TRANSITION, Reusable buffer
     char _Buffer[_Format_min_buffer_length];
-    const auto [_End, _Ec] =
-        _STD to_chars(_Buffer, _STD end(_Buffer), reinterpret_cast<uintptr_t>(_Value), 16);
+    const auto [_End, _Ec] = _STD to_chars(_Buffer, _STD end(_Buffer), reinterpret_cast<uintptr_t>(_Value), 16);
     _STL_ASSERT(_Ec == errc{}, "to_chars failed");
     *_Out++ = '0';
     *_Out++ = 'x';

How to run this Code Format Validation test locally?

You don't need to, really. Validation is just clang-format and validate.cpp. If you open repo folder with VS Code and use that to save the file it should automatically ensure all of that (I think it uses the .clang-format file by default for formatting otherwise you would need to change that in the settings), due to the settings.json.

Thanks.
I saw this diff but was absolutely sure I didn't make this line-break.

If I run clang-format.exe for <format> from the repo root it makes some changes to the formatting that were not there before.
(for instance it changes
if (_Val > static_cast<unsigned long long>((numeric_limits<int>::max)())) { to
if (_Val > static_cast<unsigned long long>((numeric_limits<int>::max) ())) {

May be a different version or something...
I just haven't figured it out yet. That's why I asked for the local Code Format Validation.

May be a different version or something...

Definitely, I just tried it with clang-format 12 and got the same thing. The repo currently uses the version that ships with Visual Studio which is version 11.

Well, I took the opportunity to rework <chrono>'s _Fmt_string function, as was suggested by StephanTLavavej.
It now returns basic_string<_CharT> instead of array.

So the <chrono> doesn't need to include <array> header no more, as well.

Status update: We merge changes on GitHub and our MSVC-internal repo simultaneously; as this is a semi-manual process we batch up PRs to save time. Your PR will be part of the next batch, almost certainly this week.

Thanks for improving compiler throughput - and congratulations on your first microsoft/STL commit! 😺 🎉 🚀

This will ship in VS 2022 17.0 Preview 4.