ADT: Make SmallVector::set_size() private
authorDuncan P. N. Exon Smith <dexonsmith@apple.com>
Tue, 7 Dec 2021 20:48:03 +0000 (12:48 -0800)
committerDuncan P. N. Exon Smith <dexonsmith@apple.com>
Thu, 13 Jan 2022 18:50:06 +0000 (10:50 -0800)
Stop allowing use of `SmallVectorBase::set_size()` outside of the
SmallVector implementation, which sets the size without calling
constructors or destructors.

Most callers should probably just use `resize()`. Or, if the new size is
guaranteed to be `<= size()`, then the new-ish `truncate()` works too
(and optimizes better).

Some callers want to avoid initializing memory before overwriting, but
need a pointer to the memory and so cannot use `push_back()`,
`emplace_back()`, or `append()`. Before this commit, this depended on
`reserve()` and `set_size()`:

```
V.reserve(V.size() + NumNew);      // Reserve expected size.
NumNew = initialize(V.end(), ...); // Get number added.
V.set_size(V.size() + NumNew);     // Set size to match.
```

Such code should be updated to use `resize_for_overwrite()` and
`truncate()`:

```
auto Size = V.size();                        // Save initial size.
V.resize_for_overwrite(Size + NumNew);       // Resize to expected size.
NumNew = initialize(V.begin() + Size, ...)); // Get number added.
V.truncate(Size + NumNew);                   // Truncate to match.
```

The new pattern is safe even for non-trivial types, since
`resize_for_overwrite()` calls constructors and `truncate()` calls
destructors. For trivial types, it should optimize the same way as the
old pattern.

Downstream code adapt to the disappearance of `set_size()` using this
new pattern should carefully audit uses of `V` between the resize and
the truncate:

- Change `V.size()` => `Size`.
- Change `V.capacity()` => `V.size()` (mostly).
- Change `V.end()` => `V.begin() + Size`.
- If `V` is an out-parameter, early returns need a `V.truncate()` or
  `V.clear()`. A scope exit is recommended.

Differential Revision: https://reviews.llvm.org/D115380

llvm/include/llvm/ADT/SmallVector.h

index 804567e..321546f 100644 (file)
@@ -72,15 +72,11 @@ public:
 
   LLVM_NODISCARD bool empty() const { return !Size; }
 
+protected:
   /// Set the array size to \p N, which the current array must have enough
   /// capacity for.
   ///
   /// This does not construct or destroy any elements in the vector.
-  ///
-  /// Clients can use this in conjunction with capacity() to write past the end
-  /// of the buffer when they know that more elements are available, and only
-  /// update the size later. This avoids the cost of value initializing elements
-  /// which will only be overwritten.
   void set_size(size_t N) {
     assert(N <= capacity());
     Size = N;
@@ -588,6 +584,9 @@ public:
   }
 
 private:
+  // Make set_size() private to avoid misuse in subclasses.
+  using SuperClass::set_size;
+
   template <bool ForOverwrite> void resizeImpl(size_type N) {
     if (N == this->size())
       return;