[Lldb-commits] [PATCH] D68995: clean up the implementation of PythonCallable::GetNumArguments

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

In D68995#1712396 <https://reviews.llvm.org/D68995#1712396>, @lawrence_danna wrote:

> >> Looks like the old implementation also doesn't work for class or static methods, or for objects with `__call__` method.    I added a bunch of tests for script commands with various callable types in the followon patch.
> > 
> > Ok, I see... Would it be hard/impossible to fix that using the previous method, so that we have a unified implementation? Or would that result in a bunch of ifdefs too ? I'm sorry if that's obvious, but I don't know much about the C python api..
>
> I did fix the previous method for python2, at least for the callable types I could think of to test.
>
> There must be some way to get something resembling the old method to work for python3, because `inspect` does it.   But it would be harder to get right, and a lot less likely to keep being correct in future versions of python.    And I don't think it's possible to have a single implementation like that that works for both 2 and 3 without ifdefs.


Right, that's what I was thinking/fearing. I think I am satisfied with that explanation. Thanks for explaining.

> I was thinking that since python2 is EOL in like two months anyway, the best thing to do is to just keep the old implementation around until we drop python 2 support, and then delete it.

Yeah, though I don't know if that means that lldb will drop support for it straight away. I haven't heard of any plans like that, though the next weeks dev meeting will probably be a good place to talk about it. Will you be there by any chance?



================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:822
+#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
+static const char *get_arg_info_script = R"(
+from inspect import signature, Parameter, ismethod
----------------
`static const char get_arg_info_script[]` (one pointer more efficient)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68995