[Lldb-commits] [PATCH] D68671: Add the ability to pass extra args to a Python breakpoint command function

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna marked an inline comment as done.
lawrence_danna added inline comments.


================
Comment at: lldb/include/lldb/Interpreter/ScriptInterpreter.h:469
+  
+  virtual int GetNumArgumentsForCallable(const char *callable_name) { 
+    return -1;
----------------
labath wrote:
> In light of varargs functions (`*args, **kwargs`), which are fairly popular in python, the concept of "number of arguments of a callable" does not seem that well defined. The current implementation seems to return the number of fixed arguments, which might be fine, but I think this behavior should be documented. Also, it would be good to modernize this function signature -- have it take a StringRef, and return a `Expected<unsigned (?)>` -- ongoing work by @lawrence_danna will make it possible to return errors from the python interpreter, and this will make it possible to display those, instead of just guessing that this is because the callable was not found (it could in fact be because the named thing is not a callable, of because resolving the name produced an exception, ...).
I just took a look at PythonCallable::GetNumArguments() and it's horribly broken.  

It doesn't even work for the simplest test case I could think of.

```  
auto builtins = PythonModule::Import("builtins");
ASSERT_THAT_EXPECTED(builtins, llvm::Succeeded());
auto hex = As<PythonCallable>(builtins.get().GetAttribute("hex"));
ASSERT_THAT_EXPECTED(hex, llvm::Succeeded());  
EXPECT_EQ(hex.get().GetNumArguments().count, 1u);
```

we should really re-write it to use inspect.signature.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68671