SBFile::GetFile: convert SBFile back into python native files.

Summary:
This makes SBFile::GetFile public and adds a SWIG typemap to convert
the result back into a python native file.

If the underlying File itself came from a python file, it is returned
identically.   Otherwise a new python file object is created using
the file descriptor.

Reviewers: JDevlieghere, jasonmolenda, labath

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D68737

llvm-svn: 374911

Author: lawrence-danna-apple

lldb/include/lldb/API/SBFile.h

@@ -36,6 +36,8 @@ class LLDB_API SBFile {
   operator bool() const;
   bool operator!() const;
 
+  FileSP GetFile() const;
+
 private:
   FileSP m_opaque_sp;
 };

lldb/include/lldb/Host/File.h

@@ -62,6 +62,8 @@ class File : public IOObject {
   static mode_t ConvertOpenOptionsForPOSIXOpen(OpenOptions open_options);
   static llvm::Expected<OpenOptions> GetOptionsFromMode(llvm::StringRef mode);
   static bool DescriptorIsValid(int descriptor) { return descriptor >= 0; };
+  static llvm::Expected<const char *>
+  GetStreamOpenModeFromOptions(OpenOptions options);
 
   File()
       : IOObject(eFDTypeFile), m_is_interactive(eLazyBoolCalculate),
@@ -317,6 +319,25 @@ class File : public IOObject {
   ///     format string \a format.
   virtual size_t PrintfVarArg(const char *format, va_list args);
 
+  /// Return the OpenOptions for this file.
+  ///
+  /// Some options like eOpenOptionDontFollowSymlinks only make
+  /// sense when a file is being opened (or not at all)
+  /// and may not be preserved for this method.  But any valid
+  /// File should return either or both of eOpenOptionRead and
+  /// eOpenOptionWrite here.
+  ///
+  /// \return
+  ///    OpenOptions flags for this file, or an error.
+  virtual llvm::Expected<OpenOptions> GetOptions() const;
+
+  llvm::Expected<const char *> GetOpenMode() const {
+    auto opts = GetOptions();
+    if (!opts)
+      return opts.takeError();
+    return GetStreamOpenModeFromOptions(opts.get());
+  }
+
   /// Get the permissions for a this file.
   ///
   /// \return
@@ -352,6 +373,10 @@ class File : public IOObject {
 
   bool operator!() const { return !IsValid(); };
 
+  static char ID;
+  virtual bool isA(const void *classID) const { return classID == &ID; }
+  static bool classof(const File *file) { return file->isA(&ID); }
+
 protected:
   LazyBool m_is_interactive;
   LazyBool m_is_real_terminal;
@@ -399,6 +424,13 @@ class NativeFile : public File {
   Status Flush() override;
   Status Sync() override;
   size_t PrintfVarArg(const char *format, va_list args) override;
+  llvm::Expected<OpenOptions> GetOptions() const override;
+
+  static char ID;
+  virtual bool isA(const void *classID) const override {
+    return classID == &ID || File::isA(classID);
+  }
+  static bool classof(const File *file) { return file->isA(&ID); }
 
 protected:
   bool DescriptorIsValid() const {

lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py

@@ -772,7 +772,21 @@ def test_stdout(self):
 
 
     @add_test_categories(['pyapi'])
-    @expectedFailureAll() # FIXME implement SBFile::GetFile
+    def test_stdout_file(self):
+        with open(self.out_filename, 'w') as f:
+            status = self.debugger.SetOutputFile(f)
+            self.assertTrue(status.Success())
+            self.handleCmd(r"script sys.stdout.write('foobar\n')")
+        with open(self.out_filename, 'r') as f:
+            # In python2 sys.stdout.write() returns None, which
+            # the REPL will ignore, but in python3 it will
+            # return the number of bytes written, which the REPL
+            # will print out.
+            lines = [x for x in f.read().strip().split() if x != "7"]
+            self.assertEqual(lines, ["foobar"])
+
+
+    @add_test_categories(['pyapi'])
     @skipIf(py_version=['<', (3,)])
     def test_identity(self):
 
@@ -826,3 +840,22 @@ def test_identity(self):
 
         with open(self.out_filename, 'r') as f:
             self.assertEqual("foobar", f.read().strip())
+
+
+    @add_test_categories(['pyapi'])
+    def test_back_and_forth(self):
+        with open(self.out_filename, 'w') as f:
+            # at each step here we're borrowing the file, so we have to keep
+            # them all alive until the end.
+            sbf = lldb.SBFile.Create(f, borrow=True)
+            def i(sbf):
+                for i in range(10):
+                    f = sbf.GetFile()
+                    yield f
+                    sbf = lldb.SBFile.Create(f, borrow=True)
+                    yield sbf
+                    sbf.Write(str(i).encode('ascii') + b"\n")
+            files = list(i(sbf))
+        with open(self.out_filename, 'r') as f:
+            self.assertEqual(list(range(10)), list(map(int, f.read().strip().split())))
+

lldb/scripts/Python/python-typemaps.swig

@@ -434,6 +434,22 @@ bool SetNumberFromPyObject<double>(double &number, PyObject *obj) {
   }
 }
 
+%typemap(out) lldb::FileSP {
+  using namespace lldb_private;
+  $result = nullptr;
+  lldb::FileSP &sp = $1;
+  if (sp) {
+    PythonFile pyfile = unwrapOrSetPythonException(PythonFile::FromFile(*sp));
+    if (!pyfile.IsValid())
+      return nullptr;
+    $result = pyfile.release();
+  }
+  if (!$result)
+  {
+      $result = Py_None;
+      Py_INCREF(Py_None);
+  }
+}
 
 // FIXME both of these paths wind up calling fdopen() with no provision for ever calling
 // fclose() on the result.  SB interfaces that use FILE* should be deprecated for scripting

lldb/scripts/interface/SBFile.i

@@ -77,6 +77,23 @@ public:
     operator bool() const;
 
     SBError Close();
+
+    %feature("docstring", "
+    Convert this SBFile into a python io.IOBase file object.
+
+    If the SBFile is itself a wrapper around a python file object,
+    this will return that original object.
+
+    The file returned from here should be considered borrowed,
+    in the sense that you may read and write to it, and flush it,
+    etc, but you should not close it.   If you want to close the
+    SBFile, call SBFile.Close().
+
+    If there is no underlying python file to unwrap, GetFile will
+    use the file descriptor, if availble to create a new python
+    file object using `open(fd, mode=..., closefd=False)`
+    ");
+    FileSP GetFile();
 };
 
 } // namespace lldb

lldb/source/API/SBFile.cpp

@@ -108,6 +108,11 @@ bool SBFile::operator!() const {
   return LLDB_RECORD_RESULT(!IsValid());
 }
 
+FileSP SBFile::GetFile() const {
+  LLDB_RECORD_METHOD_CONST_NO_ARGS(FileSP, SBFile, GetFile);
+  return m_opaque_sp;
+}
+
 namespace lldb_private {
 namespace repro {
 
@@ -117,6 +122,7 @@ template <> void RegisterMethods<SBFile>(Registry &R) {
   LLDB_REGISTER_METHOD_CONST(bool, SBFile, IsValid, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBFile, operator bool,());
   LLDB_REGISTER_METHOD_CONST(bool, SBFile, operator!,());
+  LLDB_REGISTER_METHOD_CONST(FileSP, SBFile, GetFile, ());
   LLDB_REGISTER_METHOD(lldb::SBError, SBFile, Close, ());
 }
 } // namespace repro

lldb/source/Host/common/File.cpp

@@ -39,7 +39,8 @@ using namespace lldb;
 using namespace lldb_private;
 using llvm::Expected;
 
-static Expected<const char *> GetStreamOpenModeFromOptions(uint32_t options) {
+Expected<const char *>
+File::GetStreamOpenModeFromOptions(File::OpenOptions options) {
   if (options & File::eOpenOptionAppend) {
     if (options & File::eOpenOptionRead) {
       if (options & File::eOpenOptionCanCreateNewOnly)
@@ -226,6 +227,12 @@ size_t File::PrintfVarArg(const char *format, va_list args) {
   return result;
 }
 
+Expected<File::OpenOptions> File::GetOptions() const {
+  return llvm::createStringError(
+      llvm::inconvertibleErrorCode(),
+      "GetOptions() not implemented for this File class");
+}
+
 uint32_t File::GetPermissions(Status &error) const {
   int fd = GetDescriptor();
   if (!DescriptorIsValid(fd)) {
@@ -241,6 +248,8 @@ uint32_t File::GetPermissions(Status &error) const {
   return file_stats.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO);
 }
 
+Expected<File::OpenOptions> NativeFile::GetOptions() const { return m_options; }
+
 int NativeFile::GetDescriptor() const {
   if (DescriptorIsValid())
     return m_descriptor;
@@ -758,3 +767,6 @@ mode_t File::ConvertOpenOptionsForPOSIXOpen(OpenOptions open_options) {
 
   return mode;
 }
+
+char File::ID = 0;
+char NativeFile::ID = 0;

lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

@@ -22,6 +22,7 @@
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/Errno.h"
 
@@ -1012,8 +1013,6 @@ operator()(std::initializer_list<PythonObject> args) {
 
 PythonFile::PythonFile() : PythonObject() {}
 
-PythonFile::PythonFile(File &file, const char *mode) { Reset(file, mode); }
-
 PythonFile::PythonFile(PyRefType type, PyObject *o) { Reset(type, o); }
 
 PythonFile::~PythonFile() {}
@@ -1063,25 +1062,6 @@ void PythonFile::Reset(PyRefType type, PyObject *py_obj) {
   PythonObject::Reset(PyRefType::Borrowed, result.get());
 }
 
-void PythonFile::Reset(File &file, const char *mode) {
-  if (!file.IsValid()) {
-    Reset();
-    return;
-  }
-
-  char *cmode = const_cast<char *>(mode);
-#if PY_MAJOR_VERSION >= 3
-  Reset(PyRefType::Owned, PyFile_FromFd(file.GetDescriptor(), nullptr, cmode,
-                                        -1, nullptr, "ignore", nullptr, 0));
-#else
-  // Read through the Python source, doesn't seem to modify these strings
-  Reset(PyRefType::Owned,
-        PyFile_FromFile(file.GetStream(), const_cast<char *>(""), cmode,
-                        nullptr));
-#endif
-}
-
-
 FileUP PythonFile::GetUnderlyingFile() const {
   if (!IsValid())
     return nullptr;
@@ -1238,6 +1218,13 @@ template <typename Base> class OwnedPythonFile : public Base {
     return base_error;
   };
 
+  PyObject *GetPythonObject() const {
+    assert(m_py_obj.IsValid());
+    return m_py_obj.get();
+  }
+
+  static bool classof(const File *file) = delete;
+
 protected:
   PythonFile m_py_obj;
   bool m_borrowed;
@@ -1252,7 +1239,14 @@ class SimplePythonFile : public OwnedPythonFile<NativeFile> {
   SimplePythonFile(const PythonFile &file, bool borrowed, int fd,
                    File::OpenOptions options)
       : OwnedPythonFile(file, borrowed, fd, options, false) {}
+
+  static char ID;
+  bool isA(const void *classID) const override {
+    return classID == &ID || NativeFile::isA(classID);
+  }
+  static bool classof(const File *file) { return file->isA(&ID); }
 };
+char SimplePythonFile::ID = 0;
 } // namespace
 
 #if PY_MAJOR_VERSION >= 3
@@ -1321,7 +1315,18 @@ class PythonIOFile : public OwnedPythonFile<File> {
     return Status();
   }
 
+  Expected<File::OpenOptions> GetOptions() const override {
+    GIL takeGIL;
+    return GetOptionsForPyObject(m_py_obj);
+  }
+
+  static char ID;
+  bool isA(const void *classID) const override {
+    return classID == &ID || File::isA(classID);
+  }
+  static bool classof(const File *file) { return file->isA(&ID); }
 };
+char PythonIOFile::ID = 0;
 } // namespace
 
 namespace {
@@ -1542,4 +1547,42 @@ PythonFile::ConvertToFileForcingUseOfScriptingIOMethods(bool borrowed) {
 #endif
 }
 
+Expected<PythonFile> PythonFile::FromFile(File &file, const char *mode) {
+  if (!file.IsValid())
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                   "invalid file");
+
+  if (auto *simple = llvm::dyn_cast<SimplePythonFile>(&file))
+    return Retain<PythonFile>(simple->GetPythonObject());
+#if PY_MAJOR_VERSION >= 3
+  if (auto *pythonio = llvm::dyn_cast<PythonIOFile>(&file))
+    return Retain<PythonFile>(pythonio->GetPythonObject());
+#endif
+
+  if (!mode) {
+    auto m = file.GetOpenMode();
+    if (!m)
+      return m.takeError();
+    mode = m.get();
+  }
+
+  PyObject *file_obj;
+#if PY_MAJOR_VERSION >= 3
+  file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
+                           "ignore", nullptr, 0);
+#else
+  // Read through the Python source, doesn't seem to modify these strings
+  char *cmode = const_cast<char *>(mode);
+  // We pass ::flush instead of ::fclose here so we borrow the FILE* --
+  // the lldb_private::File still owns it.
+  file_obj =
+      PyFile_FromFile(file.GetStream(), const_cast<char *>(""), cmode, ::fflush);
+#endif
+
+  if (!file_obj)
+    return exception();
+
+  return Take<PythonFile>(file_obj);
+}
+
 #endif

lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h

@@ -638,7 +638,7 @@ class PythonCallable : public PythonObject {
   void Reset(PyRefType type, PyObject *py_obj) override;
 
   ArgInfo GetNumArguments() const;
-  
+
   // If the callable is a Py_Class, then find the number of arguments
   // of the __init__ method.
   ArgInfo GetNumInitArguments() const;
@@ -658,7 +658,6 @@ class PythonCallable : public PythonObject {
 class PythonFile : public PythonObject {
 public:
   PythonFile();
-  PythonFile(File &file, const char *mode);
   PythonFile(PyRefType type, PyObject *o);
 
   ~PythonFile() override;
@@ -668,7 +667,21 @@ class PythonFile : public PythonObject {
   using PythonObject::Reset;
 
   void Reset(PyRefType type, PyObject *py_obj) override;
-  void Reset(File &file, const char *mode);
+
+  static llvm::Expected<PythonFile> FromFile(File &file,
+                                             const char *mode = nullptr);
+
+  // FIXME delete this after FILE* typemaps are deleted
+  // and ScriptInterpreterPython is fixed
+  PythonFile(File &file, const char *mode = nullptr) {
+    auto f = FromFile(file, mode);
+    if (f)
+      *this = std::move(f.get());
+    else {
+      Reset();
+      llvm::consumeError(f.takeError());
+    }
+  }
 
   lldb::FileUP GetUnderlyingFile() const;