Skip to content

Conversation

JMazurkiewicz
Copy link
Contributor

  • Remove used by pair, tuple, (...) comment from _Fill_align_and_width_specs - it no longer serves its purpose.
  • Make _Fill_align_and_width_specs_setter<T>::_Specs member variable protected, so that we can create derived _Range_specs_setter.
  • Implement range_formatter:
    • range_formatter::format can be used at runtime only if format context's output iterator is back_insert_iterator<_Fmt_buffer<T>>. Creation of nested basic_format_context requires copying format args from old context<X> to context<Y> and X and Y have to be the same type to do this. This should not be a problem anyway, since basic_format_context can be constructed only by the standard library.
    • Towards P2286R8 Formatting Ranges #2919.

@JMazurkiewicz JMazurkiewicz requested a review from a team as a code owner April 29, 2024 19:58
@StephanTLavavej StephanTLavavej added ranges C++20/23 ranges format C++20/23 format cxx23 C++23 feature labels Apr 30, 2024
@StephanTLavavej StephanTLavavej self-assigned this Apr 30, 2024
@StephanTLavavej
Copy link
Member

Thanks, this is super awesome! 😻 I pushed a bunch of nitpicky changes, plus one bugfix and test coverage for it.

@StephanTLavavej StephanTLavavej removed their assignment May 29, 2024
@StephanTLavavej StephanTLavavej self-assigned this May 29, 2024
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej
Copy link
Member

I had to push an additional commit to dramatically reduce compiler memory consumption (and improve test throughput) by reducing the number of input ranges being tested.

Memory consumption was a severe problem for the MSVC-internal mirror for a couple of reasons:

  • MSVC-internal uses an x86-native compiler, unlike the GitHub PR/CI system which uses x64-hosted x86-targeting,
  • MSVC-internal uses a "checked" compiler with assertions enabled, which consumes slightly more memory.

Together, this imposed a 4 GB limit, and /analyze configurations (which are extremely memory-hungry) ran out of memory.

With my changes, MSVC-internal compiler memory consumption (as measured by /d1reportMemorySummary; that switch works for the public build too) decreased from 2280 MB to 395 MB for plain, and from 4GB+ to 718 MB for /analyze.

This also dramatically improved test throughput. On the GitHub side, with my physical machine and the x64-native compiler, test execution time dropped from 114 seconds to 18 seconds, a delicious 6.3x speedup.

@StephanTLavavej StephanTLavavej merged commit 0d32e40 into microsoft:main May 30, 2024
@StephanTLavavej
Copy link
Member

Thanks for this significant step in one of the last remaining C++23 features! 😻 📈 ⭐

@JMazurkiewicz JMazurkiewicz deleted the format/ranges branch May 30, 2024 18:09
@JMazurkiewicz JMazurkiewicz restored the format/ranges branch June 4, 2024 19:43
@JMazurkiewicz JMazurkiewicz deleted the format/ranges branch June 4, 2024 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cxx23 C++23 feature format C++20/23 format ranges C++20/23 ranges
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants