[libcxx-commits] [PATCH] D153405: [libc++] Add get_allocator and some constructor overloads of P0408R7 (Efficient Access to basic_stringbuf's Buffer)

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

In D153405#4446413 <https://reviews.llvm.org/D153405#4446413>, @pfusik wrote:

> In D153405#4446388 <https://reviews.llvm.org/D153405#4446388>, @Mordante wrote:
>
>> Sorry, but would it be possible to to a patch per class instead, starting with stringbuf? That way I can keep track that all classes are done. The subtile differences in all overloads make it really annoying the review for correctness.
>
> Would you like **all ** the changes in `stringbuf` in one patch? That's quite a lot.
> The remaining three classes are just simple wrappers over `stringbuf`.
> I thought it would be easier to split into features.

Yes I would like that, I've already reviewed all code changes in `stringbuf` so that would be faster for me to review again.

> Also, I expect that users don't typically use `stringbuf` directly, so having features exposed in `*stringstream` is what they care. That's how we proceeded with the already merged `view()`.

I agree, that most users don't care about these "internals". Still we need them to implement the "externals". I think the externals are smaller and can be reviewed a lot faster. It's mainly that I have limited time to work on libc++ and large complex patches are quite hard to find time for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153405