Re: [PHP-DEV] [RFC] Allow calls to global functions in constant expressions

2020-02-09 Thread Mike Schinkel
> On Feb 9, 2020, at 8:34 PM, tyson andre  wrote:
> 
>> 1. Why again are MyClass::methodName() not considered for the non-whitelist 
>> vote? 
>> 
>> Seems to me a developer would be more inclined to implement the expressions 
>> that define the class constant's value in a method of the class than in an 
>> external function.
> 
> My reasons for doing this were to keep the scope small and easy to implement,
> and work on an RFC for methods immediately after if the vote for allowing 
> arbitrary functions passed.
> 
> The other concern was the edge case `static::method_name()` in class 
> constants,
> but `call_user_func(['static', 'method_name'])` or 
> `array_map('static::method_name', VALS)`
> causes the same issues.
> 
> This leads me to a blocker I've discovered with this implementation calling 
> without a whitelist:
> The way PHP resolves the scope is to look for the closest closure or 
> user-defined function,
> then to extract the class of that function. (in zend_get_executed_scope).
> I thought the scope would work because `const X = self::Y` works,
> but it turns out ZEND_AST_CLASS_CONST is special cased in AST evaluation.
> My current ideas for fixing this (for class constants and property defaults):
> (Parameter defaults, static variables, and global constants should already 
> have reasonable scopes)
> 
> 1. Wrap all of the calls the engine is making in a byte copy of 
> call_user_func, but with the property `zend_class_entry *scope` set, so that 
> zend_get_executed_scope will see that frame's scope and use it instead of the 
> caller's scope.
> 2. Add a fake stack frame that acts as if an extra closure (bound to the 
> declaring class's scope) was being evaluated.
> 3. Add a brand new function.
> 
> If I don't do that, then constant/property evaluation uses the scope of the 
> caller, not the class
> 
> ```
> class MyClass {
>public static function example() {}
>const X = call_user_func('self::example');
> }
> // this throws in the current implementation
> // due to no active class scope, and that's a bug
> echo MyClass::X; 
> ```
> 
> If I (or anyone else) can't figure out how to implement the fix, or the fix 
> has issues,
> I might remove the secondary vote from the RFC and just vote on the whitelist,
> which excluded functions accepting callables.
> If anyone comes up with a working implementation for a fix (including 
> exception handling),
> I'd like to know.
> 

Thanks for the reply.

>> 2. Do we really want to add a standard library function 53 characters long? 
>> 
>> Can we not come up with a more concise name than 
>> get_defined_functions_allowed_in_constant_expressions(), like maybe 
>> get_const_expr_funcs() or get_const_expressions()?
> 
> I was planning to change it if I thought of a better name.
> For get_const_expr_funcs(), I'd think it would be important to be consistent 
> about
> naming in the standard library.
> `get_const_expr_functions()` would work, though.

Oh, okay.  But don't you think the 'consistent naming in PHP' ship sailed eons 
ago?!?

(Sorry, I just could not resist!  :-)

-Mike

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Allow calls to global functions in constant expressions

2020-02-09 Thread tyson andre
> 1. Why again are MyClass::methodName() not considered for the non-whitelist 
> vote? 
> 
> Seems to me a developer would be more inclined to implement the expressions 
> that define the class constant's value in a method of the class than in an 
> external function.

My reasons for doing this were to keep the scope small and easy to implement,
and work on an RFC for methods immediately after if the vote for allowing 
arbitrary functions passed.

The other concern was the edge case `static::method_name()` in class constants,
but `call_user_func(['static', 'method_name'])` or 
`array_map('static::method_name', VALS)`
causes the same issues.

This leads me to a blocker I've discovered with this implementation calling 
without a whitelist:
The way PHP resolves the scope is to look for the closest closure or 
user-defined function,
then to extract the class of that function. (in zend_get_executed_scope).
I thought the scope would work because `const X = self::Y` works,
but it turns out ZEND_AST_CLASS_CONST is special cased in AST evaluation.
My current ideas for fixing this (for class constants and property defaults):
(Parameter defaults, static variables, and global constants should already have 
reasonable scopes)

1. Wrap all of the calls the engine is making in a byte copy of call_user_func, 
but with the property `zend_class_entry *scope` set, so that 
zend_get_executed_scope will see that frame's scope and use it instead of the 
caller's scope.
2. Add a fake stack frame that acts as if an extra closure (bound to the 
declaring class's scope) was being evaluated.
3. Add a brand new function.

If I don't do that, then constant/property evaluation uses the scope of the 
caller, not the class

```
class MyClass {
public static function example() {}
const X = call_user_func('self::example');
}
// this throws in the current implementation
// due to no active class scope, and that's a bug
echo MyClass::X; 
```

If I (or anyone else) can't figure out how to implement the fix, or the fix has 
issues,
I might remove the secondary vote from the RFC and just vote on the whitelist,
which excluded functions accepting callables.
If anyone comes up with a working implementation for a fix (including exception 
handling),
I'd like to know.


> 2. Do we really want to add a standard library function 53 characters long? 
> 
> Can we not come up with a more concise name than 
> get_defined_functions_allowed_in_constant_expressions(), like maybe 
> get_const_expr_funcs() or get_const_expressions()?

I was planning to change it if I thought of a better name.
For get_const_expr_funcs(), I'd think it would be important to be consistent 
about
naming in the standard library.
`get_const_expr_functions()` would work, though.
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] Allow calls to global functions in constant expressions

2020-02-09 Thread Mike Schinkel
> On Feb 9, 2020, at 4:02 PM, tyson andre  wrote:
> 
> https://wiki.php.net/rfc/calls_in_constant_expressions has been updated and 
> moved to
> "Under Discussion".
> 
> This proposes allowing function calls in constant declarations, static 
> property defaults,
> static variables, and parameter defaults.
> It includes a secondary voting option to allow any function calls,
> and not just a small set of whitelisted core functions.
> Other expression types would continue to be forbidden in constant expressions 
> (variables, method calls, etc)
> 
> Let me know if you have any comments on the RFC or implementation.

1. Why again are MyClass::methodName() not considered for the non-whitelist 
vote?  

Seems to me a developer would be more inclined to implement the expressions 
that define the class constant's value in a method of the class than in an 
external function.

2. Do we really want to add a standard library function 53 characters long?  

Can we not come up with a more concise name than 
get_defined_functions_allowed_in_constant_expressions(), like maybe 
get_const_expr_funcs() or get_const_expressions()?

-Mike
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



[PHP-DEV] [RFC] Allow calls to global functions in constant expressions

2020-02-09 Thread tyson andre
Hi internals,

https://wiki.php.net/rfc/calls_in_constant_expressions has been updated and 
moved to
"Under Discussion".

This proposes allowing function calls in constant declarations, static property 
defaults,
static variables, and parameter defaults.
It includes a secondary voting option to allow any function calls,
and not just a small set of whitelisted core functions.
Other expression types would continue to be forbidden in constant expressions 
(variables, method calls, etc)

Let me know if you have any comments on the RFC or implementation.

Earlier discussion can be found in https://externals.io/message/108343
("Planning an RFC to allow calls to global functions in constant expressions")

I made some updates to the RFC, implementation and the proposed whitelist of 
functions.
(in response to https://externals.io/message/108343#108427)

> > As you've already realized, the main problem here is the behavior for
> > functions that have side-effects or state. Currently we mostly get away
> > with the illusion that it doesn't matter when exactly constexpr
> > initializers are evaluated. Apart from the error location and (admittedly
> > quite a few) other details, you ostensibly can't distinguish whether the
> > expression is going to evaluated on declaration, on first access or on each
> > access.
>
> Good point - If PHP ends up supporting an option where **any** global 
> function can get called,
> It'd make sense to evaluate any expression *with a function call*
> every time for instance properties and parameter defaults
> (e.g. `public $id = generate_unique_int_id()`).
> (the optimizer can later optimize any functions where it won't be noticeable).
>
> I think that would require updates to my PR for caching,
> since immutable returned zvals (including false/true) get cached permanently.

I've completed the updates to parameter defaults.
Parameter defaults containing calls now don't get cached:
They always get evaluated every time the parameter isn't passed in.

I also excluded support for calls in instance property defaults from the RFC.
I'd want that feature, but the changes to PHP's internal representation of 
classes expand
the scope of this RFC too much for me.

- PHP currently evaluates all the property defaults and permanently caches them
  the first time a property gets accessed or an object gets instantiated.
  I'd probably have to change the internal class representation as well to fix 
that,
  and I don't know how many or what types of bugs/issues that would cause.

> That reminds me:
> I forgot about functions such as `compact()`, `extract()` and 
> `get_defined_vars()`.
> (and `func_num_args()`/`func_get_args()`/`func_get_arg()`).
> It looks like all of them check `zend_forbid_dynamic_call()` to throw,
> but I need to add tests and possibly flag the call as dynamic.

Update: The implementation already properly throws an Error.

Thanks,
- Tyson
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



[PHP-DEV] Here is a talk about the being a newbie in Internals

2020-02-09 Thread Midori Koçak
Hello,

My talk about proposing Null Coalescing Assignment operator in PHPConf 2019
in Istanbul was published. If you would like to share what is a newbie
experience to people who are interested inn internals you can share it. :)

https://www.youtube.com/watch?v=AdKya1HLDZQ=PLc15X3-Essu7Qc9lBGj-zZoYqhct8IESV=9

Thanks,
Midori