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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits