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:
> lawrence_danna wrote:
> > 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.
> lol :)
>
> I would actually say that we should try not to use this function(ality)
> wherever possible. Making decisions based on the number of arguments the
> thing you're about to call takes sounds weird. I don't want to get too
> involved in this, but I was designing this, I'd just say that if one tries to
> pass arguments to the callback then the callback MUST take three arguments
> (or we'll abort processing the breakpoint command). If he wants his function
> to be called both with arguments and without, he can just add a default value
> to the third argument. (And if his function takes two arguments, but he still
> tries to pass something... well, it's his own fault).
>
> Anyway, feel free to ignore this comment, but I felt like I had to say
> something. :)
I completely agree with Pavel. Inspecting a function signature before
calling it is a big code smell in python. If there's a way to avoid doing
that introspection, that would be better.
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68671/new/
https://reviews.llvm.org/D68671
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
