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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I think this is looking pretty good. I like how the read/write methods look now. Just one more question about the PythonBuffer class.



================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1258-1265
+  // you must check PyErr_Occurred() after calling this constructor.
+  PythonBuffer(PythonObject &obj, int flags = PyBUF_SIMPLE)
+      : m_buffer({}), m_error(Error::success()) {
+    PyObject_GetBuffer(obj.get(), &m_buffer, flags);
+    if (!m_buffer.obj) {
+      m_error = llvm::make_error<PythonException>();
+    }
----------------
Storing the Error as a member is quite unusual. It would be better to have a factory function which returns Expected<PythonBuffer>.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188