lawrence_danna marked an inline comment as done.
lawrence_danna added inline comments.
================
Comment at:
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py:854
files = list(i(sbf))
+ # delete them in reverse order, again because each is a borrow
+ # of the previous.
----------------
lawrence_danna wrote:
> mgorny wrote:
> > For the record, this doesn't really guarantee specific order of
> > destruction. Generally, in Python code you shouldn't rely on destructors
> > being called at all and close files manually. That's why `with` is commonly
> > used with files.
> The point is not to rely on the file being closed at a certain time. They
> point is that each file added to the list is a borrowed reference to the
> previous one, and we should not allow those references to become dangling by
> letting the older ones go to zero-references before the younger ones.
>
> Now that I say it, I wonder if it was a bad idea to expose this kind of C++
> object lifetime semantics to python programs.
> I put in `GetFile` mainly so that in the case that a SBFile was a proxy of a
> python file, you could get the python file back.
>
> It might be better to restrict it to that case, and return None instead of a
> borrowed file in other cases.
>
> On the other hand, there's value in having a consistent API. Under the
> current semantics this just works:
>
> ```
> print("foo", file=debugger.GetOutputFile().GetFile())
> ```
>
> Overall I think I'd lean towards keeping GetFile the way it is, but I can see
> the argument for restricting it.
On second second thought, there's a better way. We don't need to expose the
borrow semantics, and we don't need to restrict GetFile like that. We can
do the same thing File.cpp does when you ask it for a FILE* on a borrowed FD --
that is, use dup and create one that python can truly own.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69488/new/
https://reviews.llvm.org/D69488
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
