[libc-commits] [PATCH] D118954: [libc] add a vector internal class

[Michael Jones via Phabricator via libc-commits]

michaelrj added inline comments.


================
Comment at: libc/src/__support/CPP/vector.h:50
+  // undefined.
+  constexpr void resize(size_t new_size) {
+    if (new_size >= array_size)
----------------
sivachandra wrote:
> michaelrj wrote:
> > sivachandra wrote:
> > > I still don't understand the argument for having a resize method. Also, it is not inserting any elements for a size increase. So, it is different from `std::vector` behavior.
> > if you look on line 36 of vector_test.cpp you can see an example usage of this function. There needs to be some way of resizing this array other than push_back, since one of the intended use cases is writing out of order. I need some way of increasing the size (represented by num_elements) as well as the capacity (represented by array_size). If you have a better idea for how to do that please share it. If it would help I can rename this class so that there's no confusion with the `std::vector` class.
> OK. So what you want really is a `reserve` method? FWIW, `std::vector` has a reserve method and the only change you will need is to remove line 53 below.
While I could change this to reserve, that wouldn't solve the complete problem.

I'm going to explain the problem from the beginning because I think there's some confusion about how vectors handle sizes.

The important thing is that vectors track two sizes:
1) the exposed size of the vector, which is read from the `size` function, incremented by `push_back`, and changed with the `resize` function (called `num_elements` in my implementation).
2) the size of the underlying array, which is read from the `capacity` function, and changed with the `reserve` function (called `array_size` in my implementation).

The `reserve` function only affects the size of the underlying array, and doesn't affect the exposed size of the vector. While it is trivial to read/write beyond the exposed size of the vector (the `[]` operator has no bounds checking even in the `std::vector` implementation) it's undefined behavior. From the client's perspective, the exposed size of the vector //is// the size of the vector. The `resize` function can increase the exposed size of the vector, as well as increase the size of the underlying array to fit. The problem is that every function that increases the size of the exposed vector either writes a specific value (e.g. `resize(count, value)`, or `push_back(value)`) or is supposed to use a default-inserted element (e.g. `resize(count)`, `vector(count)`).

We need to track the exposed size of the vector because that's the number of actual values in the array, everything else is uninitialized garbage. In order to make sure we have a function that can do that, I've changed this `resize` function to take a value as well as a size, so we don't have to worry about default-inserting elements, just inserting copies of a specific element. I think this is the only practical solution to this problem without giving up on the `std::vector` spec in some capacity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118954