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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
