labath added a comment.
Thanks for doing this. I really like how you've implemented this. Just some
small stylistic comments inline.
================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/PythonReadline.cpp:1
+#ifdef LLDB_DISABLE_LIBEDIT
+
----------------
Should this also be `#if !APPLE`? IIUC, this is not an issue on apple
platforms, and the real readline module is going to be more functional than our
mocked up version...
================
Comment at:
lldb/source/Plugins/ScriptInterpreter/Python/PythonReadline.cpp:29-32
+#else
+PyDoc_STRVAR(moduleDocumentation,
+ "Stub module meant to avoid linking GNU readline.");
+#endif
----------------
It looks like this isn't needed then.
================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/PythonReadline.cpp:82
+PyMODINIT_FUNC initlldb_readline(void) {
+#ifndef LLDB_DISABLE_LIBEDIT
+ PyOS_ReadlineFunctionPointer = simple_readline;
----------------
and this
================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/PythonReadline.h:1-2
+//===-- PythonReadline.h -------------------------------------------*- C++
+//-*-===//
+//
----------------
Fix header.
================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/PythonReadline.h:13-15
+#ifndef LLDB_DISABLE_LIBEDIT
+extern "C" PyMODINIT_FUNC initlldb_readline(void);
+#endif
----------------
How about if this file just exposes a single function like
`WorkAroundLibeditIncompatibilityIfNeeded`? Then this function can be just a
no-op if no work is needed, and there's no need for messy ifdefs anywhere...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69793/new/
https://reviews.llvm.org/D69793
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
