[libcxx-commits] [PATCH] D111947: [Demangle] Rename OutputStream to OutputString

[David Blaikie via Phabricator via libcxx-commits]

dblaikie added inline comments.


================
Comment at: libcxxabi/src/demangle/Utility.h:27
 // has been parsed.
-class OutputStream {
+class OutputString {
   char *Buffer = nullptr;
----------------
ldionne wrote:
> dblaikie wrote:
> > ldionne wrote:
> > > This was arguably written with the intent of being a stream -- that's why the class provides functions like `operator<<`. On the other hand, it also provides some functions like `operator+=`, which don't really belong in a stream, but do belong in a string.
> > > 
> > > I feel like this patch as-is kind of breaks what this class was supposed to be, since now we'll have a type that is neither a string nor a stream, but has both `operator+=` and `operator<<`. Do we use `operator<<` widely? If not, would it make sense to remove it altogether and make this closer to a real string? Otherwise, perhaps we could consider naming this something like `OutputBuffer` instead, which IMO makes it less vexing to have an `operator<<` defined on.
> > > 
> > > I don't want to stall this solely on picking a name, but I do feel like having a class called `OutputString` yet boasting `operator<<` is really un-idiomatic and I'd like to explore alternatives.
> > OutputBuffer?
> Yeah, that was my suggestion above. I'm still not a big fan of it but I find it a bit less weird than `OutputString` given the existence of `operator<<`.
Ah, right, sorry, didn't see you'd already suggested that. @ljmf00 - could you make that change to `OutputBuffer`?




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

https://reviews.llvm.org/D111947