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

Added formatter for bit_reference-like types #3570

Merged
merged 5 commits into from
Aug 6, 2023

Conversation

phprus
Copy link
Contributor

@phprus phprus commented Aug 4, 2023

Fix for issue #3567

Test is based on #3569

@phprus phprus marked this pull request as draft August 4, 2023 19:44
@phprus phprus marked this pull request as ready for review August 4, 2023 20:18
@phprus phprus marked this pull request as draft August 4, 2023 20:21
@phprus phprus marked this pull request as ready for review August 4, 2023 21:02
@phprus
Copy link
Contributor Author

phprus commented Aug 4, 2023

Final version.
Ready for review.

include/fmt/std.h Outdated Show resolved Hide resolved
@phprus phprus marked this pull request as draft August 5, 2023 08:29
@phprus phprus changed the title Added formatter for std::vector<bool>::reference Added formatter for bit_reference-like types Aug 5, 2023
@phprus
Copy link
Contributor Author

phprus commented Aug 5, 2023

@vitaut

Final-2 version :)
Ready for review.

@phprus phprus marked this pull request as ready for review August 5, 2023 09:46
include/fmt/std.h Outdated Show resolved Hide resolved
Copy link
Contributor

@vitaut vitaut left a comment

Choose a reason for hiding this comment

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

Thanks for the PR.

This looks like a lot of effort and implementation-specific code for such a corner case. I suggest just using std::addressoff and replacing FMT_CONSTEXPR with FMT_COSTEXPR20 in the value ctor overload that uses it. It only affects compile-time formatting anyway that requires a recent standard.

phprus and others added 2 commits August 5, 2023 22:13
Co-authored-by: Felix <[email protected]>
Signed-off-by: Vladislav Shchapov <[email protected]>
Signed-off-by: Vladislav Shchapov <[email protected]>
@phprus
Copy link
Contributor Author

phprus commented Aug 5, 2023

@vitaut
Done.

@wangzw
Copy link
Contributor

wangzw commented Aug 6, 2023

Works well for me. Thanks

include/fmt/std.h Outdated Show resolved Hide resolved
include/fmt/std.h Outdated Show resolved Hide resolved
Comment on lines 424 to 438
#ifdef _LIBCPP_VERSION

// Workaround for libc++ incompatibility with C++ standard.
// According to the Standard, `bitset::operator[] const` returns bool.
FMT_EXPORT
template <typename C, typename Char>
struct formatter<std::__bit_const_reference<C>, Char> : formatter<bool, Char> {
template <typename FormatContext>
FMT_CONSTEXPR20 auto format(const std::__bit_const_reference<C>& v,
FormatContext& ctx) const -> decltype(ctx.out()) {
return formatter<bool, Char>::format(v, ctx);
}
};

#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can be replaced by a specialization of is_bit_reference_like for __bit_const_reference which seems a bit cleaner (no duplicate formatter).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

phprus added 2 commits August 6, 2023 22:23
Signed-off-by: Vladislav Shchapov <[email protected]>
Signed-off-by: Vladislav Shchapov <[email protected]>
@phprus phprus requested a review from vitaut August 6, 2023 17:45
@vitaut vitaut merged commit aeb6ad4 into fmtlib:master Aug 6, 2023
@vitaut
Copy link
Contributor

vitaut commented Aug 6, 2023

Thank you!

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.

3 participants