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

This is only part of a thread. view whole thread
  108432
February 10, 2020 01:34 tysonandre775@hotmail.com (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.
  108433
February 10, 2020 04:10 mike@newclarity.net (Mike Schinkel)
> On Feb 9, 2020, at 8:34 PM, tyson andre <tysonandre775@hotmail.com> 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