RFC: [lldb] Fix editline unicode on Linux

[jankratochvil]

Summary

[[ https://lists.llvm.org/pipermail/lldb-dev/2021-July/016961.html | [lldb-dev] proposed change to remove conditional WCHAR support in libedit wrapper ]]
There is already setlocale in lldb/source/Core/IOHandlerCursesGUI.cpp but that does not apply for Editline GUI editing.
Unaware how to make automated test for this, it requires pty.

[teemperor]

In retrospective that's a rather obvious fix. Thanks!

This LGTM, but I think this deserves a comment about why we are setting this. Usually LLVM avoids the locale-dependent functions, so this code looks as-is a bit out of place in LLVM. A comment pointing out that editline calls locale-dependent functions seems good enough. Also two small nits as we're anyway touching this code.

FWIW, the Python interpreter runs in the same process as LLDB and Python code can change the locale of the current process. So in theory a user could have a script that does locale.setlocale(locale.LC_ALL, "C") and this way break editline again. But that seems such an obscure case that I don't think we should bother changing/restoring the locale when we enter/leave editline.

lldb/tools/driver/Driver.cpp

871

Can you make this std::setlocale?

872

I don't think we need this if we set LC_ALL?

[jankratochvil]

lldb/tools/driver/Driver.cpp

871

OK, I see <clocale> is for std::setlocale.

872

Originally I have just copy-pasted it. But when testing it more now I think it makes some sense:

#include <locale.h>
#include <stdio.h>
#include <wctype.h>
int main(void) {
  printf("setlocale(LC_ALL  )=%s\n",setlocale(LC_ALL  ,""));
  printf("iswprint(0x17e)=%d\n",iswprint(0x17e));
  printf("setlocale(LC_CTYPE):%s\n",setlocale(LC_CTYPE,""));
  printf("iswprint(0x17e)=%d\n",iswprint(0x17e));
  return 0;
}
$ LANG=C LC_NAME=foobar LC_CTYPE=en_US.UTF-8 ./setlocale
setlocale(LC_ALL  )=(null)
iswprint(0x17e)=0
setlocale(LC_CTYPE):en_US.UTF-8
iswprint(0x17e)=1

Because: Setting all of the categories of the locale of the process is similar to successively setting each individual category of the locale of the process, except that all error checking is done before any actions are performed.

[teemperor]

LGTM, thanks!

lldb/tools/driver/Driver.cpp

872

Good point, didn't think of that case.

lldb/source/Core/IOHandlerCursesGUI.cpp

 #endif
 
 #include <memory>
 #include <mutex>
 
 #include <cassert>
 #include <cctype>
 #include <cerrno>
-#include <clocale>
 #include <cstdint>
 #include <cstdio>
 #include <cstring>
 #include <functional>
 #include <type_traits>
 
 using namespace lldb;
 using namespace lldb_private;
~Application() {
     m_window_sp.reset();
     if (m_screen) {
       ::delscreen(m_screen);
       m_screen = nullptr;
     }
   }
 
   void Initialize() {
-    ::setlocale(LC_ALL, "");
-    ::setlocale(LC_CTYPE, "");
     m_screen = ::newterm(nullptr, m_out, m_in);
     ::start_color();
     ::curs_set(0);
     ::noecho();
     ::keypad(stdscr, TRUE);
   }
 
   void Terminate() { ::endwin(); }

lldb/tools/driver/Driver.cpp

 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include <algorithm>
 #include <atomic>
 #include <bitset>
+#include <clocale>
 #include <csignal>
 #include <string>
 #include <thread>
 #include <utility>
 
 #include <climits>
 #include <cstdio>
 #include <cstdlib>
if (!input_args.hasArg(OPT_no_generate_on_signal)) {
       }
     }
   }
 
   return llvm::None;
 }
 
 int main(int argc, char const *argv[]) {
+  // Editline uses for example iswprint which is dependent on LC_CTYPE.
+  std::setlocale(LC_ALL, "");
+  std::setlocale(LC_CTYPE, "");
+
   // Setup LLVM signal handlers and make sure we call llvm_shutdown() on
   // destruction.
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
 
   // Parse arguments.
   LLDBOptTable T;
   unsigned MissingArgIndex;
   unsigned MissingArgCount;