[lldb][NFCI] Remove use of ConstString from IOHandler

[bulbazord]

Summary

None of these need to be in the ConstString StringPool. For the most
part they are constant strings and do not require fast comparisons.

I did change IOHandlerDelegateMultiline slightly -- specifically, the
m_end_line member always has a \n at the end of it now. This was so
that IOHandlerGetControlSequence can always return a StringRef. This
did require a slight change to IOHandlerIsInputComplete where we must
drop the newline before comparing it against the input parameter.

[mib]

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

[bulbazord]

In 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.

[jingham]

I don't think we need to support the case where the "end line token" doesn't have to stand on its own line. It's reasonable for lldb to impose this policy, and if we are going to do that then requiring the "\n" after the end line token also seems odd. The way this is done seems okay to me.

[JDevlieghere]

lldb/include/lldb/Core/IOHandler.h

110

I would return {} or return "".

297–298

Can we make end_line a StringRef?

[bulbazord]

Address Jonas's comments

[bulbazord]

Address the other suggestion

lldb/include/lldb/Core/Debugger.h

public:
 
   bool IsTopIOHandler(const lldb::IOHandlerSP &reader_sp);
 
   bool CheckTopIOHandlerTypes(IOHandler::Type top_type,
                               IOHandler::Type second_top_type);
 
   void PrintAsync(const char *s, size_t len, bool is_stdout);
 
-  ConstString GetTopIOHandlerControlSequence(char ch);
+  llvm::StringRef GetTopIOHandlerControlSequence(char ch);
 
   const char *GetIOHandlerCommandPrefix();
 
   const char *GetIOHandlerHelpPrologue();
 
   void ClearIOHandlers();
 
   bool GetCloseInputOnEOF() const;

lldb/include/lldb/Core/IOHandler.h

 //===-- IOHandler.h ---------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_CORE_IOHANDLER_H
 #define LLDB_CORE_IOHANDLER_H
 
 #include "lldb/Core/ValueObjectList.h"
 #include "lldb/Host/Config.h"
 #include "lldb/Utility/CompletionRequest.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Flags.h"
 #include "lldb/Utility/Predicate.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-forward.h"
 #include "llvm/ADT/StringRef.h"
 
public:
   }
 
   virtual bool SetPrompt(llvm::StringRef prompt) {
     // Prompt support isn't mandatory
     return false;
   }
   bool SetPrompt(const char *) = delete;
 
-  virtual ConstString GetControlSequence(char ch) { return ConstString(); }
+  virtual llvm::StringRef GetControlSequence(char ch) { return {}; }
 
   virtual const char *GetCommandPrefix() { return nullptr; }
 
   virtual const char *GetHelpPrologue() { return nullptr; }
 
   int GetInputFD();
 
   int GetOutputFD();
 
public:
   ///     false indicates that more input is required.
   virtual bool IOHandlerIsInputComplete(IOHandler &io_handler,
                                         StringList &lines) {
     // Impose no requirements for input to be considered complete.  subclasses
     // should do something more intelligent.
     return true;
   }
 
-  virtual ConstString IOHandlerGetControlSequence(char ch) {
-    return ConstString();
-  }
+  virtual llvm::StringRef IOHandlerGetControlSequence(char ch) { return {}; }
 
   virtual const char *IOHandlerGetCommandPrefix() { return nullptr; }
 
   virtual const char *IOHandlerGetHelpPrologue() { return nullptr; }
 
   // Intercept the IOHandler::Interrupt() calls and do something.
   //
   // Return true if the interrupt was handled, false if the IOHandler should
   // continue to try handle the interrupt itself.
   virtual bool IOHandlerInterrupt(IOHandler &io_handler) { return false; }
 
 protected:
   Completion m_completion; // Support for common builtin completions
 };
 
 // IOHandlerDelegateMultiline
 //
 // A IOHandlerDelegate that handles terminating multi-line input when
 // the last line is equal to "end_line" which is specified in the constructor.
 class IOHandlerDelegateMultiline : public IOHandlerDelegate {
 public:
-  IOHandlerDelegateMultiline(const char *end_line,
+  IOHandlerDelegateMultiline(llvm::StringRef end_line,
                              Completion completion = Completion::None)
-      : IOHandlerDelegate(completion),
-        m_end_line((end_line && end_line[0]) ? end_line : "") {}
+      : IOHandlerDelegate(completion), m_end_line(end_line.str() + "\n") {}
 
   ~IOHandlerDelegateMultiline() override = default;
 
-  ConstString IOHandlerGetControlSequence(char ch) override {
+  llvm::StringRef IOHandlerGetControlSequence(char ch) override {
     if (ch == 'd')
-      return ConstString(m_end_line + "\n");
-    return ConstString();
+      return m_end_line;
+    return {};
   }
 
   bool IOHandlerIsInputComplete(IOHandler &io_handler,
                                 StringList &lines) override {
     // Determine whether the end of input signal has been entered
     const size_t num_lines = lines.GetSize();
-    if (num_lines > 0 && lines[num_lines - 1] == m_end_line) {
+    const llvm::StringRef end_line =
+        llvm::StringRef(m_end_line).drop_back(1); // Drop '\n'
+    if (num_lines > 0 && llvm::StringRef(lines[num_lines - 1]) == end_line) {
       // Remove the terminal line from "lines" so it doesn't appear in the
       // resulting input and return true to indicate we are done getting lines
       lines.PopBack();
       return true;
     }
     return false;
   }
 
public:
   void GotEOF() override;
 
   void Activate() override;
 
   void Deactivate() override;
 
   void TerminalSizeChanged() override;
 
-  ConstString GetControlSequence(char ch) override {
+  llvm::StringRef GetControlSequence(char ch) override {
     return m_delegate.IOHandlerGetControlSequence(ch);
   }
 
   const char *GetCommandPrefix() override {
     return m_delegate.IOHandlerGetCommandPrefix();
   }
 
   const char *GetHelpPrologue() override {
bool CheckTopIOHandlerTypes(IOHandler::Type top_type,
                               IOHandler::Type second_top_type) {
     std::lock_guard<std::recursive_mutex> guard(m_mutex);
     const size_t num_io_handlers = m_stack.size();
     return (num_io_handlers >= 2 &&
             m_stack[num_io_handlers - 1]->GetType() == top_type &&
             m_stack[num_io_handlers - 2]->GetType() == second_top_type);
   }
 
-  ConstString GetTopIOHandlerControlSequence(char ch) {
-    return ((m_top != nullptr) ? m_top->GetControlSequence(ch) : ConstString());
+  llvm::StringRef GetTopIOHandlerControlSequence(char ch) {
+    return ((m_top != nullptr) ? m_top->GetControlSequence(ch)
+                               : llvm::StringRef());
   }
 
   const char *GetTopIOHandlerCommandPrefix() {
     return ((m_top != nullptr) ? m_top->GetCommandPrefix() : nullptr);
   }
 
   const char *GetTopIOHandlerHelpPrologue() {
     return ((m_top != nullptr) ? m_top->GetHelpPrologue() : nullptr);

lldb/include/lldb/Expression/REPL.h

public:
 
   bool IOHandlerInterrupt(IOHandler &io_handler) override;
 
   void IOHandlerInputInterrupted(IOHandler &io_handler,
                                  std::string &line) override;
 
   const char *IOHandlerGetFixIndentationCharacters() override;
 
-  ConstString IOHandlerGetControlSequence(char ch) override;
+  llvm::StringRef IOHandlerGetControlSequence(char ch) override;
 
   const char *IOHandlerGetCommandPrefix() override;
 
   const char *IOHandlerGetHelpPrologue() override;
 
   bool IOHandlerIsInputComplete(IOHandler &io_handler,
                                 StringList &lines) override;
 

lldb/include/lldb/Interpreter/CommandInterpreter.h

protected:
   // This checks just the RunCommandInterpreter interruption state.  It is only
   // meant to be used in Debugger::InterruptRequested
   bool WasInterrupted() const;
 
   // IOHandlerDelegate functions
   void IOHandlerInputComplete(IOHandler &io_handler,
                               std::string &line) override;
 
-  ConstString IOHandlerGetControlSequence(char ch) override {
+  llvm::StringRef IOHandlerGetControlSequence(char ch) override {
+    static constexpr llvm::StringLiteral control_sequence("quit\n");
     if (ch == 'd')
-      return ConstString("quit\n");
-    return ConstString();
+      return control_sequence;
+    return {};
   }
 
   void GetProcessOutput();
 
   bool DidProcessStopAbnormally() const;
 
   void SetSynchronous(bool value);
 

lldb/source/API/SBCommandInterpreter.cpp

bool SBCommandInterpreter::InterruptCommand() {
   LLDB_INSTRUMENT_VA(this);
   
   return (IsValid() ? m_opaque_ptr->InterruptCommand() : false);
 }
 
 const char *SBCommandInterpreter::GetIOHandlerControlSequence(char ch) {
   LLDB_INSTRUMENT_VA(this, ch);
 
-  return (IsValid()
-              ? m_opaque_ptr->GetDebugger()
-                    .GetTopIOHandlerControlSequence(ch)
-                    .GetCString()
-              : nullptr);
+  if (!IsValid())
+    return nullptr;
+
+  return ConstString(
+             m_opaque_ptr->GetDebugger().GetTopIOHandlerControlSequence(ch))
+      .GetCString();
 }
 
 lldb::ReturnStatus
 SBCommandInterpreter::HandleCommand(const char *command_line,
                                     SBCommandReturnObject &result,
                                     bool add_to_history) {
   LLDB_INSTRUMENT_VA(this, command_line, result, add_to_history);
 

lldb/source/Core/Debugger.cpp

void Debugger::PrintAsync(const char *s, size_t len, bool is_stdout) {
   bool printed = m_io_handler_stack.PrintAsync(s, len, is_stdout);
   if (!printed) {
     lldb::StreamFileSP stream =
         is_stdout ? m_output_stream_sp : m_error_stream_sp;
     stream->Write(s, len);
   }
 }
 
-ConstString Debugger::GetTopIOHandlerControlSequence(char ch) {
+llvm::StringRef Debugger::GetTopIOHandlerControlSequence(char ch) {
   return m_io_handler_stack.GetTopIOHandlerControlSequence(ch);
 }
 
 const char *Debugger::GetIOHandlerCommandPrefix() {
   return m_io_handler_stack.GetTopIOHandlerCommandPrefix();
 }
 
 const char *Debugger::GetIOHandlerHelpPrologue() {

lldb/source/Expression/REPL.cpp

 
 void REPL::IOHandlerInputInterrupted(IOHandler &io_handler, std::string &line) {
 }
 
 const char *REPL::IOHandlerGetFixIndentationCharacters() {
   return (m_enable_auto_indent ? GetAutoIndentCharacters() : nullptr);
 }
 
-ConstString REPL::IOHandlerGetControlSequence(char ch) {
+llvm::StringRef REPL::IOHandlerGetControlSequence(char ch) {
+  static constexpr llvm::StringLiteral control_sequence(":quit\n");
   if (ch == 'd')
-    return ConstString(":quit\n");
-  return ConstString();
+    return control_sequence;
+  return {};
 }
 
 const char *REPL::IOHandlerGetCommandPrefix() { return ":"; }
 
 const char *REPL::IOHandlerGetHelpPrologue() {
   return "\nThe REPL (Read-Eval-Print-Loop) acts like an interpreter.  "
          "Valid statements, expressions, and declarations are immediately "
          "compiled and executed.\n\n"

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h

 public:
   IOHandlerPythonInterpreter(Debugger &debugger,
                              ScriptInterpreterPythonImpl *python)
       : IOHandler(debugger, IOHandler::Type::PythonInterpreter),
         m_python(python) {}
 
   ~IOHandlerPythonInterpreter() override = default;
 
-  ConstString GetControlSequence(char ch) override {
+  llvm::StringRef GetControlSequence(char ch) override {
+    static constexpr llvm::StringLiteral control_sequence("quit()\n");
     if (ch == 'd')
-      return ConstString("quit()\n");
-    return ConstString();
+      return control_sequence;
+    return {};
   }
 
   void Run() override {
     if (m_python) {
       int stdin_fd = GetInputFD();
       if (stdin_fd >= 0) {
         Terminal terminal(stdin_fd);
         TerminalState terminal_state(terminal);