[Lldb-commits] [PATCH] D68737: SBFile::GetFile: convert SBFile back into python native files.

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

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


================
Comment at: lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py:776
     @add_test_categories(['pyapi'])
-    @expectedFailure # FIXME implement SBFile::GetFile
     @skipIf(py_version=['<', (3,)])
----------------
labath wrote:
> So, if I understand correctly, these tests check that you can feed a file opened by python into lldb, and then pump it back out to python. Are there any tests which do the opposite (have lldb open a file, move it to python and then reinject it into lldb)?
Yea, `foo is bar` in python is essentially a pointer comparison between the `PyObject*` pointers, so this is testing that you get the same identical file object back in the situations where you should.

There's no test going the other way, because going the other way isn't implemented.   I didn't write anything that 
could stash an arbitrary `lldb_private::File` in a python object .   If you convert a non-python `File`, you will get a new python file based on the descriptor, if it's available, or the conversion will fail if one is not.   We do test that here, on line 801, we're testing that a `NativeFile` is converted to a working python file and the file descriptors match.

We could, in a follow-on patch make the other direction work with identity too, but right now I can't think of what it would be useful for.


================
Comment at: lldb/scripts/interface/SBFile.i:81
+
+    %feature("docstring", "convert this SBFile into a python io.IOBase file object");
+    FileSP GetFile();
----------------
labath wrote:
> I am somewhat surprised that there's no ownership handling in this patch (whereas ownership transfer was a fairly big chunk of the conversion in the other direction). Could you maybe explain what are the ownership expectations of the file returned through here (and what happens when we close it for instance)?
Oh, yea good point.    `GetFile` returns a borrowed file, it shouldn't be closed.   I'll add a comment explaining.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68737