[PATCH] D91693: [Support] Add reserve() method to the raw_ostream.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/include/llvm/Support/raw_ostream.h:634
+
+  void reserve(size_t Capacity) override { OS.reserve(Capacity); }
 };
----------------
avl wrote:
> dblaikie wrote:
> > Would it make sense for the reserve operation to be relative to where the stream has reached so far?
> > 
> > ie: for it to be implemented as:
> > ```
> > void reserve(size_t ExtraCapacity) override { OS.reserve(OS.size() + ExtraCapacity); }
> > ```
> > (similarly for other implementations)
> > Because the average caller probably shouldn't be thinking about how many bytes have already been written to the stream when deciding how much capacity they want for future write operations?
> I think - yes, it makes sense. In that case, it would probably be better to name that method reserveExtraSpace()(to look differently than std::vector::reserve()). 
> 
> Also, I think it should not be calculated against OS.size(). It looks better to calculate it against current_pos()(Because the callee can receive stream with the current position in the middle of the stream) 
> 
> Would update the review accordingly(if there are no objections).
Happy to use current_pos, though it looks like it's == size anyway, right?
```
uint64_t raw_svector_ostream::current_pos() const { return OS.size(); }
```
& raw_string_ostream's member:
```
  uint64_t current_pos() const override { return OS.size(); }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91693