[libc++] Fix flat_{multi}set insert_range

[huixie90]

fixes #136656

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Hui (huixie90)

Changes

fixes #136656

---

Full diff: https://github.com/llvm/llvm-project/pull/137462.diff

3 Files Affected:

• (modified) libcxx/include/__flat_set/utils.h (+3-1)
• (modified) libcxx/test/std/containers/container.adaptors/flat.multiset/flat.multiset.modifiers/insert_range.pass.cpp (+10)
• (modified) libcxx/test/std/containers/container.adaptors/flat.set/flat.set.modifiers/insert_range.pass.cpp (+10)

diff --git a/libcxx/include/__flat_set/utils.h b/libcxx/include/__flat_set/utils.h
index ed3b4c48580fb..542bfd886aef5 100644
--- a/libcxx/include/__flat_set/utils.h
+++ b/libcxx/include/__flat_set/utils.h
@@ -11,6 +11,7 @@
 #define _LIBCPP___FLAT_SET_UTILS_H
 
 #include <__config>
+#include <__iterator/iterator_traits.h>
 #include <__ranges/access.h>
 #include <__ranges/concepts.h>
 #include <__type_traits/container_traits.h>
@@ -60,7 +61,8 @@ struct __flat_set_utils {
       // C++23 Sequence Container should have insert_range member function
       // Note that not all Sequence Containers provide append_range.
       __set.__keys_.insert_range(__set.__keys_.end(), std::forward<_Range>(__rng));
-    } else if constexpr (ranges::common_range<_Range>) {
+    } else if constexpr (ranges::common_range<_Range> &&
+                         __has_input_iterator_category<ranges::iterator_t<_Range>>::value) {
       __set.__keys_.insert(__set.__keys_.end(), ranges::begin(__rng), ranges::end(__rng));
     } else {
       for (auto&& __x : __rng) {
diff --git a/libcxx/test/std/containers/container.adaptors/flat.multiset/flat.multiset.modifiers/insert_range.pass.cpp b/libcxx/test/std/containers/container.adaptors/flat.multiset/flat.multiset.modifiers/insert_range.pass.cpp
index 566be3921bf77..2f6a68fa0581a 100644
--- a/libcxx/test/std/containers/container.adaptors/flat.multiset/flat.multiset.modifiers/insert_range.pass.cpp
+++ b/libcxx/test/std/containers/container.adaptors/flat.multiset/flat.multiset.modifiers/insert_range.pass.cpp
@@ -18,6 +18,7 @@
 #include <flat_set>
 #include <functional>
 #include <ranges>
+#include <sstream>
 #include <vector>
 
 #include "MinSequenceContainer.h"
@@ -85,6 +86,15 @@ void test() {
     MoveOnly expected[] = {1, 1, 3, 4, 5};
     assert(std::ranges::equal(m, expected));
   }
+  {
+    // https://github.com/llvm/llvm-project/issues/136656
+    MinSequenceContainer<int> v;
+    std::flat_multiset s(v);
+    std::istringstream ints("0 1 1 0");
+    auto r = std::ranges::subrange(std::istream_iterator<int>(ints), std::istream_iterator<int>()) |
+             std::views::transform([](int i) { return i * i; });
+    s.insert_range(r);
+  }
 }
 
 void test_exception() {
diff --git a/libcxx/test/std/containers/container.adaptors/flat.set/flat.set.modifiers/insert_range.pass.cpp b/libcxx/test/std/containers/container.adaptors/flat.set/flat.set.modifiers/insert_range.pass.cpp
index 966a696c096b9..3b53ca255ceeb 100644
--- a/libcxx/test/std/containers/container.adaptors/flat.set/flat.set.modifiers/insert_range.pass.cpp
+++ b/libcxx/test/std/containers/container.adaptors/flat.set/flat.set.modifiers/insert_range.pass.cpp
@@ -18,6 +18,7 @@
 #include <flat_set>
 #include <functional>
 #include <ranges>
+#include <sstream>
 #include <vector>
 
 #include "MinSequenceContainer.h"
@@ -96,6 +97,15 @@ void test() {
     MoveOnly expected[] = {1, 3, 4, 5};
     assert(std::ranges::equal(m, expected));
   }
+  {
+    // https://github.com/llvm/llvm-project/issues/136656
+    MinSequenceContainer<int> v;
+    std::flat_set s(v);
+    std::istringstream ints("0 1 1 0");
+    auto r = std::ranges::subrange(std::istream_iterator<int>(ints), std::istream_iterator<int>()) |
+             std::views::transform([](int i) { return i * i; });
+    s.insert_range(r);
+  }
 }
 
 void test_exception() {

[frederick-vs-ja]

No (functional) change requested. Do we want to say the fallback branches are (pedantically) extensions?

[frederick-vs-ja]

On the other hand, it seems that the missing parts of MinSequenceContainer for C++23 sequence container requirements "silently" enforced our flat_meow to contain some extensions.

[huixie90]

added static_assert in the test case

[frederick-vs-ja]

Ah, I guess these cases should be moved to the libcxx/test/libcxx subdirectory.

[huixie90]

Thanks. moved the tests. leave this open for others to put their opinions on this "extension"

[ldionne]

LGTM but I'd like a LWG issue about it.

[ldionne]

Per discussion just now, I do see @huixie90 's point of view where this isn't really a bug in the Standard per se, so no LWG issue is required. LGTM with comments addressed.