-
-
Notifications
You must be signed in to change notification settings - Fork 3k
[mypyc] Generate introspection signatures for compiled functions #19307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is a very useful feature! I have one question about default values, but solving this is not necessary to merge this PR.
for op in block.ops: | ||
if isinstance(op, Assign) and op.dest.name == name: | ||
return _extract_python_literal(op.src) | ||
return _NOT_REPRESENTABLE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any ideas about how to generalize this to support more kinds of default values? I guess we could special case empty lists and dictionaries, which are somewhat common (even if you are not supposed to use them as defaults), and perhaps named constants and enum values. What about using a placeholder default value for unsupported default values, such as ...
(ellipsis)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other defaults are loaded with LoadStatic
ops, so we'd need to look at the module's __top_level__
IR to extract the values. That isn't too tricky to do (in fact I already have a rough patch that does that), but it could be expensive performance wise, since __top_level__
can be quite big and we'd be doing potentially many linear searches. Alternatively, we could try recording information about default arguments earlier on during IR building, though I haven't figured out a simple way to do that yet
What about using a placeholder default value for unsupported default values, such as
...
(ellipsis)?
I actually tried that in a previous revision :-) I was worried that it might conflict with using an actual Ellipsis default argument at runtime. I'm also not sure what effect using a placeholder might have on the tools that consume these signatures. For example, I think stubtest might complain if a runtime signature had ...
as a default argument when the parameter isn't typed to actually accept EllipsisType
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recording information about default argument values during IR building seems like the most promising approach to me, though I'm not sure how easy this would be to implement. They could perhaps be stored in FuncDecl
or FuncSignature
. I agree with you that analyzing __top_level__
seems too inefficient (and also inelegant).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine as an initial implementation. We can figure out more complex cases later.
@brianschubert Can you create a follow-up mypyc issue about the remaining cases, with some discussion based on the comments in this PR? |
Refs mypyc/mypyc#838
This PR populates
__text_signature__
for compiled functions, making runtime signature introspection possible (i.e.inspect.signature(func)
).While
__text_signature__
is an undocumented CPython implementation detail, other extension module generators are using it in practice. For example, PyO3 and Cython both support it. I think it would be reasonable for mypyc to support it too.Some function signatures can't be represented by
__text_signature__
(namely, those with complex default arguments). In those cases, no signatures are generated (same as the current behavior).