[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