Revert last change.
Included more headers and the test passes locally.

Forward declare visit_format_arg. Test whether this fixes the modular build.

In D103357#2829269, @vitaut wrote:

LGTM but this will need to be reviewed by someone familiar with libc++ of course =).

Tab -> space.

Tabs -> space.

LGTM, but there's one comment not addressed yet. Please fix that before landing the patch.

Addresses review comments.
Update module map.
Update P2216 status; it will be part of C++23 and backported to C++20.

In D104414#2822776, @Quuxplusone wrote:

If buildkite's happy, I'm happy.

Minor nit-pick found while reading e-mail, haven't looked at the rest of the patch.

Thanks for the review!

I'll look at implementing the proper constrains for the final patch.
For now I would like to determine whether we want to keep or remove the non-standard extension.
(Not sure whether they're extensions or bugs.)

Rebase to make sure CI failures aren't due to this patch.

Fixes an bug in the format_arg constructor for string_view.
Improve the format_arg constructor for string by addressing LWG3542.

In D103863#2805376, @ldionne wrote:

@Mordante The runtimes build doesn't run on macOS nodes, so I think this is fine.

Just FYI when we approve with the condition the CI is green after a rebase, you're free to land the patch after the CI passes.
So feel free to land the patch, or do you need somebody to commit it on your behalf?
If so please provide your name and e-mail address so we commit it on your behalf.

The change is fine by me, but I've some concerns with the build node used for this feature.
The Apple nodes aren't the fastest neither the most reliable build nodes we have.
This combination makes this node less than ideal for build iterations.
So I'm not happy to _only_ enable it for this node, but the improvements can be done in a separate patch.

Thanks for working this. (Actually I was considering to look at it, but happy I didn't start ;-))
Can you investigate the CI errors that are introduced with this patch?

LGTM.

We recently started implementing libc++ is more granular headers. Can you adopt that style for these changes too?
You an use D103734 as an example.
I see a lot of build failure. I did a quick look and I expect none of them are due to this patch, but to main not working properly. Can you rebase your patch to see whether it passes the build?

I didn't look closely at the unit test, but there seems several missing. "…semiregular-box<T> behaves exactly like optional<T> with the following differences…". The optional has several member functions not tested. For example constexpr T* operator->(), constexpr T& operator*(). I also miss the implementation of several of optional's constructors.

I like this direction! I assume the patch has no changes in the moved code.
LGTM modulo one minor nit.

Thanks for the clean-up. I see no big issues, but would like to discuss the backwards compatibility flags before approving.

This patch has been abandoned in favour of D103357. Some of the prerequisites have landed which causes the CI to have merge issues.

In D103444#2792065, @mstorsjo wrote:

In D103444#2791182, @Mordante wrote:

SMGT, but I'd like some unit tests to test the new correct behaviour.

Hmm, so a test under libcxx/test/libcxx somewhere, with a local class subclassing std::__1::__num_put_base so that the protected method __format_int can be called, and then testing the different flag combinations?

Rebase.
Disable a failing test on Windows.

• Rebase
• Fix octal buffer size off by 1.
• Add test for maximum integral values.
• Fixes Windows issue
• Polishing

Note that, while the base prefix for hex (0x) is slightly longer than the
base prefix for octal (0), this isn't a concern. The difference in the space
needed for the value portion of the string is enough to make up for this.

Disable the failing part of a test on Windows.

• Rebase
• Rename __pre and __post since they are reserved on Windows. D103541 will add these names to the nasty macros
• Remove a dupliated hunk. The hunk was moved to its own function to used for Boolean formatting, but the original remained instead of calling the new function.

Thanks a lot for working on this! And thanks @ldionne for setting up a CI to catch these issues earlier!

Temporary disable a failing test on Windows.

Rebase.
Improve comments.

Rebase.
Fixes minor grammar error in comment.

Tabs -> spaces.

Rebase.
Mark unit test requiring concepts.
Minor cleanup.

In D103339#2792317, @DanielMcIntosh-IBM wrote:

rebase

Removed iterator_different_t work-around
Use proper constrain in vformat_to (_CharT instead of char)
Polishing

Thanks for working on this.

Attempt to fix the build.

Rebased on main
Polishing
implement a real output_iterator concept, main has the require prerequisite.

Thanks for the updates, LGTM!

SMGT, but I'd like some unit tests to test the new correct behaviour. I think @Quuxplusone's proposed change is more readable, but I don't object against your version.

Cast to the proper signedness that should fix the AArch and ARM builds.

Adds a required inline, fixes the build issue.

Remove an accidental edit, which caused the code to become invalid.

Nice catch! Can you rebase the patch to rerun the CI?

Reupload to fix the build breakage. The patch should be stand-alone but seems
to have an issue to apply the patch when not part of the series.

Updates the paper status.
Fixes no-except and no-localization unit tests.

Disable the formatter.const_char_array.pass.cpp on gcc 11 and apple-clang.

In D103369#2788352, @Quuxplusone wrote:

Well, my solution was to add this new header (and all new headers) to module.modulemap. So I assume this PR will fail buildkite as-is; it would need to add this header (and/or other new headers) to module.modulemap in order to keep buildkite happy. But I recommend people discuss the issue/solution(s) over on https://reviews.llvm.org/D102781#inline-979125 just to keep it all in the same place.

Rebase on top of D102781, see whether the CI likes this version better.

Reupload, forgot to set the dependencies so CI failed.

Disable the iostream in unit tests when localization is disabled. This should fix the no localization build.

Another attempt to fix the build.

• used #ifndef instead of #ifdef
• test whether the CI accepts Arthurs UNSUPPORTED helper

Fix CI issues:

• Remove an extra semi-column
• Remove clang::musttail attribute since not everywhere supported
• Disable debug iterators for a test copying an library string

Reworked the enitre patch.
Changed the dependency from D93593 to D103357 since the former no longer can be build on the CI.
There are still some minor changes planned, but want to test the CI build.

Fixes build for platforms without 128-bit support.

LGTM, but please wait for the reaction of @ldionne and @mclow.lists.

LGTM, modulo the remark.

Remove non-ASCII comment.

This commit seems to be the cause of the recent build failures of main. A local revert fixed the broken unit tests.