lawrence_danna updated this revision to Diff 225303.
lawrence_danna added a comment.

rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69014

Files:
  
lldb/packages/Python/lldbsuite/test/commands/command/script/TestCommandScript.py
  lldb/packages/Python/lldbsuite/test/commands/command/script/callables.py
  lldb/packages/Python/lldbsuite/test/commands/command/script/py_import
  lldb/scripts/Python/python-wrapper.swig
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
===================================================================
--- lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -643,8 +643,8 @@
     auto arginfo = lambda.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);
-    EXPECT_EQ(arginfo.get().is_bound_method, false);
   }
 
   {
@@ -655,8 +655,8 @@
     auto arginfo = lambda.GetArgInfo();
     ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
     EXPECT_EQ(arginfo.get().count, 2);
+    EXPECT_EQ(arginfo.get().max_positional_args, 2u);
     EXPECT_EQ(arginfo.get().has_varargs, false);
-    EXPECT_EQ(arginfo.get().is_bound_method, false);
   }
 
   {
@@ -667,15 +667,27 @@
     auto arginfo = lambda.GetArgInfo();
     ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
     EXPECT_EQ(arginfo.get().count, 2);
+    EXPECT_EQ(arginfo.get().max_positional_args,
+              PythonCallable::ArgInfo::UNBOUNDED);
     EXPECT_EQ(arginfo.get().has_varargs, true);
-    EXPECT_EQ(arginfo.get().is_bound_method, false);
   }
 
   {
     const char *script = "class Foo: \n"
                          "  def bar(self, x):\n"
                          "     return x \n"
+                         "  @classmethod \n"
+                         "  def classbar(cls, x):\n"
+                         "     return x \n"
+                         "  @staticmethod \n"
+                         "  def staticbar(x):\n"
+                         "     return x \n"
+                         "  def __call__(self, x): \n"
+                         "     return x \n"
+                         "obj = Foo() \n"
                          "bar_bound   = Foo().bar \n"
+                         "bar_class   = Foo().classbar \n"
+                         "bar_static  = Foo().staticbar \n"
                          "bar_unbound = Foo.bar \n";
     PyObject *o =
         PyRun_String(script, Py_file_input, globals.get(), globals.get());
@@ -687,16 +699,37 @@
     auto arginfo = bar_bound.get().GetArgInfo();
     ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
     EXPECT_EQ(arginfo.get().count, 2); // FIXME, wrong
+    EXPECT_EQ(arginfo.get().max_positional_args, 1u);
     EXPECT_EQ(arginfo.get().has_varargs, false);
-    EXPECT_EQ(arginfo.get().is_bound_method, true);
 
     auto bar_unbound = As<PythonCallable>(globals.GetItem("bar_unbound"));
     ASSERT_THAT_EXPECTED(bar_unbound, llvm::Succeeded());
     arginfo = bar_unbound.get().GetArgInfo();
     ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
     EXPECT_EQ(arginfo.get().count, 2);
+    EXPECT_EQ(arginfo.get().max_positional_args, 2u);
+    EXPECT_EQ(arginfo.get().has_varargs, false);
+
+    auto bar_class = As<PythonCallable>(globals.GetItem("bar_class"));
+    ASSERT_THAT_EXPECTED(bar_class, llvm::Succeeded());
+    arginfo = bar_class.get().GetArgInfo();
+    ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+    EXPECT_EQ(arginfo.get().max_positional_args, 1u);
+    EXPECT_EQ(arginfo.get().has_varargs, false);
+
+    auto bar_static = As<PythonCallable>(globals.GetItem("bar_static"));
+    ASSERT_THAT_EXPECTED(bar_static, llvm::Succeeded());
+    arginfo = bar_static.get().GetArgInfo();
+    ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+    EXPECT_EQ(arginfo.get().max_positional_args, 1u);
+    EXPECT_EQ(arginfo.get().has_varargs, false);
+
+    auto obj = As<PythonCallable>(globals.GetItem("obj"));
+    ASSERT_THAT_EXPECTED(obj, llvm::Succeeded());
+    arginfo = obj.get().GetArgInfo();
+    ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+    EXPECT_EQ(arginfo.get().max_positional_args, 1u);
     EXPECT_EQ(arginfo.get().has_varargs, false);
-    EXPECT_EQ(arginfo.get().is_bound_method, false);
   }
 
 #if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
@@ -710,9 +743,9 @@
     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);
-    EXPECT_EQ(arginfo.get().is_bound_method, false);
   }
 
 #endif
 }
\ No newline at end of file
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -624,6 +624,11 @@
   using TypedPythonObject::TypedPythonObject;
 
   struct ArgInfo {
+    /* the largest number of positional arguments this callable
+     * can accept, or UNBOUNDED, ie UINT_MAX if it's a varargs
+     * function and can accept an arbitrary number */
+    unsigned max_positional_args;
+    static constexpr unsigned UNBOUNDED = UINT_MAX; // FIXME c++17 inline
     /* 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.
@@ -634,8 +639,6 @@
     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);
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -876,21 +876,23 @@
   result.count = cantFail(As<long long>(pyarginfo.get().GetAttribute("count")));
   result.has_varargs =
       cantFail(As<bool>(pyarginfo.get().GetAttribute("has_varargs")));
-  result.is_bound_method =
+  bool is_method =
       cantFail(As<bool>(pyarginfo.get().GetAttribute("is_bound_method")));
+  result.max_positional_args =
+      result.has_varargs ? ArgInfo::UNBOUNDED : result.count;
 
   // FIXME emulate old broken behavior
-  if (result.is_bound_method)
+  if (is_method)
     result.count++;
 
 #else
-
+  bool is_bound_method = false;
   PyObject *py_func_obj = m_py_obj;
   if (PyMethod_Check(py_func_obj)) {
     py_func_obj = PyMethod_GET_FUNCTION(py_func_obj);
     PythonObject im_self = GetAttributeValue("im_self");
     if (im_self.IsValid() && !im_self.IsNone())
-      result.is_bound_method = true;
+      is_bound_method = true;
   } else {
     // see if this is a callable object with an __call__ method
     if (!PyFunction_Check(py_func_obj)) {
@@ -899,9 +901,9 @@
         auto __callable__ = __call__.AsType<PythonCallable>();
         if (__callable__.IsValid()) {
           py_func_obj = PyMethod_GET_FUNCTION(__callable__.get());
-          PythonObject im_self = GetAttributeValue("im_self");
+          PythonObject im_self = __callable__.GetAttributeValue("im_self");
           if (im_self.IsValid() && !im_self.IsNone())
-            result.is_bound_method = true;
+            is_bound_method = true;
         }
       }
     }
@@ -916,12 +918,18 @@
 
   result.count = code->co_argcount;
   result.has_varargs = !!(code->co_flags & CO_VARARGS);
+  result.max_positional_args = result.has_varargs
+                                   ? ArgInfo::UNBOUNDED
+                                   : (result.count - (int)is_bound_method);
 
 #endif
 
   return result;
 }
 
+constexpr unsigned
+    PythonCallable::ArgInfo::UNBOUNDED; // FIXME delete after c++17
+
 PythonCallable::ArgInfo PythonCallable::GetNumArguments() const {
   auto arginfo = GetArgInfo();
   if (!arginfo) {
Index: lldb/scripts/Python/python-wrapper.swig
===================================================================
--- lldb/scripts/Python/python-wrapper.swig
+++ lldb/scripts/Python/python-wrapper.swig
@@ -696,15 +696,19 @@
 
     // pass the pointer-to cmd_retobj_sb or watch the underlying object disappear from under you
     // see comment above for SBCommandReturnObjectReleaser for further details
-    auto argc = pfunc.GetNumArguments();
+    auto argc = pfunc.GetArgInfo();
+    if (!argc) {
+        llvm::consumeError(argc.takeError());
+        return false;
+    }
     PythonObject debugger_arg(PyRefType::Owned, SBTypeToSWIGWrapper(debugger_sb));
     PythonObject exe_ctx_arg(PyRefType::Owned, SBTypeToSWIGWrapper(exe_ctx_sb));
     PythonObject cmd_retobj_arg(PyRefType::Owned, SBTypeToSWIGWrapper(&cmd_retobj_sb));
 
-    if (argc.count == 5 || argc.is_bound_method || argc.has_varargs)
-        pfunc(debugger_arg, PythonString(args), exe_ctx_arg, cmd_retobj_arg, dict);
-    else
+    if (argc.get().max_positional_args < 5u)
         pfunc(debugger_arg, PythonString(args), cmd_retobj_arg, dict);
+    else
+        pfunc(debugger_arg, PythonString(args), exe_ctx_arg, cmd_retobj_arg, dict);
 
     return true;
 }
Index: lldb/packages/Python/lldbsuite/test/commands/command/script/py_import
===================================================================
--- lldb/packages/Python/lldbsuite/test/commands/command/script/py_import
+++ lldb/packages/Python/lldbsuite/test/commands/command/script/py_import
@@ -11,3 +11,22 @@
 command script add tell_curr --function welcome.check_for_synchro --synchronicity curr
 command script add takes_exe_ctx --function welcome.takes_exe_ctx
 command script import decorated.py
+
+
+command script import callables.py
+
+command script add -f callables.foobar foobar
+command script add -f callables.foobar4 foobar4
+command script add -f callables.vfoobar vfoobar
+command script add -f callables.v5foobar v5foobar
+
+command script add -f callables.FooBar.sfoobar sfoobar
+command script add -f callables.FooBar.cfoobar cfoobar
+command script add -f callables.FooBarObj.ifoobar ifoobar
+
+command script add -f callables.FooBar.sfoobar4 sfoobar4
+command script add -f callables.FooBar.cfoobar4 cfoobar4
+command script add -f callables.FooBarObj.ifoobar4 ifoobar4
+
+command script add -f callables.FooBarObj ofoobar
+command script add -f callables.FooBar4Obj ofoobar4
Index: lldb/packages/Python/lldbsuite/test/commands/command/script/callables.py
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/commands/command/script/callables.py
@@ -0,0 +1,63 @@
+
+from __future__ import print_function
+
+import lldb
+
+# bunch of different kinds of python callables that should
+# all work as commands.
+
+def check(debugger, command, context, result, internal_dict):
+    if (not isinstance(debugger, lldb.SBDebugger) or
+        not isinstance(command, str) or
+        not isinstance(result, lldb.SBCommandReturnObject) or
+        not isinstance(internal_dict, dict) or
+        (not context is None and
+        not isinstance(context, lldb.SBExecutionContext))):
+      raise Exception()
+    result.AppendMessage("All good.")
+
+def vfoobar(*args):
+    check(*args)
+
+def v5foobar(debugger, command, context, result, internal_dict, *args):
+    check(debugger, command, context, result, internal_dict)
+
+def foobar(debugger, command, context, result, internal_dict):
+    check(debugger, command, context, result, internal_dict)
+
+def foobar4(debugger, command, result, internal_dict):
+    check(debugger, command, None, result, internal_dict)
+
+class FooBar:
+    @staticmethod
+    def sfoobar(debugger, command, context, result, internal_dict):
+      check(debugger, command, context, result, internal_dict)
+
+    @classmethod
+    def cfoobar(cls, debugger, command, context, result, internal_dict):
+      check(debugger, command, context, result, internal_dict)
+
+    def ifoobar(self, debugger, command, context, result, internal_dict):
+      check(debugger, command, context, result, internal_dict)
+
+    def __call__(self, debugger, command, context, result, internal_dict):
+      check(debugger, command, context, result, internal_dict)
+
+    @staticmethod
+    def sfoobar4(debugger, command, result, internal_dict):
+      check(debugger, command, None, result, internal_dict)
+
+    @classmethod
+    def cfoobar4(cls, debugger, command, result, internal_dict):
+      check(debugger, command, None, result, internal_dict)
+
+    def ifoobar4(self, debugger, command, result, internal_dict):
+      check(debugger, command, None, result, internal_dict)
+
+class FooBar4:
+    def __call__(self, debugger, command, result, internal_dict):
+      check(debugger, command, None, result, internal_dict)
+
+FooBarObj = FooBar()
+
+FooBar4Obj = FooBar4()
\ No newline at end of file
Index: lldb/packages/Python/lldbsuite/test/commands/command/script/TestCommandScript.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/commands/command/script/TestCommandScript.py
+++ lldb/packages/Python/lldbsuite/test/commands/command/script/TestCommandScript.py
@@ -4,7 +4,7 @@
 
 from __future__ import print_function
 
-
+import sys
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -22,6 +22,21 @@
     def pycmd_tests(self):
         self.runCmd("command source py_import")
 
+        # Test a bunch of different kinds of python callables with
+        # both 4 and 5 positional arguments.
+        self.expect("foobar", substrs=["All good"])
+        self.expect("foobar4", substrs=["All good"])
+        self.expect("vfoobar", substrs=["All good"])
+        self.expect("v5foobar", substrs=["All good"])
+        self.expect("sfoobar", substrs=["All good"])
+        self.expect("cfoobar", substrs=["All good"])
+        self.expect("ifoobar", substrs=["All good"])
+        self.expect("sfoobar4", substrs=["All good"])
+        self.expect("cfoobar4", substrs=["All good"])
+        self.expect("ifoobar4", substrs=["All good"])
+        self.expect("ofoobar", substrs=["All good"])
+        self.expect("ofoobar4", substrs=["All good"])
+
         # Verify command that specifies eCommandRequiresTarget returns failure
         # without a target.
         self.expect('targetname',
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to