[libcxx-commits] [PATCH] D112068: [libcxx] Throw correct exception from std::vector::reserve

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/test/libcxx/containers/sequences/vector.bool/reserve.pass.cpp:33
+    {
+        std::vector<bool, limited_allocator<storage_type, max_cap_words> > v;
+        assert(v.capacity() == 0);
----------------
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.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112068/new/

https://reviews.llvm.org/D112068