[Lldb-commits] [PATCH] D122848: Reduce extraneous temp strings in debugserver, free objects when they're not longer needed

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

In D122848#3420581 <https://reviews.llvm.org/D122848#3420581>, @JDevlieghere wrote:

> If debugserver linked against libSupport we could have saved the additional copy altogether by using `llvm::raw_string_ostream`:
>
>   std::string str;
>   llvm::raw_string_ostream stream(str);
>   stream.str() // Flushes and returns a reference to the stack allocated str
>
> Barring that, I wonder if if a little wrapper around `std::ostringstream` could improve readability and avoid bugs where someone forgets to call `stream.str("")`.
>
>   class AggressiveStream {
>   public:
>     AggressiveStream() : m_stream(std::make_unique<std::ostringstream>()) {}
>   
>     std::ostringstream &operator*() {
>       assert(m_stream && "cannot use stream after having called str()");
>       return *m_stream;
>     }
>   
>     std::string str() {
>       std::string s = m_stream->str();
>       m_stream.reset();
>       return std::move(s);
>     }
>   
>   private:
>     std::unique_ptr<std::ostringstream> m_stream;
>   };
>
> WDYT?

That's a cool idea, and certainly less error prone than this by-hand method I did, but I think I'd rather fix JSONGenerator::Dump to return quoted strings so we don't need to immediately copy the entire string into another string for quoting.  I don't feel strongly about it tho.  I might be too optimistic that I'm going to fix Dump and it would be good to adopt a safer wrapper around ostringsream.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122848