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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I haven't looked at the patch in detail, but I have one high-level question. It seems to be that instead of subclassing a fully-functional File class, it would be better to create an abstract class, that both `File` and the new python thingy could implement. That would also create a natural place to write down the FILE* conversion semantics and other stuff we talked about at lldb-dev. I don't have any opinion as to whether the name `File` should denote the new abstract class, or the real thing, but it should most likely be done as a separate patch.

What do you think about that?



================
Comment at: lldb/include/lldb/API/SBFile.h:16-21
+/* These tags make no difference at the c++ level, but
+ * when the constructors are called from python they control
+ * how python files are converted by SWIG into FileSP */
+struct FileBorrow {};
+struct FileForceScriptingIO {};
+struct FileBorrowAndForceScriptingIO {};
----------------
I don't think we have anything similar to this in any of our SB classes. It might be better to avoid it. Could the same thing be achieved e.g. with a static factory function (`static SBFile SBFile::Create(FileSP file_sp, bool borrow, bool force_scripting_io)`) ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188