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 ↗

(On Diff #357799)

Can you make this std::setlocale?

872 ↗

(On Diff #357799)

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

[jankratochvil]

lldb/tools/driver/Driver.cpp

871 ↗

(On Diff #357799)

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

872 ↗

(On Diff #357799)

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 ↗

(On Diff #357799)

Good point, didn't think of that case.

lldb/source/Host/common/Editline.cpp

 //===-- Editline.cpp ------------------------------------------------------===//
 //
 // 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 <climits>
 #include <iomanip>
 
 #include "lldb/Host/Editline.h"
 
 #include "lldb/Host/ConnectionFileDescriptor.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Utility/CompletionRequest.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/SelectHelper.h"
if (term_fd != -1) {
       if (g_init_terminal_fds_ptr->find(term_fd) ==
           g_init_terminal_fds_ptr->end()) {
         g_init_terminal_fds_ptr->insert(term_fd);
         setupterm((char *)0, term_fd, (int *)0);
       }
     }
   }
 #endif
+
+  // Required for libedit unicode as it uses iswprint.
+  ::setlocale(LC_CTYPE, "");
 }
 
 Editline::~Editline() {
   if (m_editline) {
     // Disable edit mode to stop the terminal from flushing all input during
     // the call to el_end() since we expect to have multiple editline instances
     // in this program.
     el_set(m_editline, EL_EDITMODE, 0);