[cmake] Detect presence of wide-char libedit at build time

[labath]

Summary

Instead of hardcoding a list of platforms where libedit is known to have
wide char support we detect this in cmake. The main motivation for this
is attempting to improve compatibility with different versions of
libedit, as the interface of non-wide-char functions varies slightly
between versions.

This should be NFC on all platforms except linux. On linux, we used to
hardcode to non-wide-char, now we will use wide-char if we detect
libedit supports that.

[labath]

include/lldb/Host/Editline.h

72–76 ↗

(On Diff #149429)

It's not fully clear to me whether this part is still correct. My understanding is that if we use el_wset to set the getchar callback, we should always use wchar_t, regardless of libedit version. This is only true if all wide-char capable libedit versions also define EL_CLIENTDATA.

Is this the case?

[jankratochvil]

No regressions on Fedora 28 x86_64. It does set there now LLDB_EDITLINE_USE_WCHAR although one still cannot enter unicode characters to (lldb) line.

[krytarowski]

The original reason to switch NetBSD to wide characters was to unbreak the input parser. It used to mix wide and short characters.

Typing unicode characters doesn't work well here too (before applying the patch):

(lldb) za\U+017C\U+0142\U+0107 g\U+0119\U+015Bl\U+0105 ja\U+017A\U+0144

[christos]

include/lldb/Host/Editline.h

72–76 ↗

(On Diff #149429)

I believe that you are correct: If you use el_wset, you should use wchar_t. The problem is that for libedit versions prior to 2016-04-19 the getchar function used char * for narrow and wchar_t * for wide. Versions after that use wchar_t * for both. This was an accident due to some code refactoring. If you are checking if that's the case or not, perhaps you can determine this if el_rfunc_t is defined in histedit.h or not. Unfortunately there is no compile time way to determine if that's the case or not.

[labath]

include/lldb/Host/Editline.h

72–76 ↗

(On Diff #149429)

Thank, I can do that. Just to double-check, the condition you say here should be is:

#if LLDB_EDITLINE_USE_WCHAR || defined(EL_CLIENTDATA) || LLDB_LIBEDIT_HAVE_EL_RFUNC_T
using EditLineGetCharType = wchar_t;
#else
using EditLineGetCharType = char;
#endif

Is that right ?

[christos]

include/lldb/Host/Editline.h

72–76 ↗

(On Diff #149429)

I think that's right. In any case it will probably fix cases that are currently broken.

[labath]

include/lldb/Host/Editline.h

72–76 ↗

(On Diff #149429)

Thanks a lot. I've committed the patch with this condition updated.

[davide]

Pavel, this broke unicode handling for lldb on MacOS. If you type something in the lldb cmdline, it won't print the right character but a series of unicode sequences.
The main concern is that this also breaks the Swift REPL (powered by lldb).

Looks like something like this:

$ git diff
diff --git a/lldb/include/lldb/Host/Editline.h b/lldb/include/lldb/Host/Editline.h
index cb40f1e6917..e53991c450f 100644
--- a/lldb/include/lldb/Host/Editline.h
+++ b/lldb/include/lldb/Host/Editline.h
@@ -51,6 +51,7 @@
 #include <string>
 #include <vector>
 
+#include "lldb/Host/Config.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
 #include "lldb/Host/Predicate.h"
 #include "lldb/Utility/FileSpec.h"

"fixes" the CMake build. I'm seeing failures if I apply the same patch to the Xcode project.

In file included from /Users/davide/lldb/source/Host/common/Editline.cpp:14:
/Users/davide/lldb/include/lldb/Host/Editline.h:330:29: error: no member named 'codecvt_utf8' in namespace 'std'
  std::wstring_convert<std::codecvt_utf8<wchar_t>> m_utf8conv;
                       ~~~~~^
/Users/davide/lldb/include/lldb/Host/Editline.h:330:49: error: expected '(' for function-style cast or type construction
  std::wstring_convert<std::codecvt_utf8<wchar_t>> m_utf8conv;
                                         ~~~~~~~^
/Users/davide/lldb/source/Host/common/Editline.cpp:413:24: error: use of undeclared identifier 'm_utf8conv'
    lines.AppendString(m_utf8conv.to_bytes(line));
                       ^
/Users/davide/lldb/source/Host/common/Editline.cpp:577:26: error: use of undeclared identifier 'm_utf8conv'
      lines.AppendString(m_utf8conv.to_bytes(new_line_fragment));
                         ^
/Users/davide/lldb/source/Host/common/Editline.cpp:628:30: error: use of undeclared identifier 'm_utf8conv'
                             m_utf8conv.from_bytes(lines[index]));
                             ^
/Users/davide/lldb/source/Host/common/Editline.cpp:813:41: error: use of undeclared identifier 'm_utf8conv'
  m_input_lines[m_current_line_index] = m_utf8conv.from_bytes(currentLine);
                                        ^
/Users/davide/lldb/source/Host/common/Editline.cpp:1319:14: error: use of undeclared identifier 'm_utf8conv'
      line = m_utf8conv.to_bytes(SplitLines(input)[0]);
             ^
/Users/davide/lldb/source/Host/common/Editline.cpp:1389:8: error: no member named 'codecvt_utf8' in namespace 'std'
  std::codecvt_utf8<wchar_t> cvt;
  ~~~~~^
/Users/davide/lldb/source/Host/common/Editline.cpp:1389:28: error: expected '(' for function-style cast or type construction
  std::codecvt_utf8<wchar_t> cvt;
                    ~~~~~~~^
/Users/davide/lldb/source/Host/common/Editline.cpp:1389:30: error: use of undeclared identifier 'cvt'
  std::codecvt_utf8<wchar_t> cvt;
                             ^
/Users/davide/lldb/source/Host/common/Editline.cpp:1396:13: error: use of undeclared identifier 'cvt'
    switch (cvt.in(state, input.begin(), input.end(), from_next, &out, &out + 1,
            ^
11 errors generated.

Can you please take a look? If you need anything from me, just let me know!

lldb/trunk/cmake/modules/LLDBGenerateConfig.cmake

 # This file contains all the logic for running configure-time checks
 
 include(CheckSymbolExists)
 include(CheckIncludeFile)
 include(CheckIncludeFiles)
 include(CheckLibraryExists)
+include(CheckTypeSize)
 
 set(CMAKE_REQUIRED_DEFINITIONS -D_GNU_SOURCE)
 check_symbol_exists(ppoll poll.h HAVE_PPOLL)
 set(CMAKE_REQUIRED_DEFINITIONS)
 check_symbol_exists(sigaction signal.h HAVE_SIGACTION)
 check_cxx_symbol_exists(accept4 "sys/socket.h" HAVE_ACCEPT4)
 
 check_include_file(termios.h HAVE_TERMIOS_H)
 check_include_files("sys/types.h;sys/event.h" HAVE_SYS_EVENT_H)
 
 check_cxx_symbol_exists(process_vm_readv "sys/uio.h" HAVE_PROCESS_VM_READV)
 check_cxx_symbol_exists(__NR_process_vm_readv "sys/syscall.h" HAVE_NR_PROCESS_VM_READV)
 
 check_library_exists(compression compression_encode_buffer "" HAVE_LIBCOMPRESSION)
 
 # These checks exist in LLVM's configuration, so I want to match the LLVM names
 # so that the check isn't duplicated, but we translate them into the LLDB names
 # so that I don't have to change all the uses at the moment.
 set(LLDB_CONFIG_TERMIOS_SUPPORTED ${HAVE_TERMIOS_H})
 if(NOT UNIX)
   set(LLDB_DISABLE_POSIX 1)
 endif()
 
+if (NOT LLDB_DISABLE_LIBEDIT)
+  # Check if we libedit capable of handling wide characters (built with
+  # '--enable-widec').
+  set(CMAKE_REQUIRED_LIBRARIES ${libedit_LIBRARIES})
+  set(CMAKE_REQUIRED_INCLUDES ${libedit_INCLUDE_DIRS})
+  check_symbol_exists(el_winsertstr histedit.h LLDB_EDITLINE_USE_WCHAR)
+  set(CMAKE_EXTRA_INCLUDE_FILES histedit.h)
+  check_type_size(el_rfunc_t LLDB_EL_RFUNC_T_SIZE)
+  if (LLDB_EL_RFUNC_T_SIZE STREQUAL "")
+    set(LLDB_HAVE_EL_RFUNC_T 0)
+  else()
+    set(LLDB_HAVE_EL_RFUNC_T 1)
+  endif()
+  set(CMAKE_REQUIRED_LIBRARIES)
+  set(CMAKE_REQUIRED_INCLUDES)
+  set(CMAKE_EXTRA_INCLUDE_FILES)
+endif()
+
 if(NOT LLDB_CONFIG_HEADER_INPUT)
  set(LLDB_CONFIG_HEADER_INPUT ${LLDB_INCLUDE_ROOT}/lldb/Host/Config.h.cmake)
 endif()
 
 if(NOT LLDB_CONFIG_HEADER_OUTPUT)
  set(LLDB_CONFIG_HEADER_OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/include/lldb/Host/Config.h)
 endif()
 
 # This should be done at the end
 configure_file(
   ${LLDB_CONFIG_HEADER_INPUT}
   ${LLDB_CONFIG_HEADER_OUTPUT}
   )

lldb/trunk/include/lldb/Host/Config.h

 #define LLDB_HOST_CONFIG_H
  
 #if defined(__APPLE__)
 
 // This block of code only exists to keep the Xcode project working in the
 // absence of a configuration step.
 #define LLDB_CONFIG_TERMIOS_SUPPORTED 1
 
+#define LLDB_EDITLINE_USE_WCHAR 1
+
+#define LLDB_HAVE_EL_RFUNC_T 1
+
 #define HAVE_SYS_EVENT_H 1
 
 #define HAVE_PPOLL 0
 
 #define HAVE_SIGACTION 1
 
 #else
 
 #error This file is only used by the Xcode build.
 
 #endif
 
 #endif // #ifndef LLDB_HOST_CONFIG_H

lldb/trunk/include/lldb/Host/Config.h.cmake

 //===-- Config.h -----------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_HOST_CONFIG_H
 #define LLDB_HOST_CONFIG_H
 
 #cmakedefine LLDB_CONFIG_TERMIOS_SUPPORTED
 
+#cmakedefine01 LLDB_EDITLINE_USE_WCHAR
+
+#cmakedefine01 LLDB_HAVE_EL_RFUNC_T
+
 #cmakedefine LLDB_DISABLE_POSIX
 
 #define LLDB_LIBDIR_SUFFIX "${LLVM_LIBDIR_SUFFIX}"
 
 #cmakedefine01 HAVE_SYS_EVENT_H
 
 #cmakedefine01 HAVE_PPOLL
 

lldb/trunk/include/lldb/Host/Editline.h

 // ANSI sequences starting with escape.
 // e) Emoji support is fairly terrible, presumably it doesn't understand
 // composed characters?
 
 #ifndef liblldb_Editline_h_
 #define liblldb_Editline_h_
 #if defined(__cplusplus)
 
+#include <codecvt>
 #include <locale>
 #include <sstream>
 #include <vector>
 
-// components needed to handle wide characters ( <codecvt>, codecvt_utf8,
-// libedit built with '--enable-widec' ) are available on some platforms. The
-// wchar_t versions of libedit functions will only be used in cases where this
-// is true.  This is a compile time dependecy, for now selected per target
-// Platform
-#if defined(__APPLE__) || defined(__FreeBSD__) || defined(__NetBSD__) ||       \
-    defined(__OpenBSD__)
-#define LLDB_EDITLINE_USE_WCHAR 1
-#include <codecvt>
-#else
-#define LLDB_EDITLINE_USE_WCHAR 0
-#endif
-
 #include "lldb/Host/ConnectionFileDescriptor.h"
 #include "lldb/lldb-private.h"
 
 #if defined(_WIN32)
 #include "lldb/Host/windows/editlinewin.h"
 #elif !defined(__ANDROID__)
 #include <histedit.h>
 #endif

 using EditLineStringStreamType = std::wstringstream;
 using EditLineCharType = wchar_t;
 #else
 using EditLineStringType = std::string;
 using EditLineStringStreamType = std::stringstream;
 using EditLineCharType = char;
 #endif
 
-#ifdef EL_CLIENTDATA  /* editline with wide support + wide char read function */
+// At one point the callback type of el_set getchar callback changed from char
+// to wchar_t. It is not possible to detect differentiate between the two
+// versions exactly, but this is a pretty good approximation and allows us to
+// build against almost any editline version out there.
+#if LLDB_EDITLINE_USE_WCHAR || defined(EL_CLIENTDATA) || LLDB_HAVE_EL_RFUNC_T
 using EditLineGetCharType = wchar_t;
 #else
 using EditLineGetCharType = char;
 #endif
 
 typedef int (*EditlineGetCharCallbackType)(::EditLine *editline,
                                            EditLineGetCharType *c);
 typedef unsigned char (*EditlineCommandCallbackType)(::EditLine *editline,