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

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

lawrence_danna marked 2 inline comments as done.
lawrence_danna added inline comments.


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1255-1264
+  Status Close() override {
+    assert(m_py_obj);
+    GIL takeGIL;
+    if (m_borrowed)
+      return Flush();
+    Take<PythonObject>(PyObject_CallMethod(m_py_obj, "close", "()"));
+    if (PyErr_Occurred())
----------------
labath wrote:
> Hmm... how exactly does this differ from the implementation in `OwnedPythonFile`? Is it only that it does not chain into the base class? Could we use the same trick as for IsValid to reduce the duplication?
they actually have different behavior.   The OwnedPythonFile version chains into the base class, but the PythonIOFile version also flushes the stream on the //python// side if the file is borrowed.   The idea is that closing any file should flush the buffer you've been writing too even if the file is borrowed.   That way you don't get any weird surprises where you "close" a file and nothing gets written.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188