LineEditor: Add a bare-bones readline-based implementation

[thakis]

Summary

Doesn't do tab completion yet, but it's enough to get working
Ctrl-P in clang-repl.

[amccarth]

When this gets further along, maybe we can get LLDB to use it instead of EditLine.

llvm/lib/LineEditor/LineEditor.cpp

30

Nit: In theory, this should be followed with a flush of Out.

In practice, it won't matter in most cases because stdio will ensure flushing stdout before accepting input on stdin. But there's no guarantee here that In and Out correspond to the standard streams. And since the prompt probably doesn't end with '\n', you cannot count on that flushing either.

[dblaikie]

llvm/lib/LineEditor/LineEditor.cpp

39–40

Skip else-after-return

373

prefer std::make_unique here, perhaps (makes it a bit easier to read/clear this member isn't a raw pointer/risk of leaks)

[thakis]

address comments

[teemperor]

When this gets further along, maybe we can get LLDB to use it instead of EditLine.

+1

[hans]

Are the presubmit errors related to this?

The change itself lgtm

llvm/lib/LineEditor/LineEditor.cpp

30

+1 I'd flush here just to be sure.

43

Maybe use Line.back() instead of Line[Line.size() - 1] here and below.

47

Could be pop_back() instead, which would be a nice match if using Line.back() == .. for the condition above.

llvm/CMakeLists.txt

 set(LLVM_TARGET_ARCH "host"
   CACHE STRING "Set target to use for LLVM JIT or use \"host\" for automatic detection.")
 
 option(LLVM_ENABLE_TERMINFO "Use terminfo database if available." ON)
 
 set(LLVM_ENABLE_LIBXML2 "ON" CACHE STRING "Use libxml2 if available. Can be ON, OFF, or FORCE_ON")
 
 option(LLVM_ENABLE_LIBEDIT "Use libedit if available." ON)
+option(LLVM_ENABLE_LIBREADLINE "Use libreadline if available." ON)
 
 option(LLVM_ENABLE_LIBPFM "Use libpfm for performance counters if available." ON)
 
 # On z/OS, threads cannot be used because TLS is not supported.
 if (CMAKE_SYSTEM_NAME MATCHES "OS390")
   option(LLVM_ENABLE_THREADS "Use threads if available." OFF)
 else()
   option(LLVM_ENABLE_THREADS "Use threads if available." ON)

llvm/cmake/config-ix.cmake

 check_include_file(unistd.h HAVE_UNISTD_H)
 check_include_file(valgrind/valgrind.h HAVE_VALGRIND_VALGRIND_H)
 check_include_file(fenv.h HAVE_FENV_H)
 check_symbol_exists(FE_ALL_EXCEPT "fenv.h" HAVE_DECL_FE_ALL_EXCEPT)
 check_symbol_exists(FE_INEXACT "fenv.h" HAVE_DECL_FE_INEXACT)
 
 check_include_file(mach/mach.h HAVE_MACH_MACH_H)
 check_include_file(histedit.h HAVE_HISTEDIT_H)
+check_include_file(readline/readline.h HAVE_READLINE_H)
 check_include_file(CrashReporterClient.h HAVE_CRASHREPORTERCLIENT_H)
 if(APPLE)
   include(CheckCSourceCompiles)
   CHECK_C_SOURCE_COMPILES("
      static const char *__crashreporter_info__ = 0;
      asm(\".desc ___crashreporter_info__, 0x10\");
      int main() { return 0; }"
     HAVE_CRASHREPORTER_INFO)
if(NOT LLVM_USE_SANITIZER MATCHES "Memory.*")
   # Don't look for these libraries on Windows.
   if (NOT PURE_WINDOWS)
     # Skip libedit if using ASan as it contains memory leaks.
     if (LLVM_ENABLE_LIBEDIT AND HAVE_HISTEDIT_H AND NOT LLVM_USE_SANITIZER MATCHES ".*Address.*")
       check_library_exists(edit el_init "" HAVE_LIBEDIT)
     else()
       set(HAVE_LIBEDIT 0)
     endif()
+    if (LLVM_ENABLE_LIBREADLINE AND HAVE_READLINE_H)
+      check_library_exists(readline readline "" HAVE_LIBREADLINE)
+    else()
+      set(HAVE_LIBREADLINE 0)
+    endif()
     if(LLVM_ENABLE_TERMINFO STREQUAL FORCE_ON)
       set(MAYBE_REQUIRED REQUIRED)
     else()
       set(MAYBE_REQUIRED)
     endif()
     if(LLVM_ENABLE_TERMINFO)
       find_library(TERMINFO_LIB NAMES terminfo tinfo curses ncurses ncursesw ${MAYBE_REQUIRED})
     endif()

llvm/include/llvm/Config/config.h.cmake

 #cmakedefine HAVE_GETRUSAGE ${HAVE_GETRUSAGE}
 
 /* Define to 1 if you have the `isatty' function. */
 #cmakedefine HAVE_ISATTY 1
 
 /* Define to 1 if you have the `edit' library (-ledit). */
 #cmakedefine HAVE_LIBEDIT ${HAVE_LIBEDIT}
 
+/* Define to 1 if you have the `readline' library (-lreadline). */
+#cmakedefine HAVE_LIBREADLINE ${HAVE_LIBREADLINE}
+
 /* Define to 1 if you have the `pfm' library (-lpfm). */
 #cmakedefine HAVE_LIBPFM ${HAVE_LIBPFM}
 
 /* Define to 1 if the `perf_branch_entry' struct has field cycles. */
 #cmakedefine LIBPFM_HAS_FIELD_CYCLES ${LIBPFM_HAS_FIELD_CYCLES}
 
 /* Define to 1 if you have the `psapi' library (-lpsapi). */
 #cmakedefine HAVE_LIBPSAPI ${HAVE_LIBPSAPI}

llvm/lib/LineEditor/CMakeLists.txt

 if(HAVE_LIBEDIT)
   set(link_libs edit)
 endif()
+if(HAVE_LIBREADLINE)
+  set(link_libs readline)
+endif()
 
 add_llvm_component_library(LLVMLineEditor
   LineEditor.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/LineEditor
 
   LINK_LIBS
   ${link_libs}
 
   LINK_COMPONENTS
   Support
   )

llvm/lib/LineEditor/LineEditor.cpp

 //===-- LineEditor.cpp - line editor --------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/LineEditor/LineEditor.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Config/config.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cassert>
 #include <cstdio>
-#ifdef HAVE_LIBEDIT
+#if defined(HAVE_LIBEDIT)
 #include <histedit.h>
+#elif defined(HAVE_LIBREADLINE)
+#include <readline/history.h>
+#include <readline/readline.h>
+#include <unistd.h>
 #endif
 
 using namespace llvm;
 
+#if !defined(HAVE_LIBEDIT)
+static Optional<std::string> PromptOnFILE(const std::string &Prompt, FILE *In,
+                                          FILE *Out) {
+  ::fprintf(Out, "%s", Prompt.c_str());
+
+  std::string Line;
+  do {
+    char Buf[64];
+    char *Res = ::fgets(Buf, sizeof(Buf), In);
+    if (!Res) {
+      if (Line.empty())
+        return Optional<std::string>();
+      return Line;
+    }
+    Line.append(Buf);
+  } while (Line.empty() ||
+           (Line[Line.size() - 1] != '\n' && Line[Line.size() - 1] != '\r'));
+
+  while (!Line.empty() &&
+         (Line[Line.size() - 1] == '\n' || Line[Line.size() - 1] == '\r'))
+    Line.resize(Line.size() - 1);
+
+  return Line;
+}
+#endif
+
 std::string LineEditor::getDefaultHistoryPath(StringRef ProgName) {
   SmallString<32> Path;
   if (sys::path::home_directory(Path)) {
     sys::path::append(Path, "." + ProgName + "-history");
     return std::string(Path.str());
   }
   return std::string();
 }
if (!Completer) {
     CompletionAction Action;
     Action.Kind = CompletionAction::AK_ShowCompletions;
     return Action;
   }
 
   return Completer->complete(Buffer, Pos);
 }
 
-#ifdef HAVE_LIBEDIT
+#if defined(HAVE_LIBEDIT)
 
 // libedit-based implementation.
 
 struct LineEditor::InternalData {
   LineEditor *LE;
 
   History *Hist;
   EditLine *EL;
unsigned char ElCompletionFn(EditLine *EL, int ch) {
   return CC_ERROR;
 }
 
 } // end anonymous namespace
 
 LineEditor::LineEditor(StringRef ProgName, StringRef HistoryPath, FILE *In,
                        FILE *Out, FILE *Err)
     : Prompt((ProgName + "> ").str()), HistoryPath(std::string(HistoryPath)),
-      Data(new InternalData) {
+      Data(std::make_unique<InternalData>()) {
   if (HistoryPath.empty())
     this->HistoryPath = getDefaultHistoryPath(ProgName);
 
   Data->LE = this;
   Data->Out = Out;
 
   Data->Hist = ::history_init();
   assert(Data->Hist);
Optional<std::string> LineEditor::readLine() const {
 
   HistEvent HE;
   if (LineLen > 0)
     ::history(Data->Hist, &HE, H_ENTER, Line);
 
   return std::string(Line, LineLen);
 }
 
-#else // HAVE_LIBEDIT
+#elif defined(HAVE_LIBREADLINE)
 
-// Simple fgets-based implementation.
+// libreadline-based implementation.
 
 struct LineEditor::InternalData {
   FILE *In;
   FILE *Out;
+  bool UseReadline;
 };
 
 LineEditor::LineEditor(StringRef ProgName, StringRef HistoryPath, FILE *In,
                        FILE *Out, FILE *Err)
-    : Prompt((ProgName + "> ").str()), Data(new InternalData) {
+    : Prompt((ProgName + "> ").str()), Data(std::make_unique<InternalData>()) {
   Data->In = In;
   Data->Out = Out;
+  Data->UseReadline = Out == stdout && isatty(STDOUT_FILENO) &&
+                      In == stdin && isatty(STDIN_FILENO);
+
+  if (Data->UseReadline) {
+    // FIXME: Set up tab completion. For now, tab inserts itself.
+    rl_bind_key('\t', rl_insert);
+  }
 }
 
 LineEditor::~LineEditor() {
   ::fwrite("\n", 1, 1, Data->Out);
 }
 
-void LineEditor::saveHistory() {}
-void LineEditor::loadHistory() {}
+void LineEditor::saveHistory() {
+  // FIXME: Implement.
+}
+void LineEditor::loadHistory() {
+  // FIXME: Implement.
+}
 
 Optional<std::string> LineEditor::readLine() const {
-  ::fprintf(Data->Out, "%s", Prompt.c_str());
+  // FIXME: Maybe LineEditor should have a Create() factory method that
+  // returns an FgetsLineEditor() object if !Data->UseReadline, instead of
+  // storing this state and doing the check here.
+  if (!Data->UseReadline)
+    return PromptOnFILE(Prompt, Data->Out, Data->In);
 
   std::string Line;
-  do {
-    char Buf[64];
-    char *Res = ::fgets(Buf, sizeof(Buf), Data->In);
-    if (!Res) {
-      if (Line.empty())
-        return Optional<std::string>();
-      else
-        return Line;
-    }
-    Line.append(Buf);
-  } while (Line.empty() ||
-           (Line[Line.size() - 1] != '\n' && Line[Line.size() - 1] != '\r'));
+  char *Buf;
+  if ((Buf = readline(Prompt.c_str())) == nullptr)
+
+  Line = Buf;
+  free(Buf);
+
+  if (!Line.empty())
+    add_history(Line.c_str());
+
+#else
 
-  while (!Line.empty() &&
-         (Line[Line.size() - 1] == '\n' || Line[Line.size() - 1] == '\r'))
-    Line.resize(Line.size() - 1);
+// Simple fgets-based implementation.
 
-  return Line;
+struct LineEditor::InternalData {
+  FILE *In;
+  FILE *Out;
+};
+
+LineEditor::LineEditor(StringRef ProgName, StringRef HistoryPath, FILE *In,
+                       FILE *Out, FILE *Err)
+    : Prompt((ProgName + "> ").str()), Data(std::make_unique<InternalData>()) {
+  Data->In = In;
+  Data->Out = Out;
+}
+
+LineEditor::~LineEditor() { ::fwrite("\n", 1, 1, Data->Out); }
+
+void LineEditor::saveHistory() {}
+void LineEditor::loadHistory() {}
+
+Optional<std::string> LineEditor::readLine() const {
+  return PromptOnFILE(Prompt, Data->Out, Data->In);
 }
 
 #endif // HAVE_LIBEDIT

llvm/utils/gn/build/libs/readline/BUILD.gn

+import("//llvm/utils/gn/build/libs/readline/enable.gni")
+
+config("readline_config") {
+  visibility = [ ":readline" ]
+  libs = [ "readline" ]
+}
+
+group("readline") {
+  if (llvm_enable_libreadline) {
+    public_configs = [ ":readline_config" ]
+  }
+}

llvm/utils/gn/build/libs/readline/enable.gni

+declare_args() {
+  llvm_enable_libreadline = host_os == "linux"
+}

llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn

 import("//llvm/lib/DebugInfo/PDB/enable_dia.gni")
 import("//llvm/lib/Target/targets.gni")
 import("//llvm/lib/Target/targets_with_asm_parsers.gni")
 import("//llvm/lib/Target/targets_with_disassemblers.gni")
 import("//llvm/triples.gni")
 import("//llvm/utils/gn/build/buildflags.gni")
 import("//llvm/utils/gn/build/libs/edit/enable.gni")
 import("//llvm/utils/gn/build/libs/pthread/enable.gni")
+import("//llvm/utils/gn/build/libs/readline/enable.gni")
 import("//llvm/utils/gn/build/libs/terminfo/enable.gni")
 import("//llvm/utils/gn/build/libs/xar/enable.gni")
 import("//llvm/utils/gn/build/libs/xml/enable.gni")
 import("//llvm/utils/gn/build/libs/zlib/enable.gni")
 import("//llvm/utils/gn/build/write_cmake_config.gni")
 import("//llvm/version.gni")
 
 # Contains actions to create config.h, llvm-config.h, abi-breaking.h,
write_cmake_config("config") {
   ]
 
   if (llvm_enable_libedit) {
     values += [ "HAVE_LIBEDIT=1" ]
   } else {
     values += [ "HAVE_LIBEDIT=" ]
   }
 
+  if (llvm_enable_libreadline) {
+    values += [ "HAVE_LIBREADLINE=1" ]
+  } else {
+    values += [ "HAVE_LIBREADLINE=" ]
+  }
+
   if (llvm_enable_terminfo) {
     values += [ "LLVM_ENABLE_TERMINFO=1" ]
   } else {
     values += [ "LLVM_ENABLE_TERMINFO=" ]
   }
 
   if (llvm_enable_dia_sdk) {
     values += [ "LLVM_ENABLE_DIA_SDK=1" ]

llvm/utils/gn/secondary/llvm/lib/LineEditor/BUILD.gn

 static_library("LineEditor") {
   output_name = "LLVMLineEditor"
   deps = [
     "//llvm/include/llvm/Config:config",
     "//llvm/lib/Support",
     "//llvm/utils/gn/build/libs/edit",
+    "//llvm/utils/gn/build/libs/readline",
   ]
 
   sources = [ "LineEditor.cpp" ]
 }