From d9fbff9afecd8d8b84545dd2b6ecde51ed44f40d Mon Sep 17 00:00:00 2001
From: Justin Riddell <arghnews@hotmail.co.uk>
Date: Wed, 1 May 2024 03:32:43 +0100
Subject: [PATCH] Fix fmt::join for views with input iterators

Addresses issue (#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
---
 include/fmt/ranges.h | 32 ++++++++++++++++++++++++++++++--
 test/ranges-test.cc  | 28 ++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/include/fmt/ranges.h b/include/fmt/ranges.h
index 0009ca43c7c9c..9c29bbca73968 100644
--- a/include/fmt/ranges.h
+++ b/include/fmt/ranges.h
@@ -13,10 +13,15 @@
 #  include <iterator>
 #  include <tuple>
 #  include <type_traits>
+#  include <utility>
 #endif
 
 #include "format.h"
 
+#if FMT_CPLUSPLUS >= 202002L
+#  define FMT_USE_ITERATOR_CONCEPT
+#endif
+
 FMT_BEGIN_NAMESPACE
 
 namespace detail {
@@ -571,7 +576,7 @@ struct join_view : detail::view {
   basic_string_view<Char> sep;
 
   join_view(It b, Sentinel e, basic_string_view<Char> s)
-      : begin(b), end(e), sep(s) {}
+      : begin(std::move(b)), end(e), sep(s) {}
 };
 
 template <typename It, typename Sentinel, typename Char>
@@ -591,10 +596,33 @@ struct formatter<join_view<It, Sentinel, Char>, Char> {
     return value_formatter_.parse(ctx);
   }
 
+#ifdef FMT_USE_ITERATOR_CONCEPT
+  static constexpr bool IsInputIterator =
+      std::input_iterator<It> && !std::forward_iterator<It>;
+#endif
+
   template <typename FormatContext>
+#ifdef FMT_USE_ITERATOR_CONCEPT
+    requires(!IsInputIterator)
+#endif
   auto format(const join_view<It, Sentinel, Char>& value,
               FormatContext& ctx) const -> decltype(ctx.out()) {
     auto it = value.begin;
+    return format_impl(value, ctx, it);
+  }
+
+#ifdef FMT_USE_ITERATOR_CONCEPT
+  template <typename FormatContext>
+    requires IsInputIterator
+  auto format(join_view<It, Sentinel, Char>& value,
+              FormatContext& ctx) const -> decltype(ctx.out()) {
+    return format_impl(value, ctx, value.begin);
+  }
+#endif
+
+  template <typename FormatContext>
+  auto format_impl(const join_view<It, Sentinel, Char>& value,
+                   FormatContext& ctx, It& it) const -> decltype(ctx.out()) {
     auto out = ctx.out();
     if (it != value.end) {
       out = value_formatter_.format(*it, ctx);
@@ -616,7 +644,7 @@ struct formatter<join_view<It, Sentinel, Char>, Char> {
  */
 template <typename It, typename Sentinel>
 auto join(It begin, Sentinel end, string_view sep) -> join_view<It, Sentinel> {
-  return {begin, end, sep};
+  return {std::move(begin), end, sep};
 }
 
 /**
diff --git a/test/ranges-test.cc b/test/ranges-test.cc
index 7905fd49d7a23..b977bf22bb6d9 100644
--- a/test/ranges-test.cc
+++ b/test/ranges-test.cc
@@ -623,3 +623,31 @@ struct lvalue_qualified_begin_end {
 TEST(ranges_test, lvalue_qualified_begin_end) {
   EXPECT_EQ(fmt::format("{}", lvalue_qualified_begin_end{}), "[1, 2, 3, 4, 5]");
 }
+
+#if !defined(__cpp_lib_ranges) || __cpp_lib_ranges <= 202106L
+#  define ENABLE_INPUT_RANGE_JOIN_TEST 0
+#elif FMT_CLANG_VERSION
+#  if FMT_CLANG_VERSION > 1500
+#    define ENABLE_INPUT_RANGE_JOIN_TEST 1
+#else
+#    define ENABLE_INPUT_RANGE_JOIN_TEST 0
+#  endif
+#else
+#  define ENABLE_INPUT_RANGE_JOIN_TEST 1
+#endif
+
+#if ENABLE_INPUT_RANGE_JOIN_TEST
+TEST(ranges_test, input_range_join) {
+  std::istringstream iss("1 2 3 4 5");
+  auto view = std::views::istream<std::string>(iss);
+  auto joined_view = fmt::join(view.begin(), view.end(), ", ");
+  EXPECT_EQ("1, 2, 3, 4, 5", fmt::format("{}", std::move(joined_view)));
+}
+
+TEST(ranges_test, input_range_join_overload) {
+  std::istringstream iss("1 2 3 4 5");
+  EXPECT_EQ(
+      "1.2.3.4.5",
+      fmt::format("{}", fmt::join(std::views::istream<std::string>(iss), ".")));
+}
+#endif