Skip to content

Fix nested_format_specs buffer context construction - #4963

Closed
Tyagiquamar wants to merge 2 commits into
fmtlib:mainfrom
Tyagiquamar:fix-nested-format-specs-buffer-ctx
Closed

Tyagiquamar wants to merge 2 commits into
fmtlib:mainfrom
Tyagiquamar:fix-nested-format-specs-buffer-ctx

Conversation

@Tyagiquamar

Copy link
Copy Markdown

Problem

In nested_format_specs::write, buffer_ctx was constructed directly as FormatContext(basic_appender<Char>(buf), ctx.args(), ctx.locale()). When FormatContext is not parameterized with basic_appender<Char> (such as when custom contexts or different output iterators are passed, e.g. during compilation/custom formatters with nested formatted ranges like std::array or tuples), attempting to construct FormatContext directly with basic_appender<Char> leads to template instantiation and iterator mismatch errors (#4949).

Solution

  • Explicitly instantiate basic_format_context<basic_appender<Char>, Char> for the temporary buffer context in nested_format_specs::write().

Validation

  • Validated in Linux container with GCC 14:
    • Built and passed 100% of tests in ranges-test (42/42 tests) and compile-test (19/19 tests).

…mat_specs

Signed-off-by: Tyagiquamar <mohdquamartyagi@gmail.com>
@Tyagiquamar
Tyagiquamar requested a review from vitaut as a code owner October 1, 2026 11:46

@vitaut vitaut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to move the mismatch from the output iterator to the argument pack. ctx.args() has type basic_format_args<FormatContext>, while the new buffer context expects basic_format_args<basic_format_context<basic_appender<Char>, Char>>, and there is no conversion between those when FormatContext is a generic_context.

Could you add the exact #4949 reproducer to compile-test.cc? I believe this still fails to instantiate.

basic_format_args is tied to the context type, so a generic_context pack cannot construct a basic_appender context. Nested width formatting now buffers through a context that stores the original pack, and reuses a context that already writes with an appender.

Signed-off-by: Tyagiquamar <mohdquamartyagi@gmail.com>
@Tyagiquamar

Copy link
Copy Markdown
Author

Thanks, addressed the review feedback in 8f23dec.

  • Added the FMT_COMPILE fails to format std::array #4949 reproducer to compile-test.cc, including the width and dynamic element-spec cases. On the previous head it failed to instantiate: basic_format_args<generic_context<back_insert_iterator<string>, char>> does not convert to basic_format_args<context>.
  • Nested width formatting now buffers through a context that keeps basic_format_args of the caller. A context that already writes with basic_appender is reused, so a nested range does not wrap that argument pack again.

Validation:

  • Before the fix, compile-test failed to compile with GCC 13 (no known conversion for the argument pack).
  • After the fix, compile-test 20/20 pass and ranges-test 42/42 pass (Ubuntu 24.04, GCC 13.3, Debug).

Ready for another look.

@vitaut

vitaut commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Please don't copy other PRs. This just wastes everyone's time.

@vitaut vitaut closed this Oct 8, 2026
@Tyagiquamar

Tyagiquamar commented Oct 9, 2026 •

Copy link
Copy Markdown
Author

Hi Victor, I'd like to clarify the timeline, as I think there may have been a misunderstanding.

I originally opened #4963 on October 1, before #4974 was opened on October 5. My October 7 commit addressed the argument-pack mismatch you pointed out in your earlier review.

I understand that the final implementations overlap significantly, and I respect your decision to consolidate the work into one PR. However, I didn't intentionally copy another contributor's work, and I wouldn't want to leave that impression.

Thanks for your time reviewing this.

Verified chronology

PR | Author | Opened (UTC) | Status -- | -- | -- | -- #4951 | linsen458-spec | September 18, 2026 | Closed, unmerged #4963 | Tyagiquamar (mine) | October 1, 2026 | Closed, unmerged #4974 | mayankjaiswal12 | October 5, 2026 | Open
Verified chronology PR Author Opened (UTC) Status [#4951](https://github.com//pull/4951) linsen458-spec September 18, 2026 Closed, unmerged [#4963](https://github.com//pull/4963) Tyagiquamar (mine) October 1, 2026 Closed, unmerged [#4974](https://github.com//pull/4974) mayankjaiswal12 October 5, 2026 Open

@Tyagiquamar
Tyagiquamar deleted the fix-nested-format-specs-buffer-ctx branch October 9, 2026 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants