Re: [PHP-DEV] [RFC[ Property accessor hooks, take 2

2024-03-16 Thread Ilija Tovilo
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

2024-03-16 Thread Rowan Tommins [IMSoP]

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

2024-03-16 Thread Ilija Tovilo
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

2024-03-16 Thread Niels Dossche
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

2024-03-16 Thread Saki Takamachi
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

2024-03-16 Thread Barney Laurance

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

2024-03-16 Thread Saki Takamachi
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

2024-03-16 Thread Niels Dossche
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

2024-03-16 Thread Saki Takamachi
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

2024-03-16 Thread Rowan Tommins [IMSoP]



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)

2024-03-16 Thread Sebastian Bergmann

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.