[Lldb-commits] [PATCH] D151597: [lldb][NFCI] Remove use of ConstString from IOHandler

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

In D151597#4384734 <https://reviews.llvm.org/D151597#4384734>, @mib wrote:

> TBH, it feels like a lot of - risky - changes just so `IOHandlerGetControlSequence` can return a `llvm::StringRef` ? Is that really necessary ?

I assume you're referring to `IOHandlerDelegateMultiline` storing its `m_end_line` member with a newline at the end now? My thought process was this: Pretty much every method changed here returns a constant string except for `IOHandlerDelegateMultiline::IOHandlerGetControlSequence`. That one creates a brand new string with `\n` at the end of it, so if `m_end_line` doesn't have a `\n` at the end of it, the we must always return a `std::string` which feels a little wasteful since the majority of these methods hand back a constant string. That also percolates up: `IOHandlerGetControlSequence` is the "lowest layer" of these call stacks, so whatever it returns is what the others must return as well.

The other option is to change `IOHandlerDelegateMultiline` to always take a string with a `\n`, but that is a lot harder to enforce. What do you think? I'm ok making whatever change makes this less risky in a separate commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151597