fmtlib / fmt

A modern formatting library
https://fmt.dev
Other
20.45k stars 2.45k forks source link

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

Open hartib opened 1 month ago

hartib commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

hartib commented 4 weeks ago

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 commented 2 weeks ago

@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