C++20 allows iterators for input ranges to be move-only, for example <code class="not

Fixed by <a class="issue-link js-issue-link" data-error-text="Failed to load titl

fmt::join doesn't handle move-only iterators about fmt HOT 8 OPEN

tcbrindle commented on June 8, 2024

fmt::join doesn't handle move-only iterators

from fmt.

Comments (8)

tcbrindle commented on June 8, 2024 1

Or is it unrelated?

This is a different to #3800

from fmt.

tcbrindle commented on June 8, 2024 1

Changing the signature of formatter<join_view>::format to

 template <typename FormatContext> 
 auto format(join_view<It, Sentinel, Char>& value, 
             FormatContext& ctx) const -> decltype(ctx.out())

(that is, removing the const from the join_view argument) generates the following from Clang:

/Users/tristan/Coding/fmt/include/fmt/base.h:1522:63: error: implicit instantiation of undefined template 'fmt::detail::type_is_unformattable_for<const fmt::join_view<unsigned char *, unsigned char *>, char>'
 1522 |     type_is_unformattable_for<T, typename Context::char_type> _;
      |                                                               ^
/Users/tristan/Coding/fmt/include/fmt/base.h:1908:20: note: in instantiation of function template specialization 'fmt::detail::make_arg<true, fmt::generic_context<std::back_insert_iterator<std::string>, char>, const fmt::join_view<unsigned char *, unsigned char *>, 0>' requested here
 1908 |   return {{detail::make_arg<NUM_ARGS <= detail::max_packed_args, Context>(
      |                    ^
/Users/tristan/Coding/fmt/include/fmt/compile.h:201:14: note: in instantiation of function template specialization 'fmt::make_format_args<fmt::generic_context<std::back_insert_iterator<std::string>, char>, const fmt::join_view<unsigned char *, unsigned char *>, 1UL, 0UL, 15ULL, 0>' requested here
  201 |         fmt::make_format_args<basic_format_context<OutputIt, Char>>(args...);
      |              ^
/Users/tristan/Coding/fmt/include/fmt/compile.h:438:6: note: in instantiation of function template specialization 'fmt::detail::spec_field<char, fmt::join_view<unsigned char *, unsigned char *>, 0>::format<std::back_insert_iterator<std::string>, fmt::join_view<unsigned char *, unsigned char *>>' requested here
  438 |   cf.format(std::back_inserter(s), args...);
      |      ^
/Users/tristan/Coding/fmt/include/fmt/compile.h:472:17: note: in instantiation of function template specialization 'fmt::format<fmt::detail::spec_field<char, fmt::join_view<unsigned char *, unsigned char *>, 0>, fmt::join_view<unsigned char *, unsigned char *>, char, 0>' requested here
  472 |     return fmt::format(compiled, std::forward<Args>(args)...);
      |                 ^
/Users/tristan/Coding/fmt/test/compile-test.cc:185:28: note: in instantiation of function template specialization 'fmt::format<FMT_COMPILE_STRING, fmt::join_view<unsigned char *, unsigned char *>, 0>' requested here
  185 |   EXPECT_EQ("0102af", fmt::format(FMT_COMPILE("{:02x}"), fmt::join(data, "")));
      |                            ^
/Users/tristan/Coding/fmt/include/fmt/base.h:1497:8: note: template is declared here
 1497 | struct type_is_unformattable_for;
      |        ^
/Users/tristan/Coding/fmt/include/fmt/base.h:1525:7: error: static assertion failed due to requirement 'formattable': Cannot format an argument. To make type T formattable provide a formatter<T> specialization: https://fmt.dev/latest/api.html#udt
 1525 |       formattable,
      |       ^~~~~~~~~~~
In file included from /Users/tristan/Coding/fmt/test/compile-test.cc:8:
/Users/tristan/Coding/fmt/include/fmt/compile.h:203:16: error: no matching member function for call to 'format'
  203 |     return fmt.format(get_arg_checked<T, N>(args...), ctx);
      |            ~~~~^~~~~~
/Users/tristan/Coding/fmt/include/fmt/compile.h:438:6: note: in instantiation of function template specialization 'fmt::detail::spec_field<char, fmt::join_view<unsigned char *, unsigned char *>, 0>::format<std::back_insert_iterator<std::string>, fmt::join_view<unsigned char *, unsigned char *>>' requested here
  438 |   cf.format(std::back_inserter(s), args...);
      |      ^
/Users/tristan/Coding/fmt/include/fmt/compile.h:472:17: note: in instantiation of function template specialization 'fmt::format<fmt::detail::spec_field<char, fmt::join_view<unsigned char *, unsigned char *>, 0>, fmt::join_view<unsigned char *, unsigned char *>, char, 0>' requested here
  472 |     return fmt::format(compiled, std::forward<Args>(args)...);
      |                 ^
/Users/tristan/Coding/fmt/test/compile-test.cc:185:28: note: in instantiation of function template specialization 'fmt::format<FMT_COMPILE_STRING, fmt::join_view<unsigned char *, unsigned char *>, 0>' requested here
  185 |   EXPECT_EQ("0102af", fmt::format(FMT_COMPILE("{:02x}"), fmt::join(data, "")));
      |                            ^
/Users/tristan/Coding/fmt/include/fmt/ranges.h:588:8: note: candidate function template not viable: 1st argument ('const fmt::join_view<unsigned char *, unsigned char *>') would lose const qualifier
  588 |   auto format(join_view<It, Sentinel, Char>& value,
      |        ^      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3 errors generated.

From the last line, it seems that something is trying to pass a const join_view to the format function, but I'm not sure where that call is coming from

from fmt.

vitaut commented on June 8, 2024

~~Fixed by #3800 (thanks).~~

from fmt.

vitaut commented on June 8, 2024

Or is it unrelated?

from fmt.

tcbrindle commented on June 8, 2024

There are a couple of places in the join_view constructor where iterators are copied, but these are easily solved with std::move. The difficulty is with formatter<join_view>::format():

fmt/include/fmt/ranges.h

Lines 587 to 603 in 810d175

 template <typename FormatContext> 

 auto format(const join_view<It, Sentinel, Char>& value, 

 FormatContext& ctx) const -> decltype(ctx.out()) { 

 auto it = value.begin; 

 auto out = ctx.out(); 

 if (it != value.end) { 

 out = value_formatter_.format(*it, ctx); 

 ++it; 

 while (it != value.end) { 

 out = detail::copy_str<Char>(value.sep.begin(), value.sep.end(), out); 

 ctx.advance_to(out); 

 out = value_formatter_.format(*it, ctx); 

 ++it; 

 } 

 } 

 return out; 

 }

Line 590 takes a copy of the iterator, which it then modifies. Because the join_view is passed to this function as a const reference, there's no way we can move the iterator out of it -- and we also can't directly operate on the stored iterator, as it's const.

It seems like we need to take the join_view argument to format() by mutable & rather than const&, but making this change results in fmt considering join_view to be non-formattable. Unfortunately I don't know enough about the library to understand why that happens. I also tried using a forwarding reference, but this always deduces to const& and so doesn't help.

The generic range support without join (i.e. fmt::format("{}", my_range)) handles move-only iterators just fine, so this definitely seems like it should be solvable, but I don't understand enough about the fmt internals to follow what's going on.

from fmt.

vitaut commented on June 8, 2024

It seems like we need to take the join_view argument to format() by mutable & rather than const&, but making this change results in fmt considering join_view to be non-formattable.

This should work. What error do you get?

from fmt.

tcbrindle commented on June 8, 2024

GCC gives a little more context: an error occurs in the return value of spec_field::format:

fmt/include/fmt/compile.h

Lines 197 to 204 in 3c96084

 template <typename OutputIt, typename... Args> 

 constexpr FMT_INLINE OutputIt format(OutputIt out, 

 const Args&... args) const { 

 const auto& vargs = 

 fmt::make_format_args<basic_format_context<OutputIt, Char>>(args...); 

 basic_format_context<OutputIt, Char> ctx(out, vargs); 

 return fmt.format(get_arg_checked<T, N>(args...), ctx); 

 }

Here, get_arg_checked<T, N>(args...) returns const join_view&, which causes the error as it cannot be passed to the join_view's modified format function. get_checked_arg says

fmt/include/fmt/compile.h

Lines 128 to 130 in 3c96084

 // This ensures that the argument type is convertible to `const T&`. 

 template <typename T, int N, typename... Args> 

 constexpr const T& get_arg_checked(const Args&... args) {

so I guess adding the const& is intentional, but I don't get why this triggers an error when using join and not with plain fmt::format("{}", my_range)?

from fmt.

vitaut commented on June 8, 2024

Looks like we need to propagate (lack of) constness during format string compilation. Currently it passes everything by const&.

from fmt.

fmt::join doesn't handle move-only iterators about fmt HOT 8 OPEN

Comments (8)

Related Issues (20)

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent

	template <typename FormatContext>
	auto format(const join_view<It, Sentinel, Char>& value,
	FormatContext& ctx) const -> decltype(ctx.out()) {
	auto it = value.begin;
	auto out = ctx.out();
	if (it != value.end) {
	out = value_formatter_.format(*it, ctx);
	++it;
	while (it != value.end) {
	out = detail::copy_str<Char>(value.sep.begin(), value.sep.end(), out);
	ctx.advance_to(out);
	out = value_formatter_.format(*it, ctx);
	++it;
	}
	}
	return out;
	}

	template <typename OutputIt, typename... Args>
	constexpr FMT_INLINE OutputIt format(OutputIt out,
	const Args&... args) const {
	const auto& vargs =
	fmt::make_format_args<basic_format_context<OutputIt, Char>>(args...);
	basic_format_context<OutputIt, Char> ctx(out, vargs);
	return fmt.format(get_arg_checked<T, N>(args...), ctx);
	}

	// This ensures that the argument type is convertible to `const T&`.
	template <typename T, int N, typename... Args>
	constexpr const T& get_arg_checked(const Args&... args) {