Conversation
arnaud-lb
force-pushed
the
partials-v2-gen
branch
3 times, most recently
from
7062e66 to
f65bf45
Compare
arnaud-lb
force-pushed
the
partials-v2-gen
branch
2 times, most recently
from
d820a4f to
75dbd22
Compare
There was a problem hiding this comment.
Can we get a test case where the function has optional arguments and an incorrect number is given (just to see the error message once).
There was a problem hiding this comment.
This test shows this error message: https://github.com/arnaud-lb/php-src/blob/75dbd22353d81d34d63587f9c703523f5d10b82e/Zend/tests/partial_application/rfc_examples_003.phpt
Do you prefer a dedicated test case?
There was a problem hiding this comment.
Great, I just realize that the error messages for userland and native functions are already inconsistent with each other:
arnaud-lb
force-pushed
the
partials-v2-gen
branch
2 times, most recently
from
bf4b4f6 to
b753b0c
Compare
Zend/tests/partial_application/superfluous_args_are_forwarded.phpt
Outdated
Show resolved
Hide resolved
Zend/tests/partial_application/superfluous_args_are_forwarded.phpt
Outdated
Show resolved
Hide resolved
| ZEND_API bool zend_check_type_ex( | ||
| const zend_type *type, zval *arg, zend_class_entry *scope, | ||
| bool is_return_type, bool is_internal) | ||
| { | ||
| return zend_check_type(type, arg, scope, is_return_type, is_internal); | ||
| } |
There was a problem hiding this comment.
What's the purpose of this? Wouldn't it work to expose zend_check_type() directly?
There was a problem hiding this comment.
It's not that easy, as the function is always_inline, and uses always_inline functions from this file as well. zend_check_type() is critical, I didn't want to take the risk of performance regression.
So we have the following options:
- Move the function and a bunch of functions it uses to a header file (zend_check_type, zend_check_type_slow, zend_check_intersection_type_from_list, probably others), so we don't lose inline-ability
- Make the function extern inline, but we don't do that (there was a thread about that in a PR).
- Export a wrapper, like I did here
I will think about about the other options
arnaud-lb
force-pushed
the
partials-v2-gen
branch
from
5b8cfc2 to
65a21f2
Compare
arnaud-lb
force-pushed
the
partials-v2-gen
branch
from
e1a4016 to
1616f33
Compare
arnaud-lb
force-pushed
the
partials-v2-gen
branch
2 times, most recently
from
0b7a4a5 to
2cf2ae1
Compare
arnaud-lb
force-pushed
the
partials-v2-gen
branch
4 times, most recently
from
a3a4549 to
8514af4
Compare
… on resolution failures with a new optional argument. close phpGH-20534
* ext/sqlite3: Sqlite3::openBlob() code path simplification. since the stream is opened in non persistent mode, the failure code path is dead (so are the missing leaks fixes).
arnaud-lb
force-pushed
the
partials-v2-gen
branch
from
88ff9da to
5b844c6
Compare
20 participants
PFAs are created by generating the AST of a Closure at runtime and compiling it. The compiled op_array can be cached similarly to linked classes.
About caching
There are three levels of caching:
ZCG(hash))EG(partial_function_application_cache)The cache key in
ZCG(hash)is composed of the address of the declaring opline, the address of the function being called, and a prefix.EG(partial_function_application_cache)is used when opcache is disabled, or when the PFA can not be cached in SHM (e.g. because the actual function itself is not cached). This is mostly useful in CLI (opcache is disabled by default) for polymorphic PFAs (inline cache less useful).TODO