[Lldb-commits] [PATCH] D69793: Bundle libedit-compatible readline replacement

2019-11-05 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment. Made D69846 for that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69793/new/ https://reviews.llvm.org/D69793 ___ lldb-commits mailing list ll

[Lldb-commits] [PATCH] D69793: Bundle libedit-compatible readline replacement

2019-11-05 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment. Ok, this is breaking build on NetBSD: http://lab.llvm.org:8014/builders/netbsd-amd64/builds/92/steps/ninja%20build%20local/logs/stdio Header location is not the only problem, the code fails to build after fixing that. FWICS, the previous module was built only for subset

[Lldb-commits] [PATCH] D69793: Bundle libedit-compatible readline replacement

2019-11-05 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D69793#1733774 , @serge-sans-paille wrote: > In D69793#1733672 , @labath wrote: > > > > 1. use PyMem_RawMalloc instead of PyMem_Malloc, as expected by > > > PyOS_Readline (prevents to se

[Lldb-commits] [PATCH] D69793: Bundle libedit-compatible readline replacement

2019-11-05 Thread serge via Phabricator via lldb-commits
serge-sans-paille added a comment. In D69793#1733672 , @labath wrote: > > 1. use PyMem_RawMalloc instead of PyMem_Malloc, as expected by > > PyOS_Readline (prevents to segfault upon exit of interactive session) > > It looks like this is failing on python2

[Lldb-commits] [PATCH] D69793: Bundle libedit-compatible readline replacement

2019-11-05 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. > 1. use PyMem_RawMalloc instead of PyMem_Malloc, as expected by PyOS_Readline > (prevents to segfault upon exit of interactive session) It looks like this is failing on python2, because it has no RawMalloc function https://docs.python.org/2.7/c-api/memory.html. I guess

[Lldb-commits] [PATCH] D69793: Bundle libedit-compatible readline replacement

2019-11-05 Thread serge via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9357b5d08497: Revert and patch "[Python] Remove readline module" (authored by serge-sans-paille). Changed prior to commit: https://reviews.llvm.org/D69793?vs=227823&id=227834#toc Repository: rG LLVM

[Lldb-commits] [PATCH] D69793: Bundle libedit-compatible readline replacement

2019-11-05 Thread serge via Phabricator via lldb-commits
serge-sans-paille updated this revision to Diff 227823. serge-sans-paille marked an inline comment as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69793/new/ https://reviews.llvm.org/D69793 Files: lldb/source/Plugins/ScriptInterpreter/Pytho

[Lldb-commits] [PATCH] D69793: Bundle libedit-compatible readline replacement

2019-11-04 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. LGTM. Thanks for doing this. Comment at: lldb/source/Plugins/ScriptInterpreter/Python/PythonReadline.cpp:17 +// implementing any of the readline module methods. This is meant

[Lldb-commits] [PATCH] D69793: Bundle libedit-compatible readline replacement

2019-11-04 Thread serge via Phabricator via lldb-commits
serge-sans-paille updated this revision to Diff 227821. serge-sans-paille marked 4 inline comments as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69793/new/ https://reviews.llvm.org/D69793 Files: lldb/source/Plugins/ScriptInterpreter/Pytho

[Lldb-commits] [PATCH] D69793: Bundle libedit-compatible readline replacement

2019-11-04 Thread serge via Phabricator via lldb-commits
serge-sans-paille marked 2 inline comments as done. serge-sans-paille added inline comments. Comment at: lldb/source/Plugins/ScriptInterpreter/Python/PythonReadline.h:13-15 +#ifndef LLDB_DISABLE_LIBEDIT +extern "C" PyMODINIT_FUNC initlldb_readline(void); +#endif

[Lldb-commits] [PATCH] D69793: Bundle libedit-compatible readline replacement

2019-11-04 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment. +1 on everything Pavel said :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69793/new/ https://reviews.llvm.org/D69793 ___ lldb-commits mailing list lldb-commits@lists.l

[Lldb-commits] [PATCH] D69793: Bundle libedit-compatible readline replacement

2019-11-04 Thread Pavel Labath via Phabricator via lldb-commits
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 b

[Lldb-commits] [PATCH] D69793: Bundle libedit-compatible readline replacement

2019-11-04 Thread serge via Phabricator via lldb-commits
serge-sans-paille updated this revision to Diff 227695. serge-sans-paille edited the summary of this revision. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69793/new/ https://reviews.llvm.org/D69793 Files: lldb/source/Plugins/ScriptInterpreter/Py

[Lldb-commits] [PATCH] D69793: Bundle libedit-compatible readline replacement

2019-11-04 Thread Michał Górny via Phabricator via lldb-commits
mgorny added inline comments. Comment at: lldb/source/Plugins/ScriptInterpreter/Python/PythonReadline.cpp:8 + +#ifndef LLDB_DISABLE_LIBEDIT +#include If libedit is disabled, shouldn't you disable building the whole module altogether? I suppose then the standard

[Lldb-commits] [PATCH] D69793: Bundle libedit-compatible readline replacement

2019-11-04 Thread serge via Phabricator via lldb-commits
serge-sans-paille created this revision. serge-sans-paille added reviewers: labath, JDevlieghere. serge-sans-paille added a project: LLDB. Herald added subscribers: lldb-commits, mgorny. Fix https://bugs.llvm.org/show_bug.cgi?id=43830 while avoiding polluting the global Python namespace. This bo