Add zend_function wrapper, try_call_method zval/object methods #264
Conversation
danog
changed the title
|
Will submit php-tokio integration in an upcoming PR! |
| pub fn from_function(name: &str) -> Self { | ||
| match Self::try_from_function(name) { | ||
| Some(v) => v, | ||
| None => panic!("Expected function `{}` to exist!", name), |
There was a problem hiding this comment.
Perhaps more of a architectural question, but is it better to just not offer methods that can panic? It seems to be correct, the user should always use try_from_* and call unwrap() if they're so sure it can't fail
There was a problem hiding this comment.
In my opinion, it makes sense to offer infallible variants of these two functions because there's a valid usecase for fetching native functions and native methods, which are always available, and forcing the use of try=>unwrap for known existing native functions seemed a bit unergonomic to me.
What do you think?
There was a problem hiding this comment.
Hmm yes I can see that, but is it always true that native methods / functions are always available? What about across different versions of PHP (as the caller, I don't want to worry about knowing that), or what about the use of functions in the INI disabled_functions.
I understand wanting to be ergonomic, but I value the trust in knowing I'm not going to get runtime errors to be higher. A rust panic in a PHP extension is also something I would never want to happen especially!
There was a problem hiding this comment.
Hmmmm, understandable, I guess I can just directly export the function entries of common functions, fetching them on startup, instead.
Reverting that part for now then :)
I would also like to propose myself as maintainer for this repo.
I'm currently building an async MongoDB driver @nicelocal using this library and php-tokio.
I've forked ext-php-rs due to inactivity on this repo and the need to make a lot of changes to the project, but I would gladly switch back to this repo.
I've split the async functionality into php-tokio due to plans to make a standalone extension just for the event loop (that other extensions would hook into), and it requires the nicelocal fork due to required changes, if this PR is merged I can switch php-tokio back to this repo and submit another PR to integrate it into ext-php-rs.