Re: [PHP-DEV] [RFC[ Property accessor hooks, take 2
Hi Rowan On Sat, Mar 16, 2024 at 8:23 PM Rowan Tommins [IMSoP] wrote: > > I still think there will be a lot of users coming from other languages, or > from using __get and __set, who will look at virtual properties first. Making > things less surprising for those people seems worth some effort, but I'm not > asking for a complete redesign. For clarity, you are asking for a way to make the "virtualness" of properties more explicit, correct? We touch on a keyword and why we think it's suboptimal in the FAQ section. Unfortunately, I cannot think of many alternatives. The `$field` variable made it a bit more obvious, but only marginally. I do believe that, for the most part, the user should not have to think much about whether the property is backed or virtual. The behavioral differences are mostly intuitive. For example: ```php class Test { // This property has a set hook that writes to the backing value. Since // we're using the backing value, it makes sense for there to be a way to // retrieve it. Without that, it wouldn't be useful. public $prop { set { $this->prop = strtoupper($value); } } // Similarly, a property with only a get hook that accesses the backing // value would need a way to write to the property for the get to be useful. public $prop { get => strtoupper($this->prop); } // A property with a get hook that does not use the backing value does not // need an implicit set operation, as writing to the backing value would be // useless, given that nobody will read it. public $prop { get => 42; } // Similarly, in the esoteric write-only case that does not use the backing // value, having an implicit get operation would always lead to a // "uninitialized property" error, and is not useful as such. public $prop { set { echo "Prop set\n"; } } } ``` Furthermore, `serialize`, `var_dump` and the other functions operating on raw property values will include the property only if it is backed. This also seems intuitive to me: If you never use the backing value, the backing value would always be uninitialized, so there's no reason to include it. One case that is not completely obvious is lazy-initialized properties. ```php class Test { public $prop { get => $this->prop ??= expensiveOperation(); } } ``` It's not immediately obvious that there is a public set operation here. The correct way to fix this would be with asymmetric visibility, which was previously declined. Either way, I don't consider this case alone enough to completely switch our approach. Please let me know if you are aware of any other potentially non-intuitive cases. I will admit that it is unfortunate that a user of the property has to look through the hook implementation to understand whether a property is writable. As you have previously suggested, one option might be to add an explicit `set;` declaration. Maybe it's a bit more obvious now, after my previous e-mail, why we are trying to avoid this. Apart from the things already mentioned, it's unclear to me whether, with such `set;` declarations, a `get`-only backed property should even be legal. With the complete absence of a write operation, the assignment within the `set` itself would fail. To make this work, the absence of `set;` would need to mean something like "writable, but only within another hook", which introduces yet another form of asymmetric visibility. > > Dynamic properties are not particularly relevant today. The point was > > not to show how similar these two cases are, but to explain that > > there's an existing mechanism in place that works very well for hooks. > > We may invent some new mechanism to access the backing value, like > > `field = 'value'`, but for what reason? This would only make sense if > > the syntax we use is useful for something else. However, given that > > without guards it just leads to recursion, which I really can't see > > any use for, I don't see the point. > > I can think of several reasons we *could* explore other syntax: > > 1) To make it clearer in code whether a particular line is accessing via the > hooks, or by-passing them 2) To make the code in the hooks shorter (e.g. > `$field` is significantly shorter than `$this->someDescriptiveName`) 3) To > allow code to by-pass the hooks at will, rather than only when called from > the hooks (e.g. having a single method that resets the state of several > lazy-loaded properties) > > Those reasons are probably not enough to rule out the current syntax; but > they show there are trade-offs being made. Fair enough. 1 and 2 are reasons why we added the `$field` macro as an alternative syntax in the original draft. I don't quite understand point 3. In Kotlin, `field` is only usable within its associated hook. Other languages I'm aware of do not provide a way to access the backing value directly, neither inside nor
Re: [PHP-DEV] [RFC[ Property accessor hooks, take 2
On 16/03/2024 17:51, Ilija Tovilo wrote: Properties can inherit both storage and hooks from their parent. Hopefully, that helps with the mental model. Of course, in reality it is a bit more complicated due to guards and references. That is a really helpful explanation, thanks; I hadn't thought about the significance of inheritance between hooked and non-hooked properties. I still think there will be a lot of users coming from other languages, or from using __get and __set, who will look at virtual properties first. Making things less surprising for those people seems worth some effort, but I'm not asking for a complete redesign. Dynamic properties are not particularly relevant today. The point was not to show how similar these two cases are, but to explain that there's an existing mechanism in place that works very well for hooks. We may invent some new mechanism to access the backing value, like `field = 'value'`, but for what reason? This would only make sense if the syntax we use is useful for something else. However, given that without guards it just leads to recursion, which I really can't see any use for, I don't see the point. I can think of several reasons we *could* explore other syntax: 1) To make it clearer in code whether a particular line is accessing via the hooks, or by-passing them 2) To make the code in the hooks shorter (e.g. `$field` is significantly shorter than `$this->someDescriptiveName`) 3) To allow code to by-pass the hooks at will, rather than only when called from the hooks (e.g. having a single method that resets the state of several lazy-loaded properties) Those reasons are probably not enough to rule out the current syntax; but they show there are trade-offs being made. To be honest, my biggest hesitation with the RFC remains asymmetric types (the ability to specify types in the set hook). It's quite a significant feature, with no precedent I know of, and I'm worried we'll overlook something by including it immediately. For instance, what will be the impact on people using reflection or static analysis to reason about types? I would personally be more comfortable leaving that to a follow-up RFC to consider the details more carefully. Nobody else has raised that, beyond the syntax; I'm not sure if that's because everyone is happy with it, or because the significance has been overlooked. Regards, -- Rowan Tommins [IMSoP]
Re: [PHP-DEV] [RFC[ Property accessor hooks, take 2
Hi Rowan On Sat, Mar 16, 2024 at 9:32 AM Rowan Tommins [IMSoP] wrote: > > On 16 March 2024 00:19:57 GMT, Larry Garfield wrote: > > >Well, reading/writing from within a set/get hook is an obvious use case to > >support. We cannot do cached properties easily otherwise: > > > >public string $expensive { > > get => $this->expensive ??= $this->compute(); > > set { > >if (strlen($value) < 50) throw new Exception(); > >$this->expensive = $value; > > } > >} > > > To play devil's advocate, in an implementation with only virtual properties, > this is still perfectly possible, just one declaration longer: > > private string $_expensive; > public string $expensive { > get => $this->_expensive ??= $this->compute(); > set { > if (strlen($value) < 50) throw new Exception(); > $this->_expensive = $value; > } > } > > Note that in this version there is an unambiguous way to refer to the raw > value from anywhere else in the class, if you wanted a clearAll() method for > instance. > > I can't stress enough that this is where a lot of my thinking comes from: > that backed properties are really the special case, not the default. Anything > you can do with a backed property you can do with a virtual one, but the > opposite will never be true. > > > The minimum version of backed properties is basically just sugar for that - > the property is still essentially virtual, but the language declares the > backing property for you, leading to: > > public string $expensive { > get => $field ??= $this->compute(); > set { > if (strlen($value) < 50) throw new Exception(); > $field = $value; > } > } > > I realise now that this isn't actually how the current implementation works, > but again I wanted to illustrate where I'm coming from: that backed > properties are just a convenience, not a different type of property with its > own rules. That's not really how we think about it. Our design decisions have been guided by a few factors: 1. The RFC intentionally makes plain properties and properties with hooks as fully compatible as possible. A subclass can override a plain property by adding hooks to it. Many other languages only allow doing so if the parent property already has generated accessors (`{ get; set; }`). For many of them, switching from a plain property to one with accessors is actually an ABI break. One requires generating assembly/IR instructions that access a field in some structure, the other one is a method call. This is not relevant in our case. In most languages, a consequence of `{ get; set; }` is that such properties cannot be passed by reference. This part _is_ relevant to PHP, because PHP makes heavy use of explicit by-reference passing for arrays, but not much else. However, as outlined in the RFC, arrays are not a good use-case for hooks to begin with. So instead of fragmenting the entirety of all PHP code bases into plain and `{ get; set; }` properties where it doesn't actually make a semantic difference, and then not even using them when it would matter (arrays), we have decided to avoid generated hooks altogether. The approach of making plain and hooked properties compatible also immediately means that a property can have both a "backing value" (inherited from the parent property) and hooks (from the child property). This goes against your model that backed properties are really just two properties, one for the backing value and a virtual one for the hooks. Our approach has the nice side effect of properties only containing hooks when they actually do something. We don't need to deal with optimizations like "the hook is auto-generated, revert to accessing the property directly to make it faster", or even just having the generated hook taking up unnecessary memory. You can think of our properties this way: ```php class Property { public ?Data $storage; public ?callable $getHook; public ?callable $setHook; public function get() { if ($hook = $this->getHook) { return $hook(); } else if ($storage) { return $storage->get(); } else { throw new Error('Property is write-only'); } } public function set($value) { if ($hook = $this->setHook) { $hook($value); } else if ($storage) { $storage->set($value); } else { throw new Error('Property is read-only'); } } } ``` Properties can inherit both storage and hooks from their parent. Hopefully, that helps with the mental model. Of course, in reality it is a bit more complicated due to guards and references. 2. Although you say backed properties are just syntactic, they really are not. For example, renaming a public property, making it private and replacing it with a new passthrough virtual property breaks serialization, as serialization works on the object's raw values. On the other hand, adding a hook to an existing property doesn't influence its backing
[PHP-DEV] XSLTProcessor recursion limit
Hi internals Based on https://bugs.php.net/bug.php?id=71571 I've opened a PR to implement two new properties for XSLTProcessor: maxTemplateDepth and maxTemplateVars. The reasoning behind this is that large templates with lots of recursion and/or variables can bump against the default recursion limit set by the libxslt library. PR link: https://github.com/php/php-src/pull/13731 I used an adapted version of https://github.com/nikic/popular-package-analysis to download every package that satisfies the search term "xsl". Then I checked if there are any classes that extend XSLTProcessor and have the maxTemplateDepth or maxTemplateVars field. None do, and as such no package in packagist will observe a BC break because of this PR. One sad detail however, is that you can set the recursion limit so high that you can exhaust the stack space, which will crash the process with a segfault. In fact, you can hit this already right now without the PR, using XSL. I.e. you can create a very deep VM re-entry that won't cause the stack limit protection to kick in, and then start a recursive XSL template processing that does not hit XSL's limit, but exhausts the remaining stack space. Note that as soon as you do callbacks however, the stack limit protection _will_ kick in. I tried to check if it's possible to prevent this, but the stack limit check would need to happen inside the libxslt library, so I don't think it's possible. Let me know if there are any complaints about this. If no complaints, I'll merge this in 2 weeks or so. Kind regards Niels
Re: [PHP-DEV] Supporting object types in BCMath
Hi Barney, > I would suggest if you do this only supporting immutable objects, or at least > making immutable the default, the one with with the simpler name, and > reserving the mutable version specifically for when people want to use > mutability. The strings used with bcmath today are effectively immutable. > > It's hard to see why a number should be mutable though. Users of the class > can always wrap an immutable up it in a mutable object if they want. > > If you do have both mutable and immutable it might be worth giving the > methods separate names to make the distinction clearer - e.g. "add" for > mutable, "plus" for immutable, and maybe making the add method return void. Thanks, that's what I was starting to worry about too. It seems like a good idea to support only immutability, as you say earlier in your proposal. Regards. Saki
Re: [PHP-DEV] Supporting object types in BCMath
On 16/03/2024 05:22, Saki Takamachi wrote: Yet another idea is to also support immutable objects, like DateTime. e.g. ``` $num1 = new BcNum('1.235'); $num1imm = new BcNumImmutable('1.235'); $num2 = new BcNum('2.001'); $num1result = $num1->add($num2); $num1immResult = $num1imm->add($num2); I would suggest if you do this only supporting immutable objects, or at least making immutable the default, the one with with the simpler name, and reserving the mutable version specifically for when people want to use mutability. The strings used with bcmath today are effectively immutable. It's hard to see why a number should be mutable though. Users of the class can always wrap an immutable up it in a mutable object if they want. If you do have both mutable and immutable it might be worth giving the methods separate names to make the distinction clearer - e.g. "add" for mutable, "plus" for immutable, and maybe making the add method return void.
Re: [PHP-DEV] Re: Supporting object types in BCMath
Hi Niels, > One advantage of a new API is that it could be more efficient than the > current API. > The current API takes input as strings and outputs strings as a result, and > therefore always has to reconstruct the BCMath internal state. > By keeping everything inside an object you could keep the internal state, > improving performance. Thanks for your feedback. I overlooked the cost of rebuilding the internal state. I'll give it a try and see if there's a significant difference in performance. I also noticed earlier that it may be better in some convenience aspects, such as the way to compare values. e.g. ``` $bcnum1 > $bcnum2 // bool ``` First, I'll try creating a simple prototype. Regards. Saki
Re: [PHP-DEV] Re: Supporting object types in BCMath
On 3/16/24 11:24, Saki Takamachi wrote: > Hi, > > After thinking about it, this could be done in userland. I'll try creating a > library myself first. > > Regards. > > Saki Hi Saki One advantage of a new API is that it could be more efficient than the current API. The current API takes input as strings and outputs strings as a result, and therefore always has to reconstruct the BCMath internal state. By keeping everything inside an object you could keep the internal state, improving performance. Whether it makes a difference in practice, I don't know. Kind regards Niels
[PHP-DEV] Re: Supporting object types in BCMath
Hi, After thinking about it, this could be done in userland. I'll try creating a library myself first. Regards. Saki
Re: [PHP-DEV] [RFC[ Property accessor hooks, take 2
On 16 March 2024 00:19:57 GMT, Larry Garfield wrote: >Well, reading/writing from within a set/get hook is an obvious use case to >support. We cannot do cached properties easily otherwise: > >public string $expensive { > get => $this->expensive ??= $this->compute(); > set { >if (strlen($value) < 50) throw new Exception(); >$this->expensive = $value; > } >} To play devil's advocate, in an implementation with only virtual properties, this is still perfectly possible, just one declaration longer: private string $_expensive; public string $expensive { get => $this->_expensive ??= $this->compute(); set { if (strlen($value) < 50) throw new Exception(); $this->_expensive = $value; } } Note that in this version there is an unambiguous way to refer to the raw value from anywhere else in the class, if you wanted a clearAll() method for instance. I can't stress enough that this is where a lot of my thinking comes from: that backed properties are really the special case, not the default. Anything you can do with a backed property you can do with a virtual one, but the opposite will never be true. The minimum version of backed properties is basically just sugar for that - the property is still essentially virtual, but the language declares the backing property for you, leading to: public string $expensive { get => $field ??= $this->compute(); set { if (strlen($value) < 50) throw new Exception(); $field = $value; } } I realise now that this isn't actually how the current implementation works, but again I wanted to illustrate where I'm coming from: that backed properties are just a convenience, not a different type of property with its own rules. > Being the same also makes the language more predictable, which is also a > design goal for this RFC. (Hence why "this is the same logic as > methods/__get/other very similar thing" is mentioned several times in the > RFC. Consistency in expectations is generally a good thing.) I can only speak for myself, but my expectations were based on: a) How __get and __set are used in practice. That generally involves reading and writing a private property, of either the same or different name from the public one; and that private property is visible everywhere equally, no special handling based on the call stack. b) What happens if you accidentally cause infinite recursion in a normal function or method, which is that the language eventually hits a stack depth limit and throws an error. So the assertion that the proposal was consistent with expectations surprised me. It feels to me like something that will seem surprising to people when they first encounter it, but useful once they understand the implications. Regards, Rowan Tommins [IMSoP]
Re: [PHP-DEV] Re: [PHP-CVS] [php-src] master: Deprecate implicit nullable parameter types (#12959)
Am 14.03.2024 um 15:55 schrieb Matteo Beccati: thanks, I had a quick look in the open issues and didn't find anything. AFAICS, https://github.com/nikic/PHP-Parser/pull/984 has not made it into a release yet. But apart from that there are no open issues on PHPUnit's side.