Hi Jow,
First of all, I appreciate the amount of effort you already invested into this
idea.
Anyway, I still don't agree with the following terms of RFC and corresponding
implementation:
1) While parameters allow null to be accepted as the default value, null is
never a valid value for a typed property.
I think we must have a way to make properties explicitly nullable. Otherwise we
won't be able to define even simple binary tree...
class Node {
public Node $left = null;
public Node $right = null;
}
2) The implementation will raise an exception where a typed property is
accessed before being initialized:<https://3v4l.org/cVkcj/rfc#tabs>
This makes unnecessary complication, and opens thousands possibilities to cheat
the engine.
class A {
public int $x;
}
$a = new A;
var_dump($a->x++); // prints NULL (according to RFC, this should raise
Exception)
I propose to initialize typed properties with the value of corresponding type
(if default value is omitted).
boolean => false
int => 0
double => 0.0
string => ""
array => []
object, resource, callable => null
Actually, I propose implicit default values if they are not specified.
This will always guarantee the type and eliminate run-time checks on read.
3) It is possible to unset typed properties, and return them to the same state
as a property that was never set.
This mean that properties types are not guaranteed, and Optimizer/Compilers/JIT
won't be able to generate the best code performing type check on every read.
I would prefer to disable unset() of typed properties.
4) Checking the type of a zval is an extremely cheap operation, there may be
some very minor performance impact only when typed properties are used.
The patch makes some slowdown even if typed properties are not used (0.5% more
instruction retired on 100 requests to Wordpress according to callgrind)
5) In principle, knowing the type of a variable in advance allows you to make
optimizations, and in the long term, there is good reason to think that typed
properties will allow us to improve performance.
This is an optimistic assumption [😊]
For now typed-parameters make slowdown except of expected speedup, properties
are not going to change this.
Actually the type of property may make some gain only if we know the type of
object and its definition at compile-time, but in most cases we do't know the
class definition, because definition and usage are in different PHP scripts.
Especially with (2) and (3), even if we know type of object and property, we'll
still have to check if property is initialised on each usage.
Thanks. Dmitry.
________________________________
From: Dmitry Stogov
Sent: Wednesday, April 6, 2016 10:32
To: Joe Watkins
Subject: Typed properties patch
Ho Joe,
I see some tests failures (SIGSEGV)
> Test typed properties float widen at runtime
> [Zend/tests/type_declarations/typed_properties_027.phpt]
> Test typed properties respect strict types (off)
> [Zend/tests/type_declarations/typed_properties_028.phpt]
> Test typed properties unset __get magical magic
> [Zend/tests/type_declarations/typed_properties_030.phpt]
valgrind shows problems on every 4-rd test at Zend/tests (454 of 1668 tests),
but it seems like a single problem
$ cat ../Zend/tests/add_002.mem
==26053== Conditional jump or move depends on uninitialised value(s)
==26053== at 0x8627895: _zend_object_has_type_hints (zend_execute.h:367)
==26053== by 0x8629098: zend_std_write_property (zend_object_handlers.c:674)
==26053== by 0x85FA1B5: zend_update_property (zend_API.c:3894)
==26053== by 0x85FA38E: zend_update_property_string (zend_API.c:3952)
==26053== by 0x860F8DB: zend_default_exception_new_ex (zend_exceptions.c:222)
==26053== by 0x860F96F: zend_default_exception_new (zend_exceptions.c:236)
==26053== by 0x85F347E: _object_and_properties_init (zend_API.c:1303)
==26053== by 0x85F34B5: _object_init_ex (zend_API.c:1311)
==26053== by 0x86124F4: zend_throw_exception (zend_exceptions.c:881)
==26053== by 0x85EF256: zend_throw_error (zend.c:1316)
==26053== by 0x85E4622: add_function (zend_operators.c:969)
==26053== by 0x868824E: ZEND_ADD_SPEC_CV_CV_HANDLER (zend_vm_execute.h:44246)
Thanks. Dmitry.
