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

Cannot suppress range formatter when type has a container_type member type. #4123

Open
hartib opened this issue Aug 16, 2024 · 4 comments
Open

Comments

@hartib
Copy link

hartib commented Aug 16, 2024

The enabling condition of the adaptor formatter around line 838 of ranges.h is:

enable_if_t<conjunction<detail::is_container_adaptor_like<T>,
                        bool_constant<range_format_kind<T, Char>::value ==
                                      range_format::disabled>>::value>>

This is probably supposed to be:

enable_if_t<conjunction<detail::is_container_adaptor_like<T>,
                        bool_constant<range_format_kind<T, Char>::value !=
                                      range_format::disabled>>::value>>

So that the formatter may be disabled via a is_range specialization.

See compilation error in https://godbolt.org/z/Kq33exr6x

@hartib
Copy link
Author

hartib commented Aug 16, 2024

Unfortunately this change leads to a failure in ranges-test.cc because there are now two partial specializations that kick in: one in line 517 and one in line 829. The problem may be that the flat_set in the test code has both a container_type member and begin/end methods (just a guess). I cannot figure out, how the enabling of the different ranges formatters is intended to work.

@vitaut
Copy link
Contributor

vitaut commented Aug 16, 2024

Could you elaborate why you want to disable the container adaptor formatter?

@hartib
Copy link
Author

hartib commented Aug 19, 2024

We are using several libraries that parse and build communication messages (IP, DVB signaling, CCSDS). These libraries use non-owning views on containers (std::vector, c or c++ arrays, user defined vector-like containers). A library usually has one or two base template classes that handle the protocol basics (like headers) and are templated on the container type. The usually define a member type container_type and they have begin() and end() members so they can be accessed like arrays. From these base templates usually two class templates are derived per message - one parser and one builder template. Then we have a partial specialization of fmt::formatter that is enabled when a base class is detected. These formatters provide the format string parsers and instantiate and call special purpose formatters that the can print the messages. All in all we probably have more than 300 of these templated views.

The first problem we had when including ranges.h is that the ranges formatter kicks in because of the begin() and end() members. This leads to having two partial specializations - the one from ranges.h and ours that should print the message. This can be disabled by defining the is_range specialization to be false.

The second problem is that the adapter specialization also kicks in because of the container_type member. I though that this could be disabled by the change I proposed, but it turns out that range_format_kind equals disabled for adapters and cannot be used to disable the adapter formatter.

I think it would be useful to have a way to disable the adapter formatters for user defined type like one can disable the range formatter. The intention is not to disable the adapter formatters in general - this effect came form my misunderstanding of the code.

@Arghnews
Copy link
Contributor

Arghnews commented Aug 28, 2024

@hartib if you change to libstdc++ you get

/opt/compiler-explorer/gcc-13.2.0/lib/gcc/x86_64-linux-gnu/13.2.0/../../../../include/c++/13.2.0/type_traits:1048:21: error: static assertion failed due to requirement 'std::__is_complete_or_unbounded(std::__type_identity<fmt::formatter<Foo<std::array<int, 3>>, char, void>>{})': template argument must be a complete class or an unbounded array
 1048 |       static_assert(std::__is_complete_or_unbounded(__type_identity<_Tp>{}),
      |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/compiler-explorer/libs/fmt/trunk/include/fmt/base.h:1531:39: note: in instantiation of template class 'std::is_constructible<fmt::formatter<Foo<std::array<int, 3>>>>' requested here
 1531 |                                      (has_formatter<U, Context>::value &&

Before that.
is_constructible says you must have a complete type else UB

I must admit, I'm not fully sure if libstdc++ or libc++ (which seems to accept the code as fine, though maybe it's UB with no diagnostic) is correct here? Would need to fix this too then, in user code and/or fmtlib

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

No branches or pull requests

3 participants