[LLDB][NFC] Simplify IOHandler's getLine to avoid strange casts and redundant checks

[teemperor]

Thanks for the patch! I think we can actually simplify that code even more by just removing the !got_line checks in both ifs? There is really no reason to have them from what I can see.

lldb/source/Core/IOHandler.cpp

388–390

Just a small FYI: I removed the m_editing variable here in 324000054652d47467d23b73ada5c8a599087167

431

This version is fine too, but we could potentially do an LLVMy early return:

if (!got_line)
  return false;
[...]
return true;

[gAlfonso-bit]

This comment was removed by gAlfonso-bit.

[gAlfonso-bit]

This comment was removed by gAlfonso-bit.

[shafik]

lldb/source/Core/IOHandler.cpp

374

A goto is not warranted here, using if (!got_line && !in is perfectly fine and less lines.

[gAlfonso-bit]

This comment was removed by gAlfonso-bit.

[gAlfonso-bit]

This comment was removed by gAlfonso-bit.

[gAlfonso-bit]

This comment was removed by gAlfonso-bit.

[DavidSpickett]

Only clang-tidy is upset and it is falsely flagging "missing" files

This is because llvm precommit doesn't actually build lldb (due to some test failures they were/are having) so those headers are unknown to clang-tidy when it runs. You can ignore it.

lldb/include/lldb/Core/IOHandler.h

 #include <cstdint>
 #include <cstdio>
 
 namespace lldb_private {
 class Debugger;
 namespace repro {
 class DataRecorder;
 }
-}
+} // namespace lldb_private
 
 namespace curses {
 class Application;
 typedef std::unique_ptr<Application> ApplicationAP;
 } // namespace curses
 
 namespace lldb_private {
 

lldb/source/Core/IOHandler.cpp

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();
-      }
-    }
-  }
+    if (prompt && prompt[0] && m_output_sp) {
 
   Optional<std::string> got_line = SplitLine(m_line_buffer);
 
-  if (!got_line && !m_input_sp) {
+  if (got_line) {
+    line = got_line.getValue();
+    if (m_data_recorder)
+      m_data_recorder->Record(line, true);
+    return true;
+  }
+
+  if (!m_input_sp) {
     // No more input file, we are done...
     SetIsDone(true);
     return false;
   }
 
   FILE *in = GetInputFILE();
   char buffer[256];
 
-  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) {
+  if (in) {
+    do {
       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 (r == nullptr)
         r = fgets(buffer, sizeof(buffer), in);
       // this is the equivalent of EINTR for Windows
       if (r == nullptr && GetLastError() == ERROR_OPERATION_ABORTED)
         continue;
 #endif
       if (r == nullptr) {
         if (ferror(in) && errno == EINTR)
           continue;
         if (feof(in))
           got_line = SplitLineEOF(m_line_buffer);
         break;
       }
       m_line_buffer += buffer;
       got_line = SplitLine(m_line_buffer);
+    } while (!got_line);
+  } else {
+    // there is no FILE*, fall back on just reading bytes from the stream.
+    do {
+      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())
+        return false;
+      m_line_buffer += StringRef(buffer, bytes_read);
+      got_line = SplitLine(m_line_buffer);
+    } while (!got_line);
   }
 
-  if (got_line) {
-    line = got_line.getValue();
-    if (m_data_recorder)
-      m_data_recorder->Record(line, true);
-  }
-
-  return (bool)got_line;
+  if (!got_line)
+    return false;
+
+  return true;
 }
 
 #if LLDB_ENABLE_LIBEDIT
 bool IOHandlerEditline::IsInputCompleteCallback(Editline *editline,
                                                 StringList &lines) {
   return m_delegate.IOHandlerIsInputComplete(*this, lines);
 }