lawrence_danna marked 6 inline comments as done.
lawrence_danna added inline comments.
================
Comment at:
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1238-1245
+// OwnedPythonFile<Base>::IsValid() chains into Base::IsValid()
+// File::IsValid() is false by default, but for the following classes
+// we want the file to be considered valid as long as the python bits
+// are valid.
+class PresumptivelyValidFile : public File {
+public:
+ bool IsValid() const override { return true; };
----------------
labath wrote:
> How about if OwnedPythonFile defines `IsValid()` as `IsPythonObjectValid() ||
> Base::IsValid()`. Then PythonIOFile could redefine it to be simply
> `IsPythonObjectValid()` without the need for the extra class?
If I did that then a SimplePythonFile would still be valid if the file was
closed on the python side... seems like the wrong behavior.
================
Comment at:
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1251
+ PythonIOFile(const PythonFile &file, bool borrowed)
+ : OwnedPythonFile(file, borrowed){};
+
----------------
labath wrote:
> no semicolon
huh?
================
Comment at:
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1278
+class BinaryPythonFile : public PythonIOFile {
+ friend class PythonFile;
+
----------------
labath wrote:
> What is this needed for? I don't see the PythonFile class doing anything
> funny (and it probably shouldn't).
python API functions expect you to check for an exception with PyErr_Occured()
and clear it before calling into them again. If you don't things usually work
out OK, except on a debug build in which an assert will trip.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68188/new/
https://reviews.llvm.org/D68188
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
