@llvm/pr-subscribers-libcxx

Author: Alex Guteniev (AlexGuteniev)

diff --git a/libcxx/test/support/MinSequenceContainer.h b/libcxx/test/support/MinSequenceContainer.h
index 6e61aff06344b..b17ced6dfc2ff 100644
--- a/libcxx/test/support/MinSequenceContainer.h
+++ b/libcxx/test/support/MinSequenceContainer.h
@@ -28,6 +28,13 @@ struct MinSequenceContainer {
   template <class It>
   explicit MinSequenceContainer(It first, It last) : data_(first, last) {}
   MinSequenceContainer(std::initializer_list<T> il) : data_(il) {}
+
+  template <class It>
+  void assign(It first, It last) {
+    data_.assign(first, last);
+  }
+  void assign(std::initializer_list<T> il) { data_.assign(il); }
+  void assign(size_type n, value_type t) { data_.assign(n, t); }
   iterator begin() { return iterator(data_.data()); }
   const_iterator begin() const { return const_iterator(data_.data()); }
   const_iterator cbegin() const { return const_iterator(data_.data()); }
@@ -47,6 +54,8 @@ struct MinSequenceContainer {
     return from_vector_iterator(data_.insert(to_vector_iterator(p), std::move(value)));
   }
 
+  iterator insert_range(const_iterator p, auto rg) { return data_.insert_range((to_vector_iterator(p), rg); }
+
   iterator erase(const_iterator first, const_iterator last) {
     return from_vector_iterator(data_.erase(to_vector_iterator(first), to_vector_iterator(last)));
   }

✅ With the latest revision this PR passed the C/C++ code formatter.

thanks for the fix. actually i made a commit recently which relies on this helper type's absence of insert_range
4b104c6#diff-0c32ebccc1456e2143518d2ce055db04535240cab1881f8f3412362f8ae22521

this means i need to refactor the test not to use MinSenquenceContainer to make CI green.

actually i made a commit recently which relies on this helper type's absence of insert_range

Maybe we can solve this with prerocessor then

Maybe we can solve this with prerocessor then

Or maybe we should consider supporting this too.

@StephanTLavavej , please confirm if supporting pre-C++23 containers in <flat_set> is non-goal for MSVC STL

@AlexGuteniev I fixed my tests not relying on MinSequenceContainer
#140372

please confirm if supporting pre-C++23 containers in <flat_set> is non-goal for MSVC STL

I'd say yes, that's a non-goal. It seems to me that if someone wants to use a new container adaptor, they can be expected to provide a sufficiently modern container.

@AlexGuteniev I fixed my tests not relying on MinSequenceContainer

Thanks, but I don't think it is a path forward. The goal of my PR is to make the tests, including yours, pass with MSVC STL, so with your fix my PR does not make sense.

I think we should instead use preprocessor and make insert_range unavailable when _LIBCPP_VERSION is defined. This matches the practice of LIBCPP_ASSERT macro.

@AlexGuteniev I fixed my tests not relying on MinSequenceContainer

Thanks, but I don't think it is a path forward. The goal of my PR is to make the tests, including yours, pass with MSVC STL, so with your fix my PR does not make sense.

I think we should instead use preprocessor and make insert_range unavailable when _LIBCPP_VERSION is defined. This matches the practice of LIBCPP_ASSERT macro.

No I don't think you should guard against libc++. Your original fix looks good to me. the helper type in the test/std folder is meant to be used for all implementations. not just libc++.

Anything that is libc++ specific is our extension and should be put into test/libcxx folder. and this is what I was doing in #140372

No I don't think you should guard against libc++. Your original fix looks good to me.

Ok, will revert that

btw, the CI looks OK to me. the msan failure has nothing to do with your patch

MemorySanitizer: CHECK failed: msan_linux.cpp:193 "((personality(old_personality | ADDR_NO_RANDOMIZE))) != ((-1))" (0xffffffffffffffff, 0xffffffffffffffff) (tid=5352)

This failure seems happen all the time now.