[libcxx] Throw correct exception from std::vector::reserve

[miyuki]

Summary

According to the standard [vector.capacity]/5, std::vector<T>::reserve
shall throw an exception of type std::length_error when the requested
capacity exceeds max_size().

This behavior is not implemented correctly: the function 'reserve'
simply propagates the exception from allocator<T>::allocate. Before
D110846 that exception used to be of type std::length_error (which is
correct for vector<T>::reserve, but incorrect for
allocator<T>::allocate).

This patch fixes the issue and adds regression tests.

[ldionne]

libcxx/include/vector

1592

Do we need to fix this in vector<bool> too?

libcxx/test/libcxx/containers/sequences/vector.bool/reserve.pass.cpp

1

I assume you thought about it and there's no way to test this without relying on libc++ internal details? It would be great if there was a way to avoid that.

[• Quuxplusone]

libcxx/test/libcxx/containers/sequences/vector.bool/reserve.pass.cpp

33

I don't even think this use of an internal detail is correct. This should be std::vector<bool, limited_allocator<bool, max_cap_words> > — the allocator's value_type should be the value_type of the container. vector<bool> will rebind the allocator internally as an implementation detail, but the user doesn't see that and in fact isn't allowed to pass in a wrongly-typed allocator AFAIK — I think this code is "library UB" right now.

I think the answer is to pass in a limited_allocator<bool, 10>, and then try to v.reserve(10 * 64 * 100), which "ought to be big enough for anybody," even some hypothetical system where vector<bool>::__storage_type is 128-bit or 256-bit or 6399-bit. (Premise: I don't think we need to test that the happy path on line 43 doesn't throw. We already test that in plenty of places, right? But actually, even if we wanted to test that, v.reserve(10) should still hit the happy path.)

[miyuki]

1. Addressed reviewers' comments.
2. Added a prerequisite patch.

[• Quuxplusone]

LGTM now! (Of course now D112150 blocks this one: but hey, good, we fixed another bug too. :))

libcxx/test/std/containers/sequences/vector.bool/reserve.pass.cpp

67 ↗

(On Diff #380995)

reasonable

libcxx/include/vector

vector<_Tp, _Allocator>::at(size_type __n) const
     return this->__begin_[__n];
 }
 
 template <class _Tp, class _Allocator>
 void
 vector<_Tp, _Allocator>::reserve(size_type __n)
 {
     if (__n > capacity())
     {
+        if (__n > max_size())
+            this->__throw_length_error();
         allocator_type& __a = this->__alloc();
         __split_buffer<value_type, allocator_type&> __v(__n, size(), __a);
         __swap_out_circular_buffer(__v);
     }
 }
 
 template <class _Tp, class _Allocator>
 void

libcxx/test/libcxx/containers/sequences/vector.bool/reserve.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <vector>
+// vector<bool>
+
+// void reserve(size_type new_cap);
+
+#include <vector>
+#include <cassert>
+
+#include "test_macros.h"
+#include "test_allocator.h"
+
+int main(int, char**)
+{
+    typedef std::vector<bool>::__storage_type storage_type;
+    const unsigned bits_per_word
+        = static_cast<unsigned>(sizeof(storage_type) * CHAR_BIT);
+    // Maximum capacity in words (units of type storage_type)
+    const unsigned max_cap_words = 100;
+    // Maximum capacity in bits
+    const unsigned max_cap_bits = bits_per_word * max_cap_words;
+    // Half of the maximum capacity in bits
+    const unsigned half_cap_bits = max_cap_bits / 2;
+
+    {
+        std::vector<bool, limited_allocator<storage_type, max_cap_words> > v;
+        assert(v.capacity() == 0);
+        assert(v.size() == 0);
+        v.reserve(half_cap_bits);
+        assert(v.capacity() == half_cap_bits);
+        assert(v.size() == 0);
+    }
+#ifndef TEST_HAS_NO_EXCEPTIONS
+    {
+        std::vector<bool, limited_allocator<storage_type, max_cap_words> > v;
+        v.reserve(half_cap_bits);
+        assert(v.capacity() == half_cap_bits);
+        try {
+            v.reserve(max_cap_bits + 1);
+            assert(false);
+        } catch (const std::length_error&) {
+            // no-op
+        }
+        assert(v.capacity() == half_cap_bits);
+    }
+#endif
+
+    return 0;
+}

libcxx/test/std/containers/sequences/vector/vector.capacity/reserve.pass.cpp

#if TEST_STD_VER >= 11
         assert(v.size() == 100);
         assert(v.capacity() == 100);
         v.reserve(150);
         assert(v.size() == 100);
         assert(v.capacity() == 150);
         assert(is_contiguous_container_asan_correct(v));
     }
 #endif
+#ifndef TEST_HAS_NO_EXCEPTIONS
+    {
+        std::vector<int, limited_allocator<int, 100> > v;
+        v.reserve(50);
+        assert(v.capacity() == 50);
+        assert(is_contiguous_container_asan_correct(v));
+        try {
+            v.reserve(101);
+            assert(false);
+        } catch (const std::length_error&) {
+            // no-op
+        }
+        assert(v.capacity() == 50);
+        assert(is_contiguous_container_asan_correct(v));
+    }
+#endif
 
   return 0;
 }