clean up the implementation of PythonCallable::GetNumArguments

Summary:
The current implementation of PythonCallable::GetNumArguments
is not exception safe, has weird semantics, and is just plain
incorrect for some kinds of functions.

Python 3.3 introduces inspect.signature, which lets us easily
query for function signatures in a sane and documented way.

This patch leaves the old implementation in place for < 3.3,
but uses inspect.signature for modern pythons.   It also leaves
the old weird semantics in place, but with FIXMEs grousing about
it.   We should update the callers and fix the semantics in a
subsequent patch.    It also adds some tests.

Reviewers: JDevlieghere, clayborg, labath, jingham

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

llvm-svn: 375181

1 files changed, 53 insertions, 15 deletions

diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
index 255fa06c057..7f460234ffb 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -309,20 +309,37 @@ protected:
   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 {
     const char format[] = {'(', PythonFormat<T>::format..., ')', 0};
-#if PY_MAJOR_VERSION < 3
-    PyObject *obj = PyObject_CallMethod(m_py_obj, const_cast<char *>(name),
-                                        const_cast<char *>(format),
-                                        PythonFormat<T>::get(t)...);
-#else
     PyObject *obj =
-        PyObject_CallMethod(m_py_obj, name, format, PythonFormat<T>::get(t)...);
-#endif
+        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 {
+    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);
@@ -386,6 +403,9 @@ template <> llvm::Expected<bool> As<bool>(llvm::Expected<PythonObject> &&obj);
 template <>
 llvm::Expected<long long> As<long long>(llvm::Expected<PythonObject> &&obj);
 
+template <>
+llvm::Expected<std::string> As<std::string>(llvm::Expected<PythonObject> &&obj);
+
 } // namespace python
 
 template <class T> class TypedPythonObject : public PythonObject {
@@ -559,8 +579,14 @@ public:
 
   PythonList GetKeys() const;
 
-  PythonObject GetItemForKey(const PythonObject &key) const;
-  void SetItemForKey(const PythonObject &key, const PythonObject &value);
+  PythonObject GetItemForKey(const PythonObject &key) const; // DEPRECATED
+  void SetItemForKey(const PythonObject &key,
+                     const PythonObject &value); // DEPRECATED
+
+  llvm::Expected<PythonObject> GetItem(const PythonObject &key) const;
+  llvm::Expected<PythonObject> GetItem(const char *key) const;
+  llvm::Error SetItem(const PythonObject &key, const PythonObject &value) const;
+  llvm::Error SetItem(const char *key, const PythonObject &value) const;
 
   StructuredData::DictionarySP CreateStructuredDictionary() const;
 };
@@ -600,19 +626,31 @@ public:
   using TypedPythonObject::TypedPythonObject;
 
   struct ArgInfo {
-    size_t count;
-    bool is_bound_method : 1;
-    bool has_varargs : 1;
-    bool has_kwargs : 1;
+    /* the number of positional arguments, including optional ones,
+     * and excluding varargs.  If this is a bound method, then the
+     * count will still include a +1 for self.
+     *
+     * FIXME. That's crazy.  This should be replaced with
+     * an accurate min and max for positional args.
+     */
+    int count;
+    /* does the callable have positional varargs? */
+    bool has_varargs : 1; // FIXME delete this
+    /* is the callable a bound method written in python? */
+    bool is_bound_method : 1; // FIXME delete this
   };
 
   static bool Check(PyObject *py_obj);
 
-  ArgInfo GetNumArguments() const;
+  llvm::Expected<ArgInfo> GetArgInfo() const;
+
+  llvm::Expected<ArgInfo> GetInitArgInfo() const;
+
+  ArgInfo GetNumArguments() const; // DEPRECATED
 
   // If the callable is a Py_Class, then find the number of arguments
   // of the __init__ method.
-  ArgInfo GetNumInitArguments() const;
+  ArgInfo GetNumInitArguments() const; // DEPRECATED
 
   PythonObject operator()();