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

Alexey Lapshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 19 07:00:52 PST 2021


avl added inline comments.


================
Comment at: llvm/include/llvm/Support/raw_ostream.h:634
+
+  void reserve(size_t Capacity) override { OS.reserve(Capacity); }
 };
----------------
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).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91693



More information about the llvm-commits mailing list