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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
