Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc++][format] Formatting range with m range-type is incorrect #90196

Open
JMazurkiewicz opened this issue Apr 26, 2024 · 3 comments · May be fixed by #94562
Open

[libc++][format] Formatting range with m range-type is incorrect #90196

JMazurkiewicz opened this issue Apr 26, 2024 · 3 comments · May be fixed by #94562
Assignees
Labels
format C++20 std::format or std::print, and anything related to them libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@JMazurkiewicz
Copy link
Contributor

[format.range.formatter], Table 80:

(m option) Indicates that the opening bracket should be "{", the closing bracket should be "}", the separator should be ", ", and each range element should be formatted as if m were specified for its tuple-type.

Current implementation does not format range elements as if m tuple-type was specified, see for example https://godbolt.org/z/nzenfobqW:

#include <format>
#include <print>
#include <vector>
#include <utility>

int main() {
    std::vector<std::pair<int, int>> v = {{1, 2}, {3, 4}, {5, 6}};
    std::println("{:m}", v);
}

Expected:

{1: 2, 3: 4, 5: 6}

Got:

{(1, 2), (3, 4), (5, 6)}
@github-actions github-actions bot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Apr 26, 2024
@frederick-vs-ja
Copy link
Contributor

It seems that we should add

   __underlying_.set_separator(_LIBCPP_STATICALLY_WIDEN(_CharT, ": "));
   __underlying_.set_brackets({}, {});

to

if constexpr (__fmt_pair_like<_Tp>) {
set_brackets(_LIBCPP_STATICALLY_WIDEN(_CharT, "{"), _LIBCPP_STATICALLY_WIDEN(_CharT, "}"));
set_separator(_LIBCPP_STATICALLY_WIDEN(_CharT, ", "));
++__begin;
} else

@ldionne ldionne added the format C++20 std::format or std::print, and anything related to them label Apr 26, 2024
@mordante
Copy link
Member

Thanks for the report. I thought I had addressed this issue. I'll try to look at it soon.

@JMazurkiewicz
Copy link
Contributor Author

JMazurkiewicz commented Apr 26, 2024

It looks like someone tried to address this issue: #70616.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
format C++20 std::format or std::print, and anything related to them libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants