remove multi-argument form of PythonObject::Reset()

[lawrence_danna]

Summary

With this patch, only the no-argument form of Reset() remains in
PythonDataObjects. It also deletes PythonExceptionState in favor of
PythonException, because the only call-site of PythonExceptionState was
also using Reset, so I cleaned up both while I was there.

[lawrence_danna]

rebased

[labath]

No fudnamental issues with this patch (I'm ignoring the CStringArg topic here). I do think that it would be nice to have some more unit tests for the new APIs you're introducing (or elevating to API status). I'm mainly thinking of the PythonScript stuff (a test where we evaluate the script successfully) and the runString functions.

lldb/include/lldb/Interpreter/ScriptInterpreter.h

68–70

I should've mentioned this earlier, but llvm style is to generally use // comments https://llvm.org/docs/CodingStandards.html#comment-formatting.,

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

261–264

Do I understand correctly that the idea is to eventually replace these functions with constructor calls too?

1057

I just realized I don't actually know what this means. Is it that the object takes the GIL internally? Or that one must have the GIL taken before he starts to interact with it? Maybe it'd be better to clarify this in the class description and just delete these comments.

1064–1065

maybe just return toString(backtrace.takeError()) here?

1512

Use early return here (if (valid) return Success;)

1516–1517

if (Error error = ...)

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

331

Maybe it's time to move the entire PythonObject hierarchy into the python namespace? That would be consistent with some of the newer lldb plugins, which have all of their code inside a sub-namespace of lldb_private..

778

I'm wondering if there's a way to avoid passing in the function name argument. Perhaps if the callable is returned as a result of the script ("def foo: ...; return foo")? WDYT?

789–790

if (llvm::Error e = ...)

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

1169–1180

Would something like this also work?

llvm::Error error = handleErrors(return_value.takeError(), [](PythonException &E) {
  llvm::Error str = makeStringError(E.ReadBacktrace());
  if (!options.GetMaskoutErrors)
    E.Restore();
  return str;
});
return std::move(error);

1986–1987

If doesn't look like this Reset call is really needed here..

lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

789–802

Maybe:

EXPECT_THAT_EXPECTED(foo(),
  llvm::Failed<PythonException>(testing::Property(&PythonException::ReadBacktrace,
     testing::ContainsRegex(...) )) ));

[lawrence_danna]

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

1057

Almost everything in PythonDataObjects requires you to call it with the GIL already locked. The only things that don't are the File methods because they're called by things that don't know anything about python.

Normally when I need to do something like this I would use dispatch_once, or whatever the LLVM version of that is, so you avoid static initializers, but also initialize the thing in a thread-safe way.

But in this case we already know that this function can only be called by a thread that has the GIL, so it's safe to just initialize a global variable without further synchronization.

[lawrence_danna]

fixes

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

261–264

yea

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

778

I can't use return, but i can have the scripts put their function in a well-known output variable.

789–790

Is that style considered normative?

I have a strong preference against putting function calls that are being run for their side effects inside an if.

lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

789–802

that doesn't seem to support multiple regexes, and is doing it 5 times really any more clear than just doing it manually?

[lawrence_danna]

comment

[labath]

I'm at the airport, so I haven't had a chance to look over the full set of changes you made, but here's my responses to your comments.

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

778

Yea, I like that. Maybe instead of assigning the defined function to a well-known some variable, we could just use that name in the def declaration already? Also, aren't names starting with a single underscore considered "private" in python? Maybe it could just be "function" (or "main", or "script_main", ...)

789–790

I didn't find any mention of that in the official style guide, but i think there's a lot of informal consensus about that. I've seen a lot of reviewers asking for that style, and I've never seen anyone be asked to change it back.
The main advantage of that is that it reduces the scope of the Error object, which is particularly important given that Error objects tend to do funny things when being destroyed.

lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

789–802

I was thinking you would just do a ContainsRegex("line 3.*line 5.*line 7...") If you want to emulate the current behavior precisely, you could do a MatchesAll(HasSubstr("line 3"), HasSubstr("line 5"), ...).

The main advantage I see here is in the error message that you get when this thing fails. In the current setup, you'll get " expected 0xffff != 0xffff, got 0xffff == 0xffff". This expression will end up producing the full error string as the "obtained" value, which will hopefully give you some clue about what went wrong. Which is particuarly useful if the failure happens only on some bot, whose configuration you can't easily reproduce.

[lawrence_danna]

fixes

[lawrence_danna]

fixed

[lawrence_danna]

oops

[labath]

lldb/scripts/Python/python-extensions.swig

683–686 ↗

(On Diff #225800)

You don't have to do this if you don't want, but I couldn't help noticing that the "else" branch is completely unnecessary in all of these scriptlets.

lldb/scripts/Python/python-typemaps.swig

4 ↗

(On Diff #225800)

Could you delete these empty lines too. You don't have to delete absolutely all of them, if you think they're useful for better visual separation, but I definitely don't think empty lines at the very beginning of a block are useful.

lldb/scripts/Python/python-wrapper.swig

193–194 ↗

(On Diff #225800)

These even have a double empty line now. :)

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

261–264

awesome.

1057

Ok, thanks for explaining. The new comment is much clearer now (though I am not sure if it's even needed TBH).

1067–1069

This won't work. The reason why the Twine class is so efficient, is that it establishes links between various temporary objects. Those temporaries will be destroyed when the enclosing full expression is evaluated. This means it's never safe to store Twine objects as local variables.

1076

How likely are we to get another exception while trying to retrieve the backtrace? I am wondering if all this complexity is worth it...

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

26–27

No using declarations in headers.

lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

789–792

EXPECT_THAT_EXPECTED(As<long long>(factorial(5ll)), llvm::HasValue(120))

841–842

HasValue(foobarbaz)

[lawrence_danna]

fixes

lldb/scripts/Python/python-extensions.swig

683–686 ↗

(On Diff #225800)

good point.

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

1076

It's actually not that unusual that someone writes a __str__ method for their exception which is broken and also throws an exception.

1076

actually, it looks like traceback.print_exception already takes care of that situation, so you're right it's not very useful.

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

26–27

even an "Impl" header?

[labath]

lldb/scripts/Python/python-wrapper.swig

226 ↗

(On Diff #225942)

There are empty lines from this point on in this file. Before they made sense because they were separating the using directive from the actual code, but now they've just become useless empty lines at the start of a block. Please delete them.

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

26–27

Nope. The "impl" here comes from the class name, but this is still a header like any other. Theoretically you could also put everything declared in this header into the python namespace, but otoh we have to stop somewhere, and this patch has gotten quite big already.

[lawrence_danna]

removed blank lines from python-wrapper.swig

lldb/include/lldb/Interpreter/ScriptInterpreter.h

public:
   public:
     ExecuteScriptOptions()
         : m_enable_io(true), m_set_lldb_globals(true), m_maskout_errors(true) {}
 
     bool GetEnableIO() const { return m_enable_io; }
 
     bool GetSetLLDBGlobals() const { return m_set_lldb_globals; }
 
+    /* If this is true then any exceptions raised by the script will be
+     * cleared with PyErr_Clear().   If false then they will be left for
+     * the caller to clean up */
     bool GetMaskoutErrors() const { return m_maskout_errors; }
 
     ExecuteScriptOptions &SetEnableIO(bool enable) {
       m_enable_io = enable;
       return *this;
     }
 
     ExecuteScriptOptions &SetSetLLDBGlobals(bool set) {

lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt

 if(NOT LLDB_PYTHON_RELATIVE_PATH)
   message(FATAL_ERROR "LLDB_PYTHON_RELATIVE_PATH is not set.")
 endif()
 add_definitions(-DLLDB_PYTHON_RELATIVE_LIBDIR="${LLDB_PYTHON_RELATIVE_PATH}")
 
 add_lldb_library(lldbPluginScriptInterpreterPython PLUGIN
   PythonDataObjects.cpp
-  PythonExceptionState.cpp
   ScriptInterpreterPython.cpp
 
   LINK_LIBS
     lldbBreakpoint
     lldbCore
     lldbDataFormatters
     lldbHost
     lldbInterpreter
     lldbTarget
     ${PYTHON_LIBRARY}
 
   LINK_COMPONENTS
     Support
   )

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

 template <typename T> T Retain(PyObject *obj) {
   assert(obj);
   assert(!PyErr_Occurred());
   T thing(PyRefType::Borrowed, obj);
   assert(thing.IsValid());
   return std::move(thing);
 }
 
+inline llvm::Error nullDeref() {
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                 "A NULL PyObject* was dereferenced");
+}
+
+inline llvm::Error exception(const char *s = nullptr) {
+  return llvm::make_error<PythonException>(s);
+}
+
+inline llvm::Error keyError() {
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                 "key not in dict");
+}
+
 } // namespace python
 
 enum class PyInitialValue { Invalid, Empty };
 
 template <typename T, typename Enable = void> struct PythonFormat;
 
 template <> struct PythonFormat<unsigned long long> {
   static constexpr char format = 'K';
   static auto get(unsigned long long value) { return value; }
 };
 
 template <> struct PythonFormat<long long> {
   static constexpr char format = 'L';
   static auto get(long long value) { return value; }
 };
 
+template <> struct PythonFormat<PyObject *> {
+  static constexpr char format = 'O';
+  static auto get(PyObject *value) { return value; }
+};
+
 template <typename T>
 struct PythonFormat<
     T, typename std::enable_if<std::is_base_of<PythonObject, T>::value>::type> {
   static constexpr char format = 'O';
   static auto get(const T &value) { return value.get(); }
 };
 
 /* This is a helper class for functions that wrap python C API functions
public:
   template <typename T>
   CStringArg(T x,
              typename std::enable_if<std::is_convertible<T, llvm::Twine>::value,
                                      void *>::type = 0) {
     llvm::Twine twine(x);
     llvm::StringRef ref = twine.toNullTerminatedStringRef(storage);
     cstr = ref.data();
   }
+
   const char *str() const { return cstr; }
 };
 
 class PythonObject {
 public:
   PythonObject() : m_py_obj(nullptr) {}
 
-  PythonObject(PyRefType type, PyObject *py_obj) : m_py_obj(nullptr) {
-    Reset(type, py_obj);
+  PythonObject(PyRefType type, PyObject *py_obj) {
+    m_py_obj = py_obj;
+    // If this is a borrowed reference, we need to convert it to
+    // an owned reference by incrementing it.  If it is an owned
+    // reference (for example the caller allocated it with PyDict_New()
+    // then we must *not* increment it.
+    if (m_py_obj && Py_IsInitialized() && type == PyRefType::Borrowed)
+      Py_XINCREF(m_py_obj);
   }
 
   PythonObject(const PythonObject &rhs)
       : PythonObject(PyRefType::Borrowed, rhs.m_py_obj) {}
 
   PythonObject(PythonObject &&rhs) {
     m_py_obj = rhs.m_py_obj;
     rhs.m_py_obj = nullptr;
   }
 
   ~PythonObject() { Reset(); }
 
   void Reset() {
     if (m_py_obj && Py_IsInitialized())
       Py_DECREF(m_py_obj);
     m_py_obj = nullptr;
   }
 
-  void Reset(PyRefType type, PyObject *py_obj) {
-    if (py_obj == m_py_obj)
-      return;
-
-    if (Py_IsInitialized())
-      Py_XDECREF(m_py_obj);
-
-    m_py_obj = py_obj;
-
-    // If this is a borrowed reference, we need to convert it to
-    // an owned reference by incrementing it.  If it is an owned
-    // reference (for example the caller allocated it with PyDict_New()
-    // then we must *not* increment it.
-    if (m_py_obj && Py_IsInitialized() && type == PyRefType::Borrowed)
-      Py_XINCREF(m_py_obj);
-  }
-
   void Dump() const {
     if (m_py_obj)
       _PyObject_Dump(m_py_obj);
     else
       puts("NULL");
   }
 
   void Dump(Stream &strm) const;
template <typename T> T AsType() const {
     if (!T::Check(m_py_obj))
       return T();
     return T(PyRefType::Borrowed, m_py_obj);
   }
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
 protected:
-  static llvm::Error nullDeref() {
-    return llvm::createStringError(llvm::inconvertibleErrorCode(),
-                                   "A NULL PyObject* was dereferenced");
-  }
-  static llvm::Error exception(const char *s = nullptr) {
-    return llvm::make_error<PythonException>(s);
-  }
-  static llvm::Error keyError() {
-    return llvm::createStringError(llvm::inconvertibleErrorCode(),
-                                   "key not in dict");
-  }
 
 #if PY_MAJOR_VERSION < 3
   // The python 2 API declares some arguments as char* that should
   // be const char *, but it doesn't actually modify them.
   static char *py2_const_cast(const char *s) { return const_cast<char *>(s); }
 #else
   static const char *py2_const_cast(const char *s) { return s; }
 #endif
 
 public:
   template <typename... T>
   llvm::Expected<PythonObject> CallMethod(const char *name,
                                           const T &... t) const {
+    using namespace python;
     const char format[] = {'(', PythonFormat<T>::format..., ')', 0};
     PyObject *obj =
         PyObject_CallMethod(m_py_obj, py2_const_cast(name),
                             py2_const_cast(format), PythonFormat<T>::get(t)...);
     if (!obj)
       return exception();
     return python::Take<PythonObject>(obj);
   }
 
   template <typename... T>
   llvm::Expected<PythonObject> Call(const T &... t) const {
+    using namespace python;
     const char format[] = {'(', PythonFormat<T>::format..., ')', 0};
     PyObject *obj = PyObject_CallFunction(m_py_obj, py2_const_cast(format),
                                           PythonFormat<T>::get(t)...);
     if (!obj)
       return exception();
     return python::Take<PythonObject>(obj);
   }
 
   llvm::Expected<PythonObject> GetAttribute(CStringArg name) const {
+    using namespace python;
     if (!m_py_obj)
       return nullDeref();
     PyObject *obj = PyObject_GetAttrString(m_py_obj, name.str());
     if (!obj)
       return exception();
     return python::Take<PythonObject>(obj);
   }
 
   llvm::Expected<bool> IsTrue() {
+    using namespace python;
     if (!m_py_obj)
       return nullDeref();
     int r = PyObject_IsTrue(m_py_obj);
     if (r < 0)
       return exception();
     return !!r;
   }
 
   llvm::Expected<long long> AsLongLong() {
+    using namespace python;
     if (!m_py_obj)
       return nullDeref();
     assert(!PyErr_Occurred());
     long long r = PyLong_AsLongLong(m_py_obj);
     if (PyErr_Occurred())
       return exception();
     return r;
   }
 
   llvm::Expected<bool> IsInstance(const PythonObject &cls) {
+    using namespace python;
     if (!m_py_obj || !cls.IsValid())
       return nullDeref();
     int r = PyObject_IsInstance(m_py_obj, cls.get());
     if (r < 0)
       return exception();
     return !!r;
   }
 

 } // namespace python
 
 template <class T> class TypedPythonObject : public PythonObject {
 public:
   // override to perform implicit type conversions on Reset
   // This can be eliminated once we drop python 2 support.
   static void Convert(PyRefType &type, PyObject *&py_obj) {}
 
-  void Reset() { PythonObject::Reset(); }
-
-  void Reset(PyRefType type, PyObject *py_obj) = delete;
-
   TypedPythonObject(PyRefType type, PyObject *py_obj) {
     if (!py_obj)
       return;
     T::Convert(type, py_obj);
     if (T::Check(py_obj))
-      PythonObject::Reset(type, py_obj);
+      PythonObject::operator=(PythonObject(type, py_obj));
     else if (type == PyRefType::Owned)
       Py_DECREF(py_obj);
   }
 
   TypedPythonObject() {}
 };
 
 class PythonBytes : public TypedPythonObject<PythonBytes> {

 public:
   static char ID;
   const char *toCString() const;
   PythonException(const char *caller = nullptr);
   void Restore();
   ~PythonException();
   void log(llvm::raw_ostream &OS) const override;
   std::error_code convertToErrorCode() const override;
+  bool Matches(PyObject *exc) const;
+  std::string ReadBacktrace() const;
 };
 
 // This extracts the underlying T out of an Expected<T> and returns it.
 // If the Expected is an Error instead of a T, that error will be converted
 // into a python exception, and this will return a default-constructed T.
 //
 // This is appropriate for use right at the boundary of python calling into
 // C++, such as in a SWIG typemap.   In such a context you should simply

 // This is only here to help incrementally migrate old, exception-unsafe
 // code.
 template <typename T> T unwrapIgnoringErrors(llvm::Expected<T> expected) {
   if (expected)
     return std::move(expected.get());
   llvm::consumeError(expected.takeError());
   return T();
 }
+
+llvm::Expected<PythonObject> runStringOneLine(CStringArg string,
+                                              const PythonDictionary &globals,
+                                              const PythonDictionary &locals);
+
+llvm::Expected<PythonObject> runStringMultiLine(CStringArg string,
+                                                const PythonDictionary &globals,
+                                                const PythonDictionary &locals);
+
 } // namespace python
 
+/* Sometimes the best way to interact with a python interpreter is
+ * to run some python code.   You construct a PythonScript with
+ * script string and a function name, and you get a C++ callable
+ * object that calls the python function.
+ *
+ * Example:
+ *
+ * const char script[] = R"(
+ * def foo(x, y):
+ *    ....
+ * )";
+ *
+ * Expected<PythonObject> cpp_foo_wrapper(PythonObject x, PythonObject y) {
+ *   // global is protected by the GIL
+ *   static PythonScript foo(script, "foo")
+ *   return  foo(x, y);
+ * }
+ */
+class PythonScript {
+  const char *script;
+  const char *function_name;
+  PythonCallable function;
+
+  llvm::Error Init();
+
+public:
+  PythonScript(const char *script, const char *name)
+      : script(script), function_name(name), function() {}
+
+  template <typename... Args>
+  llvm::Expected<PythonObject> operator()(Args &&... args) {
+    llvm::Error e = Init();
+    if (e)
+      return std::move(e);
+    return function.Call(std::forward<Args>(args)...);
+  }
+};
+
 } // namespace lldb_private
 
 #endif
 
 #endif // LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_PYTHONDATAOBJECTS_H

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

 }
 
 size_t PythonBytes::GetSize() const {
   if (!IsValid())
     return 0;
   return PyBytes_Size(m_py_obj);
 }
 
 void PythonBytes::SetBytes(llvm::ArrayRef<uint8_t> bytes) {
   const char *data = reinterpret_cast<const char *>(bytes.data());
-  PyObject *py_bytes = PyBytes_FromStringAndSize(data, bytes.size());
-  PythonObject::Reset(PyRefType::Owned, py_bytes);
+  *this = Take<PythonBytes>(PyBytes_FromStringAndSize(data, bytes.size()));
 }
 
 StructuredData::StringSP PythonBytes::CreateStructuredString() const {
   StructuredData::StringSP result(new StructuredData::String);
   Py_ssize_t size;
   char *c;
   PyBytes_AsStringAndSize(m_py_obj, &c, &size);
   result->SetValue(std::string(c, size));
   return result;
if (overflow != 0) {
       result = static_cast<int64_t>(uval);
     }
     return result;
   }
   return UINT64_MAX;
 }
 
 void PythonInteger::SetInteger(int64_t value) {
-  PythonObject::Reset(PyRefType::Owned, PyLong_FromLongLong(value));
+  *this = Take<PythonInteger>(PyLong_FromLongLong(value));
 }
 
 StructuredData::IntegerSP PythonInteger::CreateStructuredInteger() const {
   StructuredData::IntegerSP result(new StructuredData::Integer);
   result->SetValue(GetInteger());
   return result;
 }
 
 // PythonBoolean
 
 PythonBoolean::PythonBoolean(bool value) {
   SetValue(value);
 }
 
 bool PythonBoolean::Check(PyObject *py_obj) {
   return py_obj ? PyBool_Check(py_obj) : false;
 }
 
 bool PythonBoolean::GetValue() const {
   return m_py_obj ? PyObject_IsTrue(m_py_obj) : false;
 }
 
 void PythonBoolean::SetValue(bool value) {
-  PythonObject::Reset(PyRefType::Owned, PyBool_FromLong(value));
+  *this = Take<PythonBoolean>(PyBool_FromLong(value));
 }
 
 StructuredData::BooleanSP PythonBoolean::CreateStructuredBoolean() const {
   StructuredData::BooleanSP result(new StructuredData::Boolean);
   result->SetValue(GetValue());
   return result;
 }
 

 Expected<PythonCallable::ArgInfo> PythonCallable::GetArgInfo() const {
   ArgInfo result = {};
   if (!IsValid())
     return nullDeref();
 
 #if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
 
   // this global is protected by the GIL
-  static PythonCallable get_arg_info;
-
-  if (!get_arg_info.IsValid()) {
-    PythonDictionary globals(PyInitialValue::Empty);
-
-    auto builtins = PythonModule::BuiltinsModule();
-    Error error = globals.SetItem("__builtins__", builtins);
-    if (error)
-      return std::move(error);
-    PyObject *o = PyRun_String(get_arg_info_script, Py_file_input,
-                               globals.get(), globals.get());
-    if (!o)
-      return exception();
-    Take<PythonObject>(o);
-    auto function = As<PythonCallable>(globals.GetItem("get_arg_info"));
-    if (!function)
-      return function.takeError();
-    get_arg_info = std::move(function.get());
-  }
-
-  Expected<PythonObject> pyarginfo = get_arg_info.Call(*this);
+  static PythonScript get_arg_info(get_arg_info_script, "get_arg_info");
+  Expected<PythonObject> pyarginfo = get_arg_info(*this);
   if (!pyarginfo)
     return pyarginfo.takeError();
   result.count = cantFail(As<long long>(pyarginfo.get().GetAttribute("count")));
   result.has_varargs =
       cantFail(As<bool>(pyarginfo.get().GetAttribute("has_varargs")));
   bool is_method =
       cantFail(As<bool>(pyarginfo.get().GetAttribute("is_bound_method")));
   result.max_positional_args =

 }
 
 void PythonException::log(llvm::raw_ostream &OS) const { OS << toCString(); }
 
 std::error_code PythonException::convertToErrorCode() const {
   return llvm::inconvertibleErrorCode();
 }
 
+bool PythonException::Matches(PyObject *exc) const {
+  return PyErr_GivenExceptionMatches(m_exception_type, exc);
+}
+
+const char read_exception_script[] = R"(
+import sys
+from traceback import print_exception
+if sys.version_info.major < 3:
+  from StringIO import StringIO
+else:
+  from io import StringIO
+def read_exception(exc_type, exc_value, tb):
+  f = StringIO()
+  print_exception(exc_type, exc_value, tb, file=f)
+  return f.getvalue()
+)";
+
+std::string PythonException::ReadBacktrace() const {
+
+  // global is protected by the GIL
+  static PythonScript read_exception(read_exception_script, "read_exception");
+
+  Expected<std::string> backtrace = As<std::string>(
+      read_exception(m_exception_type, m_exception, m_traceback));
+
+  if (!backtrace) {
+    llvm::consumeError(backtrace.takeError());
+    return "backtrace unavailable";
+  }
+
+  return std::move(backtrace.get());
+}
+
 char PythonException::ID = 0;
 
 llvm::Expected<File::OpenOptions>
 GetOptionsForPyObject(const PythonObject &obj) {
 #if PY_MAJOR_VERSION >= 3
   auto options = File::OpenOptions(0);
   auto readable = As<bool>(obj.CallMethod("readable"));
   if (!readable)
     return readable.takeError();
   auto writable = As<bool>(obj.CallMethod("writable"));
   if (!writable)
     return writable.takeError();
   if (readable.get())
     options |= File::eOpenOptionRead;

 #endif
 
   if (!file_obj)
     return exception();
 
   return Take<PythonFile>(file_obj);
 }
 
+Error PythonScript::Init() {
+  if (!function.IsValid()) {
+    PythonDictionary globals(PyInitialValue::Empty);
+
+    auto builtins = PythonModule::BuiltinsModule();
+    Error error = globals.SetItem("__builtins__", builtins);
+    if (error)
+      return error;
+    PyObject *o =
+        PyRun_String(script, Py_file_input, globals.get(), globals.get());
+    if (!o)
+      return exception();
+    Take<PythonObject>(o);
+    auto f = As<PythonCallable>(globals.GetItem(function_name));
+    if (!f)
+      return f.takeError();
+    function = std::move(f.get());
+  }
+  return Error::success();
+}
+
+llvm::Expected<PythonObject>
+python::runStringOneLine(CStringArg string, const PythonDictionary &globals,
+                         const PythonDictionary &locals) {
+  if (!globals.IsValid() || !locals.IsValid())
+    return nullDeref();
+
+  PyObject *code = Py_CompileString(string.str(), "<string>", Py_eval_input);
+  if (!code) {
+    PyErr_Clear();
+    code = Py_CompileString(string.str(), "<string>", Py_single_input);
+  }
+  if (!code)
+    return exception();
+  auto code_ref = Take<PythonObject>(code);
+
+#if PY_MAJOR_VERSION < 3
+  PyObject *result = PyEval_EvalCode((PyCodeObject*)code, globals.get(), locals.get());
+#else
+  PyObject *result = PyEval_EvalCode(code, globals.get(), locals.get());
+#endif
+
+  if (!result)
+    return exception();
+
+  return Take<PythonObject>(result);
+}
+
+llvm::Expected<PythonObject>
+python::runStringMultiLine(CStringArg string, const PythonDictionary &globals,
+                           const PythonDictionary &locals) {
+  if (!globals.IsValid() || !locals.IsValid())
+    return nullDeref();
+  PyObject *result =
+      PyRun_String(string.str(), Py_file_input, globals.get(), locals.get());
+  if (!result)
+    return exception();
+  return Take<PythonObject>(result);
+}
+
 #endif

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

-//===-- PythonExceptionState.h ----------------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_PYTHONEXCEPTIONSTATE_H
-#define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_PYTHONEXCEPTIONSTATE_H
-
-#ifndef LLDB_DISABLE_PYTHON
-
-#include "PythonDataObjects.h"
-
-namespace lldb_private {
-
-class PythonExceptionState {
-public:
-  explicit PythonExceptionState(bool restore_on_exit);
-  ~PythonExceptionState();
-
-  void Acquire(bool restore_on_exit);
-
-  void Restore();
-
-  void Discard();
-
-  void Reset();
-
-  static bool HasErrorOccurred();
-
-  bool IsError() const;
-
-  PythonObject GetType() const;
-
-  PythonObject GetValue() const;
-
-  PythonObject GetTraceback() const;
-
-  std::string Format() const;
-
-private:
-  std::string ReadBacktrace() const;
-
-  bool m_restore_on_exit;
-
-  PythonObject m_type;
-  PythonObject m_value;
-  PythonObject m_traceback;
-};
-}
-
-#endif
-
-#endif

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

-//===-- PythonExceptionState.cpp --------------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLDB_DISABLE_PYTHON
-
-// LLDB Python header must be included first
-#include "lldb-python.h"
-
-#include "PythonExceptionState.h"
-
-#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/raw_ostream.h"
-
-using namespace lldb_private;
-
-PythonExceptionState::PythonExceptionState(bool restore_on_exit)
-    : m_restore_on_exit(restore_on_exit) {
-  Acquire(restore_on_exit);
-}
-
-PythonExceptionState::~PythonExceptionState() {
-  if (m_restore_on_exit)
-    Restore();
-}
-
-void PythonExceptionState::Acquire(bool restore_on_exit) {
-  // If a state is already acquired, the user needs to decide whether they want
-  // to discard or restore it.  Don't allow the potential silent loss of a
-  // valid state.
-  assert(!IsError());
-
-  if (!HasErrorOccurred())
-    return;
-
-  PyObject *py_type = nullptr;
-  PyObject *py_value = nullptr;
-  PyObject *py_traceback = nullptr;
-  PyErr_Fetch(&py_type, &py_value, &py_traceback);
-  // PyErr_Fetch clears the error flag.
-  assert(!HasErrorOccurred());
-
-  // Ownership of the objects returned by `PyErr_Fetch` is transferred to us.
-  m_type.Reset(PyRefType::Owned, py_type);
-  m_value.Reset(PyRefType::Owned, py_value);
-  m_traceback.Reset(PyRefType::Owned, py_traceback);
-  m_restore_on_exit = restore_on_exit;
-}
-
-void PythonExceptionState::Restore() {
-  if (m_type.IsValid()) {
-    // The documentation for PyErr_Restore says "Do not pass a null type and
-    // non-null value or traceback.  So only restore if type was non-null to
-    // begin with.  In this case we're passing ownership back to Python so
-    // release them all.
-    PyErr_Restore(m_type.release(), m_value.release(), m_traceback.release());
-  }
-
-  // After we restore, we should not hold onto the exception state.  Demand
-  // that it be re-acquired.
-  Discard();
-}
-
-void PythonExceptionState::Discard() {
-  m_type.Reset();
-  m_value.Reset();
-  m_traceback.Reset();
-}
-
-void PythonExceptionState::Reset() {
-  if (m_restore_on_exit)
-    Restore();
-  else
-    Discard();
-}
-
-bool PythonExceptionState::HasErrorOccurred() { return PyErr_Occurred(); }
-
-bool PythonExceptionState::IsError() const {
-  return m_type.IsValid() || m_value.IsValid() || m_traceback.IsValid();
-}
-
-PythonObject PythonExceptionState::GetType() const { return m_type; }
-
-PythonObject PythonExceptionState::GetValue() const { return m_value; }
-
-PythonObject PythonExceptionState::GetTraceback() const { return m_traceback; }
-
-std::string PythonExceptionState::Format() const {
-  // Don't allow this function to modify the error state.
-  PythonExceptionState state(true);
-
-  std::string backtrace = ReadBacktrace();
-  if (!IsError())
-    return std::string();
-
-  // It's possible that ReadPythonBacktrace generated another exception. If
-  // this happens we have to clear the exception, because otherwise
-  // PyObject_Str() will assert below.  That's why we needed to do the save /
-  // restore at the beginning of this function.
-  PythonExceptionState bt_error_state(false);
-
-  std::string error_string;
-  llvm::raw_string_ostream error_stream(error_string);
-  error_stream << m_value.Str().GetString() << "\n";
-
-  if (!bt_error_state.IsError()) {
-    // If we were able to read the backtrace, just append it.
-    error_stream << backtrace << "\n";
-  } else {
-    // Otherwise, append some information about why we were unable to obtain
-    // the backtrace.
-    PythonString bt_error = bt_error_state.GetValue().Str();
-    error_stream << "An error occurred while retrieving the backtrace: "
-                 << bt_error.GetString() << "\n";
-  }
-  return error_stream.str();
-}
-
-std::string PythonExceptionState::ReadBacktrace() const {
-  std::string retval("backtrace unavailable");
-
-  auto traceback_module = PythonModule::ImportModule("traceback");
-#if PY_MAJOR_VERSION >= 3
-  auto stringIO_module = PythonModule::ImportModule("io");
-#else
-  auto stringIO_module = PythonModule::ImportModule("StringIO");
-#endif
-  if (!m_traceback.IsAllocated())
-    return retval;
-
-  if (!traceback_module.IsAllocated() || !stringIO_module.IsAllocated())
-    return retval;
-
-  auto stringIO_builder =
-      stringIO_module.ResolveName<PythonCallable>("StringIO");
-  if (!stringIO_builder.IsAllocated())
-    return retval;
-
-  auto stringIO_buffer = stringIO_builder();
-  if (!stringIO_buffer.IsAllocated())
-    return retval;
-
-  auto printTB = traceback_module.ResolveName<PythonCallable>("print_tb");
-  if (!printTB.IsAllocated())
-    return retval;
-
-  auto printTB_result =
-      printTB(m_traceback.get(), Py_None, stringIO_buffer.get());
-  auto stringIO_getvalue =
-      stringIO_buffer.ResolveName<PythonCallable>("getvalue");
-  if (!stringIO_getvalue.IsAllocated())
-    return retval;
-
-  auto printTB_string = stringIO_getvalue().AsType<PythonString>();
-  if (!printTB_string.IsAllocated())
-    return retval;
-
-  llvm::StringRef string_data(printTB_string.GetString());
-  retval.assign(string_data.data(), string_data.size());
-
-  return retval;
-}
-
-#endif

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

 // Python is disabled in this build
 
 #else
 
 // LLDB Python header must be included first
 #include "lldb-python.h"
 
 #include "PythonDataObjects.h"
-#include "PythonExceptionState.h"
 #include "ScriptInterpreterPythonImpl.h"
 
 #include "lldb/API/SBFrame.h"
 #include "lldb/API/SBValue.h"
 #include "lldb/Breakpoint/StoppointCallbackContext.h"
 #include "lldb/Breakpoint/WatchpointOptions.h"
 #include "lldb/Core/Communication.h"
 #include "lldb/Core/Debugger.h"

 #include <mutex>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string>
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::python;
+using llvm::Expected;
 
 // Defined in the SWIG source file
 #if PY_MAJOR_VERSION >= 3
 extern "C" PyObject *PyInit__lldb(void);
 
 #define LLDBSwigPyInit PyInit__lldb
 
 #else
bool ScriptInterpreterPythonImpl::EnterSession(uint16_t on_entry_flags,
   }
 
   if (PyErr_Occurred())
     PyErr_Clear();
 
   return true;
 }
 
-PythonObject &ScriptInterpreterPythonImpl::GetMainModule() {
+PythonModule &ScriptInterpreterPythonImpl::GetMainModule() {
   if (!m_main_module.IsValid())
-    m_main_module.Reset(PyRefType::Borrowed, PyImport_AddModule("__main__"));
+    m_main_module = unwrapIgnoringErrors(PythonModule::Import("__main__"));
   return m_main_module;
 }
 
 PythonDictionary &ScriptInterpreterPythonImpl::GetSessionDictionary() {
   if (m_session_dict.IsValid())
     return m_session_dict;
 
   PythonObject &main_module = GetMainModule();
if (state) {
       return true;
     }
   }
   LLDB_LOGF(log,
             "ScriptInterpreterPythonImpl::Interrupt() python code not running, "
             "can't interrupt");
   return false;
 }
+
 bool ScriptInterpreterPythonImpl::ExecuteOneLineWithReturn(
     llvm::StringRef in_string, ScriptInterpreter::ScriptReturnType return_type,
     void *ret_value, const ExecuteScriptOptions &options) {
 
   Locker locker(this,
                 Locker::AcquireLock | Locker::InitSession |
                     (options.GetSetLLDBGlobals() ? Locker::InitGlobals : 0) |
                     Locker::NoSTDIN,
                 Locker::FreeAcquiredLock | Locker::TearDownSession);
 
-  PythonObject py_return;
-  PythonObject &main_module = GetMainModule();
-  PythonDictionary globals(PyRefType::Borrowed,
-                           PyModule_GetDict(main_module.get()));
-  PythonObject py_error;
-  bool ret_success = false;
-  int success;
+  PythonModule &main_module = GetMainModule();
+  PythonDictionary globals = main_module.GetDictionary();
 
   PythonDictionary locals = GetSessionDictionary();
-
-  if (!locals.IsValid()) {
+  if (!locals.IsValid())
     locals = unwrapIgnoringErrors(
         As<PythonDictionary>(globals.GetAttribute(m_dictionary_name)));
-  }
-
   if (!locals.IsValid())
     locals = globals;
 
-  py_error.Reset(PyRefType::Borrowed, PyErr_Occurred());
-  if (py_error.IsValid())
-    PyErr_Clear();
+  Expected<PythonObject> maybe_py_return =
+      runStringOneLine(in_string, globals, locals);
 
-  std::string as_string = in_string.str();
-  { // scope for PythonInputReaderManager
-    // PythonInputReaderManager py_input(options.GetEnableIO() ? this : NULL);
-    py_return.Reset(PyRefType::Owned,
-                    PyRun_String(as_string.c_str(), Py_eval_input,
-                                 globals.get(), locals.get()));
-    if (!py_return.IsValid()) {
-      py_error.Reset(PyRefType::Borrowed, PyErr_Occurred());
-      if (py_error.IsValid())
+  if (!maybe_py_return) {
+    llvm::handleAllErrors(
+        maybe_py_return.takeError(),
+        [&](PythonException &E) {
+          E.Restore();
+          if (options.GetMaskoutErrors()) {
+            if (E.Matches(PyExc_SyntaxError)) {
+              PyErr_Print();
+            }
             PyErr_Clear();
-
-      py_return.Reset(PyRefType::Owned,
-                      PyRun_String(as_string.c_str(), Py_single_input,
-                                   globals.get(), locals.get()));
           }
+        },
+        [](const llvm::ErrorInfoBase &E) {});
+    return false;
   }
 
-  if (py_return.IsValid()) {
-    switch (return_type) {
-    case eScriptReturnTypeCharPtr: // "char *"
-    {
-      const char format[3] = "s#";
-      success = PyArg_Parse(py_return.get(), format, (char **)ret_value);
-      break;
-    }
-    case eScriptReturnTypeCharStrOrNone: // char* or NULL if py_return ==
-                                         // Py_None
-    {
-      const char format[3] = "z";
-      success = PyArg_Parse(py_return.get(), format, (char **)ret_value);
-      break;
-    }
-    case eScriptReturnTypeBool: {
-      const char format[2] = "b";
-      success = PyArg_Parse(py_return.get(), format, (bool *)ret_value);
-      break;
-    }
-    case eScriptReturnTypeShortInt: {
-      const char format[2] = "h";
-      success = PyArg_Parse(py_return.get(), format, (short *)ret_value);
-      break;
-    }
-    case eScriptReturnTypeShortIntUnsigned: {
-      const char format[2] = "H";
-      success =
-          PyArg_Parse(py_return.get(), format, (unsigned short *)ret_value);
-      break;
-    }
-    case eScriptReturnTypeInt: {
-      const char format[2] = "i";
-      success = PyArg_Parse(py_return.get(), format, (int *)ret_value);
-      break;
-    }
-    case eScriptReturnTypeIntUnsigned: {
-      const char format[2] = "I";
-      success = PyArg_Parse(py_return.get(), format, (unsigned int *)ret_value);
-      break;
-    }
-    case eScriptReturnTypeLongInt: {
-      const char format[2] = "l";
-      success = PyArg_Parse(py_return.get(), format, (long *)ret_value);
-      break;
-    }
-    case eScriptReturnTypeLongIntUnsigned: {
-      const char format[2] = "k";
-      success =
-          PyArg_Parse(py_return.get(), format, (unsigned long *)ret_value);
-      break;
-    }
-    case eScriptReturnTypeLongLong: {
-      const char format[2] = "L";
-      success = PyArg_Parse(py_return.get(), format, (long long *)ret_value);
-      break;
-    }
-    case eScriptReturnTypeLongLongUnsigned: {
-      const char format[2] = "K";
-      success =
-          PyArg_Parse(py_return.get(), format, (unsigned long long *)ret_value);
-      break;
-    }
-    case eScriptReturnTypeFloat: {
-      const char format[2] = "f";
-      success = PyArg_Parse(py_return.get(), format, (float *)ret_value);
-      break;
-    }
-    case eScriptReturnTypeDouble: {
-      const char format[2] = "d";
-      success = PyArg_Parse(py_return.get(), format, (double *)ret_value);
-      break;
-    }
-    case eScriptReturnTypeChar: {
-      const char format[2] = "c";
-      success = PyArg_Parse(py_return.get(), format, (char *)ret_value);
-      break;
-    }
-    case eScriptReturnTypeOpaqueObject: {
-      success = true;
-      PyObject *saved_value = py_return.get();
-      Py_XINCREF(saved_value);
-      *((PyObject **)ret_value) = saved_value;
-      break;
-    }
-    }
-
-    ret_success = success;
-  }
-
-  py_error.Reset(PyRefType::Borrowed, PyErr_Occurred());
-  if (py_error.IsValid()) {
-    ret_success = false;
-    if (options.GetMaskoutErrors()) {
-      if (PyErr_GivenExceptionMatches(py_error.get(), PyExc_SyntaxError))
-        PyErr_Print();
-      PyErr_Clear();
-    }
+  PythonObject py_return = std::move(maybe_py_return.get());
+  assert(py_return.IsValid());
+
+    return PyArg_Parse(py_return.get(), format, (char **)ret_value);
+    return PyArg_Parse(py_return.get(), format, (char **)ret_value);
+    return PyArg_Parse(py_return.get(), format, (bool *)ret_value);
+    return PyArg_Parse(py_return.get(), format, (short *)ret_value);
+    return PyArg_Parse(py_return.get(), format, (unsigned short *)ret_value);
+    return PyArg_Parse(py_return.get(), format, (int *)ret_value);
+    return PyArg_Parse(py_return.get(), format, (unsigned int *)ret_value);
+    return PyArg_Parse(py_return.get(), format, (long *)ret_value);
+    return PyArg_Parse(py_return.get(), format, (unsigned long *)ret_value);
+    return PyArg_Parse(py_return.get(), format, (long long *)ret_value);
+    return PyArg_Parse(py_return.get(), format,
+                       (unsigned long long *)ret_value);
+    return PyArg_Parse(py_return.get(), format, (float *)ret_value);
+    return PyArg_Parse(py_return.get(), format, (double *)ret_value);
+    return PyArg_Parse(py_return.get(), format, (char *)ret_value);
+    *((PyObject **)ret_value) = py_return.release();
+    return true;
   }
-
-  return ret_success;
 }
 
 Status ScriptInterpreterPythonImpl::ExecuteMultipleLines(
     const char *in_string, const ExecuteScriptOptions &options) {
-  Status error;
+
+  if (in_string == nullptr)
+    return Status();
 
   Locker locker(this,
                 Locker::AcquireLock | Locker::InitSession |
                     (options.GetSetLLDBGlobals() ? Locker::InitGlobals : 0) |
                     Locker::NoSTDIN,
                 Locker::FreeAcquiredLock | Locker::TearDownSession);
 
-  PythonObject return_value;
-  PythonObject &main_module = GetMainModule();
-  PythonDictionary globals(PyRefType::Borrowed,
-                           PyModule_GetDict(main_module.get()));
-  PythonObject py_error;
+  PythonModule &main_module = GetMainModule();
+  PythonDictionary globals = main_module.GetDictionary();
 
   PythonDictionary locals = GetSessionDictionary();
-
   if (!locals.IsValid())
     locals = unwrapIgnoringErrors(
         As<PythonDictionary>(globals.GetAttribute(m_dictionary_name)));
-
   if (!locals.IsValid())
     locals = globals;
 
-  py_error.Reset(PyRefType::Borrowed, PyErr_Occurred());
-  if (py_error.IsValid())
-    PyErr_Clear();
-
-  if (in_string != nullptr) {
-    PythonObject code_object;
-    code_object.Reset(PyRefType::Owned,
-                      Py_CompileString(in_string, "temp.py", Py_file_input));
-
-    if (code_object.IsValid()) {
-// In Python 2.x, PyEval_EvalCode takes a PyCodeObject, but in Python 3.x, it
-// takes a PyObject.  They are convertible (hence the function
-// PyCode_Check(PyObject*), so we have to do the cast for Python 2.x
-#if PY_MAJOR_VERSION >= 3
-      PyObject *py_code_obj = code_object.get();
-#else
-      PyCodeObject *py_code_obj =
-          reinterpret_cast<PyCodeObject *>(code_object.get());
-#endif
-      return_value.Reset(
-          PyRefType::Owned,
-          PyEval_EvalCode(py_code_obj, globals.get(), locals.get()));
-    }
+  Expected<PythonObject> return_value =
+      runStringMultiLine(in_string, globals, locals);
+  if (!return_value) {
+    Status error;
+    llvm::handleAllErrors(
+        return_value.takeError(),
+        [&](PythonException &E) {
+          error.SetErrorString(E.ReadBacktrace());
+          if (!options.GetMaskoutErrors())
+            E.Restore();
+        },
+        [&](const llvm::ErrorInfoBase &E) {
+          error.SetErrorString(E.message());
+        });
+    return error;
   }
 
-  PythonExceptionState exception_state(!options.GetMaskoutErrors());
-  if (exception_state.IsError())
-    error.SetErrorString(exception_state.Format().c_str());
-
-  return error;
+  return Status();
 }
 
 void ScriptInterpreterPythonImpl::CollectDataForBreakpointCommandCallback(
     std::vector<BreakpointOptions *> &bp_options_vec,
     CommandReturnObject &result) {
   m_active_io_handler = eIOHandlerBreakpoint;
   m_debugger.GetCommandInterpreter().GetPythonCommandsFromIOHandler(
       "    ", *this, true, &bp_options_vec);
StructuredData::DictionarySP ScriptInterpreterPythonImpl::GetDynamicSettings(
   StructuredData::Generic *generic = plugin_module_sp->GetAsGeneric();
   if (!generic)
     return StructuredData::DictionarySP();
 
   PythonObject reply_pyobj;
   Locker py_lock(this,
                  Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
   TargetSP target_sp(target->shared_from_this());
-  reply_pyobj.Reset(PyRefType::Owned,
-                    (PyObject *)LLDBSWIGPython_GetDynamicSetting(
-                        generic->GetValue(), setting_name, target_sp));
+
+  auto setting = (PyObject *)LLDBSWIGPython_GetDynamicSetting(
+      generic->GetValue(), setting_name, target_sp);
+  if (setting)
+    reply_pyobj = Take<PythonObject>(setting);
+  else
+    reply_pyobj.Reset();
 
   PythonDictionary py_dict(PyRefType::Borrowed, reply_pyobj.get());
   return py_dict.CreateStructuredDictionary();
 }
 
 StructuredData::ObjectSP
 ScriptInterpreterPythonImpl::CreateSyntheticScriptedProvider(
     const char *class_name, lldb::ValueObjectSP valobj) {

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

 #include "ScriptInterpreterPython.h"
 
 #include "lldb/Host/Terminal.h"
 #include "lldb/Utility/StreamString.h"
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 
 namespace lldb_private {
 class IOHandlerPythonInterpreter;
 class ScriptInterpreterPythonImpl : public ScriptInterpreterPython {
 public:
   friend class IOHandlerPythonInterpreter;
 
   ScriptInterpreterPythonImpl(Debugger &debugger);
 
   ~ScriptInterpreterPythonImpl() override;
 
public:
   }
 
   enum ActiveIOHandler {
     eIOHandlerNone,
     eIOHandlerBreakpoint,
     eIOHandlerWatchpoint
   };
 
-  PythonObject &GetMainModule();
+  PythonModule &GetMainModule();
 
   PythonDictionary &GetSessionDictionary();
 
   PythonDictionary &GetSysModuleDictionary();
 
   bool GetEmbeddedInterpreterModuleObjects();
 
   bool SetStdHandle(lldb::FileSP file, const char *py_name,
                     PythonObject &save_file, const char *mode);
 
   PythonObject m_saved_stdin;
   PythonObject m_saved_stdout;
   PythonObject m_saved_stderr;
-  PythonObject m_main_module;
+  PythonModule m_main_module;
   PythonDictionary m_session_dict;
   PythonDictionary m_sys_module_dict;
   PythonObject m_run_one_line_function;
   PythonObject m_run_one_line_str_global;
   std::string m_dictionary_name;
   ActiveIOHandler m_active_io_handler;
   bool m_session_is_active;
   bool m_pty_slave_is_open;

lldb/unittests/ScriptInterpreter/Python/CMakeLists.txt

 add_lldb_unittest(ScriptInterpreterPythonTests
   PythonDataObjectsTests.cpp
-  PythonExceptionStateTests.cpp
   PythonTestSuite.cpp
 
   LINK_LIBS
     lldbHost
     lldbPluginScriptInterpreterPython
     LLVMTestingSupport
   LINK_COMPONENTS
     Support
   )
 No newline at end of file

lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

 #include "lldb/Host/HostInfo.h"
 #include "lldb/lldb-enumerations.h"
 #include "llvm/Testing/Support/Error.h"
 
 #include "PythonTestSuite.h"
 
 using namespace lldb_private;
 using namespace lldb_private::python;
+using llvm::Expected;
 
 class PythonDataObjectsTest : public PythonTestSuite {
 public:
   void SetUp() override {
     PythonTestSuite::SetUp();
 
     m_sys_module = unwrapIgnoringErrors(PythonModule::Import("sys"));
     m_main_module = PythonModule::MainModule();
// the old implementation of GetArgInfo just doesn't work on builtins.
     auto arginfo = hex.get().GetArgInfo();
     ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
     EXPECT_EQ(arginfo.get().count, 1);
     EXPECT_EQ(arginfo.get().max_positional_args, 1u);
     EXPECT_EQ(arginfo.get().has_varargs, false);
   }
 
 #endif
+}
+
+TEST_F(PythonDataObjectsTest, TestExceptions) {
+
+  static const char script[] = R"(
+def foo():
+  return bar()
+def bar():
+  return baz()
+def baz():
+  return 1 / 0
+)";
+
+  PythonScript foo(script, "foo");
+  Expected<PythonObject> r = foo();
+
+  bool failed = !r;
+  ASSERT_TRUE(failed);
+
+  std::string backtrace;
+  llvm::handleAllErrors(r.takeError(), [&](const PythonException &E) {
+    backtrace = E.ReadBacktrace();
+  });
+
+  EXPECT_NE(backtrace.find("line 3, in foo"), std::string::npos);
+  EXPECT_NE(backtrace.find("line 5, in bar"), std::string::npos);
+  EXPECT_NE(backtrace.find("line 7, in baz"), std::string::npos);
+  EXPECT_NE(backtrace.find("ZeroDivisionError"), std::string::npos);
 }
 No newline at end of file

lldb/unittests/ScriptInterpreter/Python/PythonExceptionStateTests.cpp

-//===-- PythonExceptionStateTest.cpp ------------------------------*- C++
-//-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "gtest/gtest.h"
-
-#include "Plugins/ScriptInterpreter/Python/PythonDataObjects.h"
-#include "Plugins/ScriptInterpreter/Python/PythonExceptionState.h"
-#include "Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h"
-#include "Plugins/ScriptInterpreter/Python/lldb-python.h"
-
-#include "PythonTestSuite.h"
-
-using namespace lldb_private;
-
-class PythonExceptionStateTest : public PythonTestSuite {
-public:
-protected:
-  void RaiseException() {
-    PyErr_SetString(PyExc_RuntimeError, "PythonExceptionStateTest test error");
-  }
-};
-
-TEST_F(PythonExceptionStateTest, TestExceptionStateChecking) {
-  PyErr_Clear();
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  RaiseException();
-  EXPECT_TRUE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-}
-
-TEST_F(PythonExceptionStateTest, TestAcquisitionSemantics) {
-  PyErr_Clear();
-  PythonExceptionState no_error(false);
-  EXPECT_FALSE(no_error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-  RaiseException();
-  PythonExceptionState error(false);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-  error.Discard();
-
-  PyErr_Clear();
-  RaiseException();
-  error.Acquire(false);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-}
-
-TEST_F(PythonExceptionStateTest, TestDiscardSemantics) {
-  PyErr_Clear();
-
-  // Test that discarding an exception does not restore the exception
-  // state even when auto-restore==true is set
-  RaiseException();
-  PythonExceptionState error(true);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  error.Discard();
-  EXPECT_FALSE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-}
-
-TEST_F(PythonExceptionStateTest, TestResetSemantics) {
-  PyErr_Clear();
-
-  // Resetting when auto-restore is true should restore.
-  RaiseException();
-  PythonExceptionState error(true);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-  error.Reset();
-  EXPECT_FALSE(error.IsError());
-  EXPECT_TRUE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-
-  // Resetting when auto-restore is false should discard.
-  RaiseException();
-  PythonExceptionState error2(false);
-  EXPECT_TRUE(error2.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-  error2.Reset();
-  EXPECT_FALSE(error2.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-}
-
-TEST_F(PythonExceptionStateTest, TestManualRestoreSemantics) {
-  PyErr_Clear();
-  RaiseException();
-  PythonExceptionState error(false);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  error.Restore();
-  EXPECT_FALSE(error.IsError());
-  EXPECT_TRUE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-}
-
-TEST_F(PythonExceptionStateTest, TestAutoRestoreSemantics) {
-  PyErr_Clear();
-  // Test that using the auto-restore flag correctly restores the exception
-  // state on destruction, and not using the auto-restore flag correctly
-  // does NOT restore the state on destruction.
-  {
-    RaiseException();
-    PythonExceptionState error(false);
-    EXPECT_TRUE(error.IsError());
-    EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-  }
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-  {
-    RaiseException();
-    PythonExceptionState error(true);
-    EXPECT_TRUE(error.IsError());
-    EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-  }
-  EXPECT_TRUE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-}
-
-TEST_F(PythonExceptionStateTest, TestAutoRestoreChanged) {
-  // Test that if we re-acquire with different auto-restore semantics,
-  // that the new semantics are respected.
-  PyErr_Clear();
-
-  RaiseException();
-  PythonExceptionState error(false);
-  EXPECT_TRUE(error.IsError());
-
-  error.Reset();
-  EXPECT_FALSE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  RaiseException();
-  error.Acquire(true);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  error.Reset();
-  EXPECT_FALSE(error.IsError());
-  EXPECT_TRUE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-}