Skip to content

Commit d7b2646

Browse files
committed
Fix fmt::join for views with input iterators
Addresses issue (fmtlib#3802) For input_ranges (ie. a range with an input iterator backing it) we should not copy the iterator and should mutate the iterator on the view. For forward_ranges (or better), we can keep using them as const and make a copy of the iterator etc. This is an issue for code like: std::istringstream iss("1 2 3 4 5"); auto view = std::views::istream<int>(iss) fmt::join(std::move(view), ", ") Since the std::ranges::basic_istream_view::__iterator is not copyable And the struct formatter<join_view<It, Sentinel, Char>, Char> only has a template <typename FormatContext> auto format(const join_view<It, Sentinel, Char>& value, FormatContext& ctx) const -> decltype(ctx.out()) { auto it = value.begin; Which takes the value param by const ref and copies the iterator Fix is disabling the const ref format function overload when we have an input iterator passed to our join_view, and instead then just mutate the iterator in place and use a mutable join_view& overload for this case
1 parent 48c9084 commit d7b2646

File tree

2 files changed

+58
-2
lines changed

2 files changed

+58
-2
lines changed

include/fmt/ranges.h

+30-2
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,15 @@
1313
# include <iterator>
1414
# include <tuple>
1515
# include <type_traits>
16+
# include <utility>
1617
#endif
1718

1819
#include "format.h"
1920

21+
#if FMT_CPLUSPLUS >= 202002L
22+
# define FMT_USE_ITERATOR_CONCEPT
23+
#endif
24+
2025
FMT_BEGIN_NAMESPACE
2126

2227
namespace detail {
@@ -571,7 +576,7 @@ struct join_view : detail::view {
571576
basic_string_view<Char> sep;
572577

573578
join_view(It b, Sentinel e, basic_string_view<Char> s)
574-
: begin(b), end(e), sep(s) {}
579+
: begin(std::move(b)), end(e), sep(s) {}
575580
};
576581

577582
template <typename It, typename Sentinel, typename Char>
@@ -591,10 +596,33 @@ struct formatter<join_view<It, Sentinel, Char>, Char> {
591596
return value_formatter_.parse(ctx);
592597
}
593598

599+
#ifdef FMT_USE_ITERATOR_CONCEPT
600+
static constexpr bool is_input_iterator =
601+
std::input_iterator<It> && !std::forward_iterator<It>;
602+
#endif
603+
594604
template <typename FormatContext>
605+
#ifdef FMT_USE_ITERATOR_CONCEPT
606+
requires(!is_input_iterator)
607+
#endif
595608
auto format(const join_view<It, Sentinel, Char>& value,
596609
FormatContext& ctx) const -> decltype(ctx.out()) {
597610
auto it = value.begin;
611+
return format_impl(value, ctx, it);
612+
}
613+
614+
#ifdef FMT_USE_ITERATOR_CONCEPT
615+
template <typename FormatContext>
616+
requires is_input_iterator
617+
auto format(join_view<It, Sentinel, Char>& value,
618+
FormatContext& ctx) const -> decltype(ctx.out()) {
619+
return format_impl(value, ctx, value.begin);
620+
}
621+
#endif
622+
623+
template <typename FormatContext>
624+
auto format_impl(const join_view<It, Sentinel, Char>& value,
625+
FormatContext& ctx, It& it) const -> decltype(ctx.out()) {
598626
auto out = ctx.out();
599627
if (it != value.end) {
600628
out = value_formatter_.format(*it, ctx);
@@ -616,7 +644,7 @@ struct formatter<join_view<It, Sentinel, Char>, Char> {
616644
*/
617645
template <typename It, typename Sentinel>
618646
auto join(It begin, Sentinel end, string_view sep) -> join_view<It, Sentinel> {
619-
return {begin, end, sep};
647+
return {std::move(begin), end, sep};
620648
}
621649

622650
/**

test/ranges-test.cc

+28
Original file line numberDiff line numberDiff line change
@@ -623,3 +623,31 @@ struct lvalue_qualified_begin_end {
623623
TEST(ranges_test, lvalue_qualified_begin_end) {
624624
EXPECT_EQ(fmt::format("{}", lvalue_qualified_begin_end{}), "[1, 2, 3, 4, 5]");
625625
}
626+
627+
#if !defined(__cpp_lib_ranges) || __cpp_lib_ranges <= 202106L
628+
# define ENABLE_INPUT_RANGE_JOIN_TEST 0
629+
#elif FMT_CLANG_VERSION
630+
# if FMT_CLANG_VERSION > 1500
631+
# define ENABLE_INPUT_RANGE_JOIN_TEST 1
632+
#else
633+
# define ENABLE_INPUT_RANGE_JOIN_TEST 0
634+
# endif
635+
#else
636+
# define ENABLE_INPUT_RANGE_JOIN_TEST 1
637+
#endif
638+
639+
#if ENABLE_INPUT_RANGE_JOIN_TEST
640+
TEST(ranges_test, input_range_join) {
641+
std::istringstream iss("1 2 3 4 5");
642+
auto view = std::views::istream<std::string>(iss);
643+
auto joined_view = fmt::join(view.begin(), view.end(), ", ");
644+
EXPECT_EQ("1, 2, 3, 4, 5", fmt::format("{}", std::move(joined_view)));
645+
}
646+
647+
TEST(ranges_test, input_range_join_overload) {
648+
std::istringstream iss("1 2 3 4 5");
649+
EXPECT_EQ(
650+
"1.2.3.4.5",
651+
fmt::format("{}", fmt::join(std::views::istream<std::string>(iss), ".")));
652+
}
653+
#endif

0 commit comments

Comments
 (0)