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
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
