[Lldb-commits] [PATCH] D105779: RFC: [lldb] Fix editline unicode on Linux

Mon Jul 12 03:48:08 PDT 2021

teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

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.

================
Comment at: lldb/tools/driver/Driver.cpp:871
 int main(int argc, char const *argv[]) {
+  ::setlocale(LC_ALL, "");
+  ::setlocale(LC_CTYPE, "");
----------------
Can you make this `std::setlocale`?

================
Comment at: lldb/tools/driver/Driver.cpp:872
+  ::setlocale(LC_ALL, "");
+  ::setlocale(LC_CTYPE, "");
+
----------------
I don't think we need this if we set `LC_ALL`?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105779/new/

https://reviews.llvm.org/D105779