Re: [PHP-DEV] Re: [RFC] [Discussion] [VOTE] Rounding Integers as int

2024-03-18 Thread Bob Weinand

Hey Marc,

On 18.3.2024 08:53:01, Marc Bennewitz wrote:

Hi Bob,

On 17.03.24 14:59, Bob Weinand wrote:

On 17.3.2024 13:23:04, Marc Bennewitz wrote:

Hello internals,

I have opened the vote for the "Rounding Integers as int" RFC:
https://wiki.php.net/rfc/integer-rounding

Do to Easter weekend the vote will run for two weeks and two days 
until Tue the 2nd of April 2024.


Best regards,

Marc Bennewitz


Hey Marc,

I've voted no; it should be just changed without any force_float 
parameter. Just always return int when possible (and the input was int).
If users wish to have the old behaviour, they should just explicitly 
cast via (float).


The effective BC break of that would be quite small if some things 
which return float today now would return int. I cannot imagine many 
cases where this would actually be unwanted. And as said, explicit 
(float) casts are always possible.


I also dislike force_float, as it cannot just be added to a function 
in any code which shall be backwards compatible to 8.3 and older. It 
would just emit Uncaught Error: Unknown named parameter $force_float.


Changing the return type from float to int is a non trivial quite hard 
to find behavior change.


Imaging code like this:

$x = 800;
$y = 800;
round($x/$y) === 1.0;

This will return false instead of true especially because we teach 
users to use strict comparison.

Such behavior change should be done in a major version.


I see, here we disagree:
- Strict comparison should be avoided when working with numbers. Strict 
comparisons are generally for strings and booleans.

- There's no reason to artificially wait years here.

With the additional parameter it's possible to opt-in into the new 
behavior already in 8.4 while in PHP 9.0 the default behavior will 
change but previously opted in code does not need to get touched again.


Just changing the behavior means waiting for PHP 9.0 without a way to 
opt-in in 8.4 already. If you are not interested in opting in in 8.4 
already you can just ignore the additional argument as this will be 
the default in 9.0.
I'm not interested in having an additional parameter I have to carry 
forward for quite some years.
To mimic the previous behavior in a fully BC way it's as simple as 
explicitly casting the value to float.


I would rather have preferred to see a static analysis solution how 
often round()ed results are compared strictly, assess the actual BC 
impact and possibly encourage tools like Rector to recognize such patterns.




Bob


Regards,
Marc


Bob


Re: [PHP-DEV] Proposal: Arbitrary precision native scalar type

2024-03-18 Thread Rowan Tommins [IMSoP]

On 18/03/2024 04:39, Alexander Pravdin wrote:

I'm not in the context of the core team plans regarding "strict
types". Could you share some details here? What is the current plan
regarding it? To make strict types on by default eventually? Or
something else?



PHP doesn't really have a defined "core team". There are contributors 
who are particularly active at a given time (sometimes, but far from 
always, because someone is paying them), contributors who are 
particularly long-standing and respected, contributors who throw 
themselves into a pet project and make it happen, and so on.


Partly as a consequence of this, it's often hard to pin down any 
long-term plan about anything, outside of what particular people would 
like to see. So Gina's opinion (it was suffixed "IMHO") that strict 
types was a mistake shouldn't be read as "we have a solid plan for what 
is going to replace strict_types which everyone is on board with".


I think a reasonable number of people do share the sentiment that having 
two separate modes was a mistake; and neither mode is actually perfect. 
It's not about "making it on by default", it's about coming up with a 
unified behaviour that makes the setting redundant.



All of which is something of a diversion from the topic at hand, which 
is this:



How can we introduce the ability to write user code in default
decimals and at the same time keep the old way of working as it was
before, to not introduce any troubles into the existing code and not
introduce performance issues? As a user, I would like to have a
choice.




I don't think choice is really what you want: if you were designing a 
language from scratch, I doubt you would say "let's give the user a 
choice of what type 1 / 10 returns". What it's actually about is 
*backwards compatibility*: what will happen to code that expects 1/10 to 
give a float, if it suddenly starts giving a decimal.


For most cases, I think the rule can be as simple as "decimal in means 
decimal out". What's maybe not as obvious at first sight is that that 
can apply to operators as functions, and already does: 100 / 10 gives 
int(10), but 100.0 / 10  gives float(10.0), as do 100 / 10.0  and 100.0 
/ 10.0


By the same logic, decimal(1) / 10 can produce decimal(0.1) instead of 
float(0.1), and we don't need any fancy directives. Even better if we 
can introduce a shorter syntax for decimal literals, so that it becomes 
1_d / 10



Where things get more complicated is with *fixed-precision* decimals, 
which is what is generally wanted for something like money. What is the 
correct result of decimal(1.03, precision: 2) / 2 - decimal(0.515, 3)? 
decimal(0.51, 2)? decimal (0.52, 2)? an error? And what about 
decimal(10) / 3?


If you stick to functions / methods, this is slightly less of an issue, 
because you can have decimal(1.03, 2)->dividedBy(2, RoundingMode::DOWN) 
== decimal(0.51, 2); or decimal(1.03, 2)->split(2) == [ decimal(0.52, 
2), decimal(0.51, 2) ] Example names taken directly from the brick/money 
package.


At that point, backwards compatibility is less of an issue as well: make 
the new functions convenient to use, but distinct from the existing ones.



In short, the best way of avoiding declare() directives is not to 
replace them with something else, but to choose a design where nobody 
feels the need for them.


Regards,

--
Rowan Tommins
[IMSoP]


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

2024-03-18 Thread Larry Garfield
On Mon, Mar 18, 2024, at 9:13 AM, Lynn wrote:

> In regards to arrays, what about additional operations next to get/set? 
> I doubt this solution will cover all the use-cases or perhaps even 
> over-complicate things, just throwing the idea out there.
>
> ```php
> class Test {
> private array $_myData = [];
> public array $myData {
> get => $this->_myData;
> append => $this->_myData[] = $value;
> }
> }
> ``` 
>
>  Thinking about the other post about offset and containers 
> (https://github.com/Girgias/php-rfcs/blob/master/container-offset-behaviour.md):
> ```php
> class Test {
> private array $_myData = [];
> public array $myData {
> get => $this->_myData;
> append => $this->_myData[] = $value;
> write_dimension => $this->_myData[$offset] = $value;
> }
> }
> ```

Those hooks  may be possible; we'd have to try it and see.  However, they also 
wouldn't be able to fully emulate arrays.  $foo->bar['baz'][] = 'beep' could 
get very weird.  Those wouldn't cover unsetting.  Array functions like 
array_splice() still wouldn't work.  Basically, there will never be a way to 
make arrays 100% transparent with hooks.  That's why the RFC recommends still 
using a method for array modification, as that's already a very well-understood 
and flexible approach.

Fortunately, as currently written (append and write_dimension are forbidden), 
those additional hooks could be considered and added in their own RFC in the 
future with no BC break.  Whether or not they make sense or cover "enough" use 
cases is a question that could be answered in the future, after Gina's RFC 
passes.

So on this one, I think "punt" is the best option for now.  It can be safely 
revisited in the future.

> Is this issue restricted to arrays only? From my understanding objects 
> functioning as arrays are already by reference and thus should not 
> suffer from this?
> ```php
> class Test {
> private ArrayObject $_myData;
> public ArrayObject $myData {
> get => $this->_myData;
> }
>
> public function __construct() {
> $this->_myData = new ArrayObject();
> }
> }
>
> // would this work without issues?
> $obj = new Test();
> $obj->myData[] = 'test';
> ```

Mostly correct.  Objects pass by handle, not by reference.  (You can pass an 
object by reference instead, and it behaves subtly differently.)  But the net 
effect is the same.  Your sample code there would run fine.

--Larry Garfield


Re: [PHP-DEV] XSLTProcessor recursion limit

2024-03-18 Thread Arnaud Le Blanc
Hi Niels,

On Sat, Mar 16, 2024 at 5:49 PM Niels Dossche 
wrote:

> 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
>

For PCRE there is a pcre.recursion_limit setting [1] with the same issue.
This is fine because that's a convenience feature (not a security one) and
it is useful by catching most cases of uncontrolled recursion in practice.

The default value is not changed by this PR (so there is no risk of BC
break by lowering the limit), and appears to be low enough to prevent stack
exhaustions on a default 2MiB stack on Linux.

+1 for this

[1]
https://www.php.net/manual/en/pcre.configuration.php#ini.pcre.recursion-limit

Kind regards,
Arnaud


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

2024-03-18 Thread Rowan Tommins [IMSoP]

On 18/03/2024 00:04, Ilija Tovilo wrote:

I realize this is somewhat inconsistent, but I believe it is
reasonable. If you want to expose the underlying property
by-reference, you need to jump through some additional hoops.



I disagree with this reasoning, because I foresee plenty of cases where 
a virtual property is necessary anyway, so doesn't provide any 
additional hoop to jump through.


But there's not much more to say on this point, so I guess we'll leave 
it there.





Again, it depends on how you think about it. As you have argued, for a
get-only property, the backing value should not be writable without an
explicit `set;` declaration. You can interpret `set;` as an
auto-generated hook, or as a marker that indicates that the backing
value is accessible without a hook.



Regardless of which of these views you start with, it still seems 
intuitive to me that accesses inside the get hook would bypass the 
normal rules and write to the raw value.


Leaving aside the implementation, there are three things that can happen 
when you write to a property:


a) the set hook is called
b) the raw property is written to
c) an error is thrown

Inside the dynamic scope of a hook, the behaviour is always (b), and I 
don't see any reason for that to change. From anywhere else, backed 
properties currently try (a) and fall back to (b); virtual properties 
try (a) and fall back to (c).


I do understand that falling back to (b) makes the implementation 
simpler, and works well with inheritance and some use cases; but falling 
back to (c) wouldn't necessarily need a "default hook", just a marker of 
"has hooks".


It occurred to me you could implement it in reverse: auto-generate a 
hook "set => throw new Error;" and then *remove* it if the user opts in 
to the default set behaviour. That would keep the "write directly" case 
optimised "for free"; but it would be awkward for inheritance, as you'd 
have to somehow avoid calling the parent's hook.





The meaning for `set;` is no longer clear. Does it mean that there's a
generated hook that accesses the backing field? Does it mean that the
backing field is accessible without a hook? Or does it mean that it
accesses the parent hook? The truth is, with inheritance there's no
way to look at the property declaration and fully understand what's
going on, unless all hooks must be spelled out for the sake of clarity
(e.g. `get => parent::$prop::get()`).



Yes, I think this is probably a good argument against requiring "set;"

I think "be careful when inheriting only one hook" will always be a key 
rule to teach anyway, because it's easy to mess up (e.g. assuming the 
parent is backed and accessing $this->foo, rather than calling the 
parent's hook implementation). But adding "set;" into the mix probably 
just makes it worse.





I seriously doubt accessing the backing value outside of the current
hook is useful. The backing value is an implementation detail. If it
is absolutely needed, `ReflectionProperty::setRawValue()` offers a way
to do it. I understand the desire for a shorter alternative like
`$field`, but it doesn't seem like the majority shares this desire at
this point in time.



The example of clearAll() is a real use case, which people will 
currently achieve with __get and __set (e.g. the Yii ActiveRecord 
implementation I linked in one of my previous messages).


The alternative wouldn't be reflection, it would just be switching to a 
virtual property with the value stored in a private field. I think 
that's fine, it's just drawing the line of which use cases backed 
properties cover: Kotlin covers more use cases than C#; PHP will cover 
more than Kotlin (methods able to by-pass a hook when called from that 
hook); but it will draw the line here.





A different syntax like `$this->prop::raw` comes with similar
complexity issues, similar to those previously discussed for
`parent::$prop`/`parent::$prop = 'prop'`.



Yeah, I can't even think of a nice syntax for it, let alone a nice 
implementation. Let's leave it as a thought experiment, no further 
action needed. :)



Regarding asymmetric types:


I can't speak for IDEs or static
analyzers, but I'm not sure what makes this case special. We can ask
some of their maintainers for feedback.



In order to reliably tell the user whether "$a->foo = $b->bar;" is a 
type-safe operation, the analyser will need to track two types for every 
property, the "gettable type" and the "settable type", and apply them in 
the correct contexts.


I've honestly no idea whether that will be easy or hard; it will 
probably vary between tools. In particular, I get the impression IDEs / 
editor plugins sometimes have a base implementation used for multiple 
programming languages, and PHP might be the only one that needed this 
extra tracking.



Regards,

--
Rowan Tommins
[IMSoP]


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

2024-03-18 Thread Lynn
On Wed, Feb 21, 2024 at 7:58 PM Larry Garfield 
wrote:

> Hello again, fine Internalians.
>
> After much on-again/off-again work, Ilija and I are back with a more
> polished property access hooks/interface properties RFC.  It’s 99%
> unchanged from last summer; the PR is now essentially complete and more
> robust, and we were able to squish the last remaining edge cases.
>
> Baring any major changes, we plan to bring this to a vote in mid-March.
>
> https://wiki.php.net/rfc/property-hooks
>
> It’s long, but that’s because we’re handling every edge case we could
> think of.  Properties involve dealing with both references and inheritance,
> both of which have complex implications.  We believe we’ve identified the
> most logical handling for all cases, though.
>
> Note the FAQ question at the end, which explains some design choices.
>
> There’s one outstanding question, which is slightly painful to ask:
> Originally, this RFC was called “property accessors,” which is the
> terminology used by most languages.  During early development, when we had
> 4 accessors like Swift, we changed the name to “hooks” to better indicate
> that one was “hooking into” the property lifecycle.  However, later
> refinement brought it back down to 2 operations, get and set.  That makes
> the “hooks” name less applicable, and inconsistent with what other
> languages call it.
>
> However, changing it back at this point would be a non-small amount of
> grunt work. There would be no functional changes from doing so, but it’s
> lots of renaming things both in the PR and the RFC. We are willing to do so
> if the consensus is that it would be beneficial, but want to ask before
> putting in the effort.
>
> --
>   Larry Garfield
>   la...@garfieldtech.com


In regards to arrays, what about additional operations next to get/set? I
doubt this solution will cover all the use-cases or perhaps even
over-complicate things, just throwing the idea out there.

```php
class Test {
private array $_myData = [];
public array $myData {
get => $this->_myData;
append => $this->_myData[] = $value;
}
}
```

 Thinking about the other post about offset and containers (
https://github.com/Girgias/php-rfcs/blob/master/container
-offset-behaviour.md):
```php
class Test {
private array $_myData = [];
public array $myData {
get => $this->_myData;
append => $this->_myData[] = $value;
write_dimension => $this->_myData[$offset] = $value;
}
}
```

Is this issue restricted to arrays only? From my understanding objects
functioning as arrays are already by reference and thus should not suffer
from this?
```php
class Test {
private ArrayObject $_myData;
public ArrayObject $myData {
get => $this->_myData;
}

public function __construct() {
$this->_myData = new ArrayObject();
}
}

// would this work without issues?
$obj = new Test();
$obj->myData[] = 'test';
```


Re: [PHP-DEV] Re: [RFC] [Discussion] [VOTE] Rounding Integers as int

2024-03-18 Thread Marc Bennewitz

Hi Vincent,

On 17.03.24 22:59, Vincent Langlet wrote:

Hi,

IMHO, the more meaningful cases of the RFC are missing :
round(float, precision: >= 0): int|float // only cast to float for int overflow
ceil(float): int|float // only cast to float for int overflow
floor(float): int|float // only cast to float for int overflow
Calling ceil or floor on integer is meaningless, because it will 
return the same value.

The RFC should be "Prefer int as return value when possible".

I do believe it's important to avoid unnecessary casts as much as 
possible and leave it up to the user making cast explicit than implicit 
else the behavior gets quite unpredictable.


Implicitly casting to float only happens on int underflow/overflow. 
Implicitly casting to int will never happen.


As you say "Calling ceil or floor on integer is meaningless, because it 
will return the same value." this is already noted in the RFC and the 
functions will be a no-op in this case.


Best regards,
Marc



OpenPGP_0x3936ABF753BC88CE.asc
Description: OpenPGP public key


OpenPGP_signature.asc
Description: OpenPGP digital signature


Re: [PHP-DEV] Re: [RFC] [Discussion] [VOTE] Rounding Integers as int

2024-03-18 Thread Marc Bennewitz

Hi Bob,

On 17.03.24 14:59, Bob Weinand wrote:

On 17.3.2024 13:23:04, Marc Bennewitz wrote:

Hello internals,

I have opened the vote for the "Rounding Integers as int" RFC:
https://wiki.php.net/rfc/integer-rounding

Do to Easter weekend the vote will run for two weeks and two days 
until Tue the 2nd of April 2024.


Best regards,

Marc Bennewitz


Hey Marc,

I've voted no; it should be just changed without any force_float 
parameter. Just always return int when possible (and the input was int).
If users wish to have the old behaviour, they should just explicitly 
cast via (float).


The effective BC break of that would be quite small if some things 
which return float today now would return int. I cannot imagine many 
cases where this would actually be unwanted. And as said, explicit 
(float) casts are always possible.


I also dislike force_float, as it cannot just be added to a function 
in any code which shall be backwards compatible to 8.3 and older. It 
would just emit Uncaught Error: Unknown named parameter $force_float.


Changing the return type from float to int is a non trivial quite hard 
to find behavior change.


Imaging code like this:

$x = 800;
$y = 800;
round($x/$y) === 1.0;

This will return false instead of true especially because we teach users 
to use strict comparison.

Such behavior change should be done in a major version.

With the additional parameter it's possible to opt-in into the new 
behavior already in 8.4 while in PHP 9.0 the default behavior will 
change but previously opted in code does not need to get touched again.


Just changing the behavior means waiting for PHP 9.0 without a way to 
opt-in in 8.4 already. If you are not interested in opting in in 8.4 
already you can just ignore the additional argument as this will be the 
default in 9.0.


To mimic the previous behavior in a fully BC way it's as simple as 
explicitly casting the value to float.




Bob


Regards,
Marc




OpenPGP_0x3936ABF753BC88CE.asc
Description: OpenPGP public key


OpenPGP_signature.asc
Description: OpenPGP digital signature