[Lldb-commits] [PATCH] D68188: allow arbitrary python streams to be converted to SBFile

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna created this revision.
lawrence_danna added reviewers: JDevlieghere, jasonmolenda, labath.
Herald added a project: LLDB.

This patch adds SWIG typemaps that can convert arbitrary python
file objects into lldb_private::File.

A SBFile may be initialized from a python file using the
constructor.   There are also alternate, tagged constructors
that allow python files to be borrowed, and for the caller
to control whether or not the python I/O methods will be
called even when a file descriptor is available.I


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68188

Files:
  lldb/include/lldb/API/SBFile.h
  lldb/include/lldb/Host/File.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/interface/SBFile.i
  lldb/source/Host/common/File.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h

Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -84,14 +84,19 @@
 
   PythonObject(const PythonObject &rhs) : m_py_obj(nullptr) { Reset(rhs); }
 
+  PythonObject(PythonObject &&rhs) {
+m_py_obj = rhs.m_py_obj;
+rhs.m_py_obj = nullptr;
+  }
+
   virtual ~PythonObject() { Reset(); }
 
   void Reset() {
 // Avoid calling the virtual method since it's not necessary
 // to actually validate the type of the PyObject if we're
 // just setting to null.
-if (Py_IsInitialized())
-  Py_XDECREF(m_py_obj);
+if (m_py_obj && Py_IsInitialized())
+  Py_DECREF(m_py_obj);
 m_py_obj = nullptr;
   }
 
@@ -467,8 +472,41 @@
   void Reset(File &file, const char *mode);
 
   lldb::FileUP GetUnderlyingFile() const;
+
+  llvm::Expected ConvertToFile(bool borrowed = false);
+  llvm::Expected
+  ConvertToFileForcingUseOfScriptingIOMethods(bool borrowed = false,
+  uint32_t options = 0);
+};
+
+class PythonException : public llvm::ErrorInfo {
+private:
+  PyObject *m_exception_type, *m_exception, *m_traceback;
+  PyObject *m_repr_bytes;
+
+public:
+  static char ID;
+  const char *toCString() const;
+  PythonException(const char *caller);
+  void Restore();
+  ~PythonException();
+  void log(llvm::raw_ostream &OS) const override;
+  std::error_code convertToErrorCode() const override;
 };
 
+template  T unwrapOrSetPythonException(llvm::Expected expected) {
+  if (expected) {
+return expected.get();
+  } else {
+llvm::handleAllErrors(
+expected.takeError(), [](PythonException &E) { E.Restore(); },
+[](const llvm::ErrorInfoBase &E) {
+  PyErr_SetString(PyExc_Exception, E.message().c_str());
+});
+return T();
+  }
+}
+
 } // namespace lldb_private
 
 #endif
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Host/File.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/StringSwitch.h"
@@ -29,6 +30,14 @@
 using namespace lldb_private;
 using namespace lldb;
 
+template  static T Owned(PyObject *obj) {
+  return T(PyRefType::Owned, obj);
+}
+
+template  static T Borrowed(PyObject *obj) {
+  return T(PyRefType::Borrowed, obj);
+}
+
 void StructuredPythonObject::Dump(Stream &s, bool pretty_print) const {
   s << "Python Obj: 0x" << GetValue();
 }
@@ -954,6 +963,8 @@
 PythonFile::~PythonFile() {}
 
 bool PythonFile::Check(PyObject *py_obj) {
+  if (!py_obj)
+return false;
 #if PY_MAJOR_VERSION < 3
   return PyFile_Check(py_obj);
 #else
@@ -961,9 +972,7 @@
   // first-class object type anymore.  `PyFile_FromFd` is just a thin wrapper
   // over `io.open()`, which returns some object derived from `io.IOBase`. As a
   // result, the only way to detect a file in Python 3 is to check whether it
-  // inherits from `io.IOBase`.  Since it is possible for non-files to also
-  // inherit from `io.IOBase`, we additionally verify that it has the `fileno`
-  // attribute, which should guarantee that it is backed by the file system.
+  // inherits from `io.IOBase`.
   PythonObject io_module(PyRefType::Owned, PyImport_ImportModule("io"));
   PythonDictionary io_dict(PyRefType::Borrowed,
PyModule_GetDict(io_module.get()));
@@ -973,8 +982,6 @@
 
   if (1 != PyObject_IsSubclass(object_type.get(), io_base_class.get()))
 return false;
-  if (!object_type.HasAttribute("fileno"))
-return false;
 
   return true;

[Lldb-commits] [PATCH] D67890: [lldb] [cmake] Fix installing Python modules on systems using /usr/lib

[Hal Gentz via Phabricator via lldb-commits]

ZeGentzy added a comment.

I'm a tad late, but I just wanted to confirm for everyone that this works. 
Thanks everybody!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67890



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68160: File::Clear() -> File::TakeStreamAndClear()

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Sounds good. If this were a more permanent interface, I'd argue that the 
function should be called `TakeStream` (in analogy with 
`llvm::Expected::takeError`), but for a temporary thing that does not matter...




Comment at: lldb/source/Host/common/File.cpp:166-167
+FILE *File::TakeStreamAndClear() {
+  GetStream();
+  FILE *stream = m_stream;
+  m_stream = NULL;

`FILE *stream = GetStream()` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68160



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D68130#1686184 , @vsk wrote:

> Great find. The changes in this patch makes sense to me locally, but I'm 
> having trouble picking up on the context necessary to confidently 'lgtm'. + 
> @JDevlieghere & @labath to get some more experienced people.


I am not very familiar with this code either, but it seems fairly safe and 
straightforward to me...

> I'd love to see the big switch in ParseTypeFromDWARF broken up into small, 
> well-commented functions bit-by-bit -- when that's done, I think I'll have a 
> much better chance at reviewing changes. If folks agree that that's a 
> reasonable refactor, I'd be happy to send a few patches (perhaps starting 
> with the DW_TAG_subroutine_type handling).

+100 to that. I've tried to reduce the function down a bit by splitting the 
dwarf parsing stuff into the separate `ParsedTypeAttributes` class, but the 
amount of code left is still far too much for a single function. If it's 
getting in the way, feel free to undo/refactor the attribute parsing code...


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

https://reviews.llvm.org/D68130



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits