Skip to content
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

RFC: py_call! macro #4414

Open
davidhewitt opened this issue Aug 2, 2024 · 5 comments · May be fixed by #4503
Open

RFC: py_call! macro #4414

davidhewitt opened this issue Aug 2, 2024 · 5 comments · May be fixed by #4503

Comments

@davidhewitt
Copy link
Member

davidhewitt commented Aug 2, 2024

At the moment, to call a Python function we have .call(args, kwargs), .call1(args) and .call0() methods.

I have two main concerns with these:

  • I think to a user the names are not immediately intuitive. That said, I can't suggest a combination I love much better.
  • These calls all use the tuple-and-dict calling convention. We now have the "vectorcall" calling convention which supports a list of arguments as a C array, (and optionally keyword argument names as a tuple), which is meaningfully more efficient.

I have been wondering for a while if we should have a py_call! macro which mirrors Python syntax and does its best to be efficient.

In all of these examples below, I assume obj to be Bound<'_, T>:

py_call!{ obj() }   // call obj without arguments, equivalent to current `obj.call0()`
py_call!{ obj(a, b) }  // call obj with positional arguments a and b. Use "vectorcall" convention for efficiency
py_call!{ obj(a, b, *args) } // call obj with positional arguments a, b, and the unpacked `*args` iterable.

py_call!{ obj(a, b, *args, x=1, y=2) } // same as above with keyword arguments x and y
py_call!{ obj(a, b, *args, x=1, y=2, **kwargs) } // and even with `kwargs` iterable

Some special cases might still use the tuple-and-dict convention:

py_call!{ obj(*args) }  // call obj with args
py_call!{ obj(**kwargs) }  // call obj with kwargs
py_call!{ obj(*args, **kwargs) }  // call obj with args and kwargs

I wonder if we would extend this to method calls:

py_call!{ obj.method(*args) }  // call obj.method with args
// etc

I suspect that this macro would prefer to be a proc-macro for better error messages etc. The implementation... would probably be a jumble of interesting traits.

I honestly have no idea how easy / hard the implementation would be, but I'm excited that this might create a nice user experience and also be a win for performance!

@davidhewitt
Copy link
Member Author

davidhewitt commented Aug 2, 2024

Worth bringing up an old concern by @mejrs in a related topic that * has a different meaning in Rust: #1463 (comment)

We do already have pyo3(signature = (*args, **kwargs)), so the extension to py_call!{ obj(*args, **kwargs) } might be acceptable, but it also might be true that this is problematic.

For example, could py_call!{ obj(*arg) } be intended to be a dereference of arg, but instead we try to unpack it? That is indeed a potential footgun. Presumably users could disambiguate with extra parentheses, e.g. py_call!{ obj((*arg)) }, but it's not pleasant :(

@birkenfeld
Copy link
Member

At least the keyword arguments part would be a huge improvement. And func(x=y) is not valid Rust syntax, so it cannot be misconstrued.

If *args and **kwds are allowed to be used the Python sense, I think no Rust syntax should be allowed at all - i.e. all parameter values need to be simple names or at most dotted.idents. (Oh and auto-referencing all of those as in the print! macro family should be considered, or are there instances in which it is required to pass Rust ownership to call?)

@davidhewitt
Copy link
Member Author

If *args and **kwds are allowed to be used the Python sense, I think no Rust syntax should be allowed at all - i.e. all parameter values need to be simple names or at most dotted.idents.

Being consistent with the print! family would be reasonable, although those macros do allow arbitrary name=expr syntax after the main format string. If we followed suit with them, we wouldn't have any

(Oh and auto-referencing all of those as in the print! macro family should be considered, or are there instances in which it is required to pass Rust ownership to call?)

If a user has a #[pyclass] MyClass which they want to create a new python object from as part of the call, then I think auto-referencing gets in the way. So I would assume following normal move semantics is better for now, though that does imply we want users to be able to write py_call!{ obj(&x, &y) }.

@mejrs
Copy link
Member

mejrs commented Aug 3, 2024

I'm generally opposed to macros because they're harder to read and reason about, they tend to not play well with IDE's and the syntax can be surprising/unintuitive. If this can be a function, it should be a function. I haven't looked too much into this, but can we not use the vectorcall apis with call and call1?

FWIW I don't have a problem with the current situation as call, call1 and call0. Yes the names are kind of silly, but they're fairly obvious, not surprising and the docs have decent examples.

@ChayimFriedman2
Copy link
Contributor

ChayimFriedman2 commented Aug 18, 2024

These calls all use the tuple-and-dict calling convention. We now have the "vectorcall" calling convention which supports a list of arguments as a C array, (and optionally keyword argument names as a tuple), which is meaningfully more efficient.

Are you claiming that this is impossible to implement with the current syntax (i.e. no macros)? I believe we should separate discussing potential other syntaxes and supporting vectorcall. vectorcall can be supported with a function as well, and it can even be supported by the existing functions (on nightly only AFAIK, unfortunately edit: actually, no, I believe it' possible on stable!).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants