IOHandler: fall back on File::Read if a FILE* isn't available.

[lawrence_danna]

Summary

IOHandler needs to read lines of input from a lldb::File.
The way it currently does this using, FILE*, which is something
we want to avoid now. I'd prefer to just replace the FILE* code
with calls to File::Read, but it contains an awkward and
delicate workaround specific to ctrl-C handling on windows, and
it's not clear if or how that workaround would translate to
lldb::File.

So in this patch, we use use the FILE* if it's available, and only
fall back on File::Read if that's the only option.

I think this is a reasonable approach here for two reasons. First
is that interactive terminal support is the one area where FILE*
can't be avoided. We need them for libedit and curses anyway,
and using them here as well is consistent with that pattern.

The second reason is that the comments express a hope that the
underlying windows bug that's being worked around will be fixed one
day, so hopefully when that happens, that whole path can be deleted.

[labath]

Even if we ignore the windows parts, there's still a lot of duplication here. The bits that extract a "complete" line from the buffer look like they could be factored into a separate function (Optional<string> GetCompleteLineFromCache(...) ?). It would not hurt to split the parts that read from a FILE and the parts reading from the lldb_private::File into separate functions too...
Other than that, the functionality seems fine to me...

[lawrence_danna]

factor out string splitting stuff

[lawrence_danna]

rebased

[labath]

lldb/source/Core/IOHandler.cpp

322–326

line = StringRef(line_buffer.begin(), pos).rtrim("\n\r") ?

331–346

This looks like too much optimization. Why not just append the new stuff and then call SplitLine ?

351–352

Technically, this check is subsumed by the next one.

353

llvm::all_of(line_buffer, ::isspace)

[lawrence_danna]

review fixes

[lawrence_danna]

fixed

[labath]

lldb/source/Core/IOHandler.cpp

313–315

Please move these to the beginning of the file, next to all the "using namespace" stuff.

[lawrence_danna]

move using declarations

lldb/include/lldb/Core/IOHandler.h

#endif
   StringList *m_current_lines_ptr;
   uint32_t m_base_line_number; // If non-zero, then show line numbers in prompt
   uint32_t m_curr_line_idx;
   bool m_multi_line;
   bool m_color_prompts;
   bool m_interrupt_exits;
   bool m_editing; // Set to true when fetching a line manually (not using
                   // libedit)
+  std::string m_line_buffer;
 };
 
 // The order of base classes is important. Look at the constructor of
 // IOHandlerConfirm to see how.
 class IOHandlerConfirm : public IOHandlerDelegate, public IOHandlerEditline {
 public:
   IOHandlerConfirm(Debugger &debugger, llvm::StringRef prompt,
                    bool default_response);

lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py

def test_binary_inout(self):
             debugger.RunCommandInterpreter(True, False, opts, 0, False, False)
             debugger.GetOutputFile().Flush()
         with open(self.out_filename, 'r') as f:
             output = f.read()
             self.assertIn('Show a list of all debugger commands', output)
 
 
     @add_test_categories(['pyapi'])
-    @expectedFailureAll() # FIXME IOHandler still using FILE*
+    @skipIf(py_version=['<', (3,)])
     def test_string_inout(self):
-        inf = io.StringIO("help help\n")
+        inf = io.StringIO("help help\np/x ~0\n")
         outf = io.StringIO()
         status = self.debugger.SetOutputFile(lldb.SBFile(outf))
         self.assertTrue(status.Success())
         status = self.debugger.SetInputFile(lldb.SBFile(inf))
         self.assertTrue(status.Success())
         opts = lldb.SBCommandInterpreterRunOptions()
         self.debugger.RunCommandInterpreter(True, False, opts, 0, False, False)
         self.debugger.GetOutputFile().Flush()
         output = outf.getvalue()
         self.assertIn('Show a list of all debugger commands', output)
+        self.assertIn('0xfff', output)
 
 
     @add_test_categories(['pyapi'])
-    @expectedFailureAll() # FIXME IOHandler still using FILE*
+    @skipIf(py_version=['<', (3,)])
     def test_bytes_inout(self):
         inf = io.BytesIO(b"help help\nhelp b\n")
         outf = io.BytesIO()
         status = self.debugger.SetOutputFile(lldb.SBFile(outf))
         self.assertTrue(status.Success())
         status = self.debugger.SetInputFile(lldb.SBFile(inf))
         self.assertTrue(status.Success())
         opts = lldb.SBCommandInterpreterRunOptions()

lldb/source/Core/IOHandler.cpp

 #include <locale.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <string.h>
 #include <type_traits>
 
 using namespace lldb;
 using namespace lldb_private;
+using llvm::None;
+using llvm::Optional;
+using llvm::StringRef;
+
 
 IOHandler::IOHandler(Debugger &debugger, IOHandler::Type type)
     : IOHandler(debugger, type,
                 FileSP(),       // Adopt STDIN from top input reader
                 StreamFileSP(), // Adopt STDOUT from top input reader
                 StreamFileSP(), // Adopt STDERR from top input reader
                 0,              // Flags
                 nullptr         // Shadow file recorder
void IOHandlerEditline::Activate() {
   m_delegate.IOHandlerActivated(*this, GetIsInteractive());
 }
 
 void IOHandlerEditline::Deactivate() {
   IOHandler::Deactivate();
   m_delegate.IOHandlerDeactivated(*this);
 }
 
+// Split out a line from the buffer, if there is a full one to get.
+static Optional<std::string> SplitLine(std::string &line_buffer) {
+  size_t pos = line_buffer.find('\n');
+  if (pos == std::string::npos)
+    return None;
+  std::string line = StringRef(line_buffer.c_str(), pos).rtrim("\n\r");
+  line_buffer = line_buffer.substr(pos + 1);
+  return line;
+}
+
+// If the final line of the file ends without a end-of-line, return
+// it as a line anyway.
+static Optional<std::string> SplitLineEOF(std::string &line_buffer) {
+  if (llvm::all_of(line_buffer, isspace))
+    return None;
+  std::string line = std::move(line_buffer);
+  line_buffer.clear();
+  return line;
+}
+
 bool IOHandlerEditline::GetLine(std::string &line, bool &interrupted) {
 #ifndef LLDB_DISABLE_LIBEDIT
   if (m_editline_up) {
     bool b = m_editline_up->GetLine(line, interrupted);
-    if (m_data_recorder)
+    if (b && m_data_recorder)
       m_data_recorder->Record(line, true);
     return b;
-  } else {
+  }
 #endif
-    line.clear();
+
 
-    FILE *in = GetInputFILE();
-    if (in) {
-      if (GetIsInteractive()) {
-        const char *prompt = nullptr;
 
-        if (m_multi_line && m_curr_line_idx > 0)
-          prompt = GetContinuationPrompt();
 
-        if (prompt == nullptr)
-          prompt = GetPrompt();
 
-        if (prompt && prompt[0]) {
-          if (m_output_sp) {
-            m_output_sp->Printf("%s", prompt);
-            m_output_sp->Flush();
-          }
-        }
-      }
-      char buffer[256];
-      bool done = false;
-      bool got_line = false;
-      m_editing = true;
-      while (!done) {
+
+  Optional<std::string> got_line = SplitLine(m_line_buffer);
+
+  if (!got_line && !m_input_sp) {
+    // No more input file, we are done...
+    SetIsDone(true);
+    return false;
+  }
+
+  FILE *in = GetInputFILE();
+
+  if (!got_line && !in && m_input_sp) {
+    // there is no FILE*, fall back on just reading bytes from the stream.
+    while (!got_line) {
+      size_t bytes_read = sizeof(buffer);
+      Status error = m_input_sp->Read((void *)buffer, bytes_read);
+      if (error.Success() && !bytes_read) {
+        got_line = SplitLineEOF(m_line_buffer);
+        break;
+      }
+      if (error.Fail())
+        break;
+      m_line_buffer += StringRef(buffer, bytes_read);
+      got_line = SplitLine(m_line_buffer);
+    }
+  }
+
+  if (!got_line && in) {
+    while (!got_line) {
+      char *r = fgets(buffer, sizeof(buffer), in);
 #ifdef _WIN32
-        // ReadFile on Windows is supposed to set ERROR_OPERATION_ABORTED
-        // according to the docs on MSDN. However, this has evidently been a
-        // known bug since Windows 8. Therefore, we can't detect if a signal
-        // interrupted in the fgets. So pressing ctrl-c causes the repl to end
-        // and the process to exit. A temporary workaround is just to attempt to
-        // fgets twice until this bug is fixed.
-        if (fgets(buffer, sizeof(buffer), in) == nullptr &&
-            fgets(buffer, sizeof(buffer), in) == nullptr) {
-          // this is the equivalent of EINTR for Windows
-          if (GetLastError() == ERROR_OPERATION_ABORTED)
-            continue;
-#else
-      if (fgets(buffer, sizeof(buffer), in) == nullptr) {
+      if (r == nullptr)
+        r = fgets(buffer, sizeof(buffer), in);
+      if (r == nullptr && GetLastError() == ERROR_OPERATION_ABORTED)
 #endif
-          const int saved_errno = errno;
-          if (feof(in))
-            done = true;
-          else if (ferror(in)) {
-            if (saved_errno != EINTR)
-              done = true;
-          }
-        } else {
-          got_line = true;
-          size_t buffer_len = strlen(buffer);
-          assert(buffer[buffer_len] == '\0');
-          char last_char = buffer[buffer_len - 1];
-          if (last_char == '\r' || last_char == '\n') {
-            done = true;
-            // Strip trailing newlines
-            while (last_char == '\r' || last_char == '\n') {
-              --buffer_len;
-              if (buffer_len == 0)
-                break;
-              last_char = buffer[buffer_len - 1];
-            }
-          }
-          line.append(buffer, buffer_len);
-        }
-      }
-      m_editing = false;
-      if (m_data_recorder && got_line)
-        m_data_recorder->Record(line, true);
-      // We might have gotten a newline on a line by itself make sure to return
-      // true in this case.
-      return got_line;
-    } else {
-      // No more input file, we are done...
-      SetIsDone(true);
-    }
-    return false;
-#ifndef LLDB_DISABLE_LIBEDIT
+      if (r == nullptr) {
+        if (ferror(in) && errno == EINTR)
+          continue;
+          got_line = SplitLineEOF(m_line_buffer);
+      m_line_buffer += buffer;
+      got_line = SplitLine(m_line_buffer);
+
+  if (got_line) {
+    line = got_line.getValue();
+    if (m_data_recorder)
+      m_data_recorder->Record(line, true);
   }
-#endif
+
+  return (bool)got_line;
 }
 
 #ifndef LLDB_DISABLE_LIBEDIT
 bool IOHandlerEditline::IsInputCompleteCallback(Editline *editline,
                                                 StringList &lines,
                                                 void *baton) {
   IOHandlerEditline *editline_reader = (IOHandlerEditline *)baton;
   return editline_reader->m_delegate.IOHandlerIsInputComplete(*editline_reader,