Re: [PHP-DEV] [RFC][Discussion] NotSerializable attribute

2024-01-03 Thread G. P. B.
On Wed, 3 Jan 2024 at 13:17, Max Semenik  wrote:

> > this actively prevents writing a child class that'd make a parent
> serializable if it wants to.
>
> Wouldn't this violate LSP?
>

No it doesn't.
Making a child class unserializable if the parent is serializable however
is violating LSP, and probably a test should be added for this.

Best regards,

Gina P. Banyard


Re: [PHP-DEV] Pre-RFC: Fixing spec bugs in the DOM extension

2023-12-29 Thread G. P. B.
On Tue, 26 Dec 2023 at 21:45, Niels Dossche  wrote:

> Hi internals
>
> The DOM extension in PHP is used to parse, query and manipulate XML/HTML
> documents. The DOM extension is based on the DOM specification.
> Originally this was the DOM Core Level 3 specification, but nowadays, that
> specification has evolved into the current "Living Specification"
> maintained by WHATWG.
>
> Unfortunately, there are many bugs in PHP's DOM extension. Most of those
> bugs are related to namespace and attribute handling. This leads to people
> trying to work around those bugs by relying on more bugs, or on
> undocumented side-effects of incorrect behaviour, leading to even more
> issues in the end. Furthermore, some of these bugs may have security
> implications [1].
>
> Some of these bugs are caused because the method or property was
> implemented incorrectly back in the day, or because the original
> specification used to be unclear. A smaller part of this is because the
> specification has made breaking changes when HTML 5 first came along and
> the specification creators had to unify what browsers implemented into a
> single specification that everyone agreed on.
>
> It's not possible to "just fix" these bugs because people actually _rely_
> on these bugs. They are also often unaware that what they're doing is
> actually incorrect or causes the internal document state to be
> inconsistent. We therefore have to fix this in a backwards-compatible way:
> i.e. a hard requirement is that all code written for the current DOM
> extension keeps working without requiring changes.
> In short: the main problem is that 20 years of buggy behaviour means that
> the bugs have become ingrained into the system.
>
> Some people have implemented userland DOM libraries on top of the existing
> DOM extension. However, even userland solutions can't fully work around
> issues caused by PHP's DOM extension. The real solution is to provide a
> BC-preserving fix at PHP's side.
>
> Roughly 1.5 months ago I merged my HTML 5 RFC [2] into the PHP 8.4
> development branch. This RFC introduced new document classes:
> DOM\HTMLDocument and DOM\XMLDocument. The idea here was to preserve
> backwards compatibility: if the user wants to keep using HTML 4, they can
> keep using the DOMDocument class. Also, when the user wants to work with
> HTML 5 and are currently using workarounds, they can migrate on their own
> pace (without deprecations or anything) to the new classes. New code can
> use DOM\{HTML,XML}Document from the start without touching the old classes.
>
> The HTML 5 RFC has left us with an interesting opportunity to also
> introduce the spec bugfixes in a BC-preserving way. The idea is that when
> the new DOM\{HTML,XML}Document classes are used, then the DOM extension
> will follow the DOM specification and therefore get rid of bugs. When you
> are using the DOMDocument class, the old implementations will be used. This
> means that backwards compatibility is kept.
>
> For the past 2.5 weeks I've been working on getting all spec bugs that I
> know of fixed. The full list of bugs that this proposal fixes can be found
> here:
> https://github.com/nielsdos/php-src/blob/dom-spec-compliance-pub/bugs.md.
> I also found some discussion [3] from some years ago where C. Scott shared
> a list of problems they encountered at Wikimedia [4]. All behavioural
> issues are fixed in my PR [5], although my PR could always use more
> testing. Currently I have tested that existing DOM code does not break (I
> have tested veewee's XML library, Mensbeam library, some SimpleSAML
> libraries). I have added tests to test the new spec-compliant behaviour. I
> also ported some of the WHATWG's WPT DOM tests (DOM spec-compliance
> testsuite) to PHP and those that I've ported all pass [6].
>
> Implementation PR can be found here:
> https://github.com/php/php-src/pull/13031
>
> Note that this is not a new extension, but an improvement to the existing
> DOM extension. As for "why not an entirely new extension?", please see the
> reasoning in my HTML 5 RFC. All interactions with SimpleXML, XSL, XPath etc
> will remain possible like you are used to. Implementation-wise, a lot of
> code internally is shared between the spec-compliant and old
> implementations.
>
> I intend to put this up for RFC. There is however one last detail that
> needs to be cleared up: what about "type issues"?
> To give an example of a "type issue": there is a `string DOMNode::$prefix`
> property. DOM spec tells us that this should be nullable: when there is no
> prefix for a node, the prefix should return NULL. However, because the
> property is a string, this currently returns an empty string instead in
> PHP. Not a big deal maybe, but there's many of these subtle
> inconsistencies: null vs false return value, arguments that should accept
> `?string` instead of `string`, etc.
> Sadly, it's not possible to fix the typing issues for properties and
> methods for DOMNode, DOMElement, ... because of BC: 

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

2023-12-14 Thread G. P. B.
On Wed, 13 Dec 2023 at 09:29, Alexander Pravdin 
wrote:

> On Wed, Dec 13, 2023 at 6:11 PM Robert Landers 
> wrote:
>
> I just ran `apt install php8.3-decimal` and tried this:
> >
> > $a = new Decimal\Decimal("1", 2);
> > $b = $a + $a;
> > PHP Warning:  Uncaught TypeError: Unsupported operand types:
> > Decimal\Decimal + Decimal\Decimal in
> >
> > So, it appears not.
> >
>
> I've pointed out this issue earlier in the discussion and Gina P. Banyard
> has replied that this can be a PHP 8.3 bug and he will look into it.
>
> BTW, my main concern is that this amazing extension doesn't look like it is
> actively maintained and I'm worrying if I will be able to maintain my
> projects that depend on it in the future.
>

Hot take, why not fund the maintainer (or someone else to fork and pick up
the work) to work on this extension?
It is not like most bundled extension within php-src are truly actively
maintained.
So why would integrating ext/decimal mean it will be maintained?
Just have a look at all the XML/DOM related extensions that were riddled
with bugs and unmaintained until Nils decided to pick up the work to
scratch an itch.

Moreover, being separate from "core" is frankly a benefit, and I very much
dislike the current status of having so many bundled extensions that cannot
improve at their own pace.
Having extensions not be tied to php-src's release cycle means they can
move more quickly, fix bugs faster, evolve more easily, etc.
Just look at ext/curl, being stuck at a certain level of support for
libcurl because it *must* be tied to the annual PHP release cycle.

Best regards,

Gina P. Banyard

PS: My pronouns are she/her


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

2023-12-14 Thread G. P. B.
On Tue, 12 Dec 2023 at 21:00, Robert Landers 
wrote:

> On Tue, Dec 12, 2023 at 2:04 PM G. P. B.  wrote:
> > GMP supports operator overloading
>
> GMP kinda-sorta-most-of-the-time supports operator overloading.
> Sometimes ... it doesn't. I implemented a field library in PHP (for
> work a couple of years ago) and occasionally, overloading would cast
> things back to float/int and break the math. I don't have access to
> that code, so I don't have any examples readily available (and the
> fact that an extension can do overloading but we can't in user-land is
> a whole different can of worms which made this library ridiculously
> hard to work with -- we rewrote everything in Scala and never looked
> back). Needless to say, if I were to go into a project that required
> GMP, I wouldn't trust the overloading.
>

I have no idea how _this_ is possible considering GMP will happily throw
type errors left and right even in cases when it shouldn't.
If you encountered this, you should have submitted a bug report.
Because, using a potential bug as an excuse for not doing this is... weird?

I have come around userland operator overloading with the proposal from
Jordan, but considering this hasn't passed it might take a while before
someone gives it a crack at it again.
And it has _always_ been a thing that the engine, and internal extensions,
can do more things than userland.
Especially nonsensical stuff like variadic parameters not at the end...

Gina P. Banyard


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

2023-12-12 Thread G. P. B.
Gina P. Banyard

On Tue, 12 Dec 2023 at 08:51, Alexander Pravdin 
wrote:

> I just found out that under PHP 8.3, basic arithmetic operations on the
> Decimal object variables are no longer supported and cause TypeError. So
> this is one more point to implementing native decimals in PHP.
>

This is either a bug in the extension or a bug in PHP 8.3 (which I doubt)
but I will have a look at this.

Gina P. Banyard


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

2023-12-12 Thread G. P. B.
On Fri, 8 Dec 2023 at 10:14, Alexander Pravdin 
wrote:

> On Thu, Dec 7, 2023 at 11:36 PM G. P. B.  wrote:
>
> - Objects are always casted to true, GMP(0) will equal to true.
>>>
>>
>> This is incorrect, GMP object do _not_ support casts to bool
>> See https://3v4l.org/LHpD1
>>
>
> This is weird. Any PHP user would expect that a zero number can be easily
> casted to boolean false. This is also why I think we need a scalar decimal
> type.
>

Internal objects can overload casts, SimpleXML overloads the boolean cast
already.
We could add this to GMP to return false for 0.


> It works the same as "float" in terms of its usage and type casting except
>>> for one thing. Float value can be passed to a decimal argument or
>>> typecasted with a warning like "Float to decimal conversion may incur
>>> unexpected results".
>>>
>>> Decimal to float conversion is allowed and smooth:
>>>
>>> function f (float $value) {}
>>>
>>> f(0.2);
>>>
>>> f(0.2d); // allowed with no warnings
>>>
>>
>> I disagree with this part, floats can not represent decimal values
>> properly e.g.
>> $f1 = 0.1;
>> $f2 = 0.2;
>> $f3 = 0.3;
>> var_dump($f1 + $f2 === $f3);
>>
> will return false.
>> As such, floats are not at all compatible with decimals.
>>
>
> Yes, I know that. I mentioned that we can not convert float to decimal
> without an intermediate value where we apply rounding and get a
> human-readable value in some representation (string or whatsoever).
>
> My intention was to provide implicit conversion with warnings because I
> guess that the majority of end users will expect that numeric scalar types
> are interoperable. To not make them scared and point them to issues in
> their code instead of slapping their hands immediately when they run
> something that is not so correct.
>
>
>
>> Moreover, the whole point of adding a warning when implicit conversions
>> from int to float happen was to be able to warn before elevating the
>> warning to a TypeError.
>>
>
> Sorry, I'm not aware of issues of converting ints to floats. Are there any
> issues? I understand the issue of the backward conversion, but not the
> forward one. Could you add details here?
>

I meant converting floats to int.
But you can lose precision when converting an integer to float as there are
only 53 bits for the coefficient.


> Therefore, introducing behaviour that warns instead of throwing a
>> TypeError is already a no-go from my PoV.
>>
>
> I'm okay with TypeErrors for float to decimal conversions as well. This is
> not a key point of my proposal. If the community prefers errors instead of
> warnings - that's also fine.
>
>
> Literal numbers in the code are converted to floats by default. If
>>> prepended by the "(decimal)" typecast, the decimal result is produced
>>> without an intermediary float.
>>>
>>
>> This behaviour is rather suboptimal, I'd rather have literals be decimals,
>> as decimals to floats should always be a reasonable implicit coercion
>> considering we already do int to float.
>>
>
> Is it optimal to perform conversions on every fractional literal? This is
> also not a key point for me and I'm okay with any implementation that the
> internals community prefers.
>
>
>
>>  New declare directive "default_decimal" is added. When used, literals and
>>
>>> math operations return decimal by default instead of float. This is to
>>> simplify creating source files working with decimals only.
>>>
>>
>> Please no, no new declare directives that affect engine behaviour.
>> Strict types was already a mistake because of this IMHO.
>>
>
> I didn't know that the strict types directive was a mistake. My intention
> is to be able to write clean all-decimal units of code and not break the
> backward compatibility. The old code should work as it was before. At the
> same time, there are use cases when the whole class/project should be
> written with decimals only. As a user, I want to do that without complex
> language structures and excessive typehints or explicit conversions. The
> all-decimal code  should be as clean as it is now with floats. This is why
> I proposed this directive. Can you suggest something better to achieve the
> same?
>

The issue is that I don't think having arbitrary precision decimals as a
core language feature is a necessity compared to rational types.
A cast from rational to float wouldn't produce a large round trip, whereas
trying to figure out arbitrary precision is more diffi

Re: [PHP-DEV] [RFC][Discussion] NotSerializable attribute

2023-12-09 Thread G. P. B.
The implementation is simple and straight to the point, and uses machinery
that has been added to the engine to deal more consistently with internal
classes.

In an ideal world (IMHO) serialization would be opt-in and __serialize()
would be used to enable and describe the serialization format.
However, this is not the case, and IMHO implementing __serialize() should
only be done to change the *default* serialization format *not* to disable
it.

Exposing this to userland makes sense, as this would prevented the mess
that was the Serializable interface that people used to disable
serialization which was less then ideal.

Moreover, having this as an attribute means, that even without adding a new
method to ReflectionClass, you could determine via Reflection if this class
is serializable or not.
Because currently, the only way to know if it is *actually* serializable is
to call serialize() on the object and see what happens.


On Sat, 9 Dec 2023 at 17:16, Rowan Tommins  wrote:

> On 9 December 2023 12:30:29 GMT, Max Semenik 
> wrote:
> >Hi, I'd like to propose a new attribute, #[NotSerializable]. This
> >functionality is already available for internal classes - userspace should
> >benefit from it, too.
>
> If this ends up approximately the same as implementing serialisation as an
> exception, it feels quite a thin feature. If you put __sleep and __wakeup
> as shown into a trait, it's already as short and explicit as "use
> NotSerializable;"
>

If the alternative solution is a trait, then it is, IMHO, a bad alternative
solution.

What would make it more compelling is if the engine itself could do more
> with the attribute. For instance, a direct isSerializable() on
> ReflectionClass that covered both internal and attribute-marked classes.
>

I do agree, adding a isSerializable() function to ReflectionClass is a good
idea.

Best regards,

Gina P. Banyard


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

2023-12-07 Thread G. P. B.
On Thu, 7 Dec 2023 at 06:36, Alex Pravdin  wrote:

> Hello internals,
> [...]
> GMP:
> - Workaround: implements the GMP class that allows basic math operations.
> - Requires using separate functions for the rest of operations.
>
> - Objects are always casted to true, GMP(0) will equal to true.
>

This is incorrect, GMP object do _not_ support casts to bool
See https://3v4l.org/LHpD1


> It works the same as "float" in terms of its usage and type casting except
> for one thing. Float value can be passed to a decimal argument or
> typecasted with a warning like "Float to decimal conversion may incur
> unexpected results".
>
> Decimal to float conversion is allowed and smooth:
>
> function f (float $value) {}
>
> f(0.2);
>
> f(0.2d); // allowed with no warnings
>

I disagree with this part, floats can not represent decimal values properly
e.g.
$f1 = 0.1;
$f2 = 0.2;
$f3 = 0.3;
var_dump($f1 + $f2 === $f3);

will return false.
As such, floats are not at all compatible with decimals.

Moreover, the whole point of adding a warning when implicit conversions
from int to float happen was to be able to warn before elevating the
warning to a TypeError.
Therefore, introducing behaviour that warns instead of throwing a TypeError
is already a no-go from my PoV.


> [...]
>
> Literal numbers in the code are converted to floats by default. If
> prepended by the "(decimal)" typecast, the decimal result is produced
> without an intermediary float.
>

This behaviour is rather suboptimal, I'd rather have literals be decimals,
as decimals to floats should always be a reasonable implicit coercion
considering we already do int to float.


> New declare directive "default_decimal" is added. When used, literals and
> math operations return decimal by default instead of float. This is to
> simplify creating source files working with decimals only.
>

Please no, no new declare directives that affect engine behaviour.
Strict types was already a mistake because of this IMHO.


> New language construct "as_decimal()" is added to produce decimal math
> results for literals and math operations instead of float without
> intermediary float:
>
> $var = 5 / 2; // returns float 2.5
> $var = as_decimal(5 / 2); // returns decimal 2.5
>
> This is a kind of "default_decimal" for a specific operation.
>

Again, this should return a decimal instead IMHO.


> If mixed float and decimal operands are used in a math operation, decimal
> is converted to float by default. If "default_decimal" directive or
> "as_decimal()" construct is used, float is converted to decimal (with a
> warning):
>
> $f = (float) 0.2;
> $d = (decimal) 0.2;
>
> $r = $f + $d; // returns float result by default
> $r = as_decimal($f + $d); // returns decimal result with a warning about
> implicit float to decimal conversion
>
> All builtin functions that currently accept float also accept decimal. So
> users don't need to care about separate function sets, and PHP developers
> don't need to maintain separate sets of functions. If such functions get
> the decimal parameter, they return decimal. If they have more than one
> float parameter and mixed float and decimal passed, decimals converted to
> float by default. If "default_decimal" or "as_decimal" used, float is
> converted to decimal with the warning.
>

Messing with the return value type has already proved controversial.
And as I said previously, I am dead against having implicit float to
decimal conversions.
Making the functions type generic is something that I am fine, however.


> The new type uses libmpdec internally to perform decimal calculations
> (same
> as Python).
>

I really don't think we need arbitrary precision decimals.
I'm also not convinced using a floating point spec is the most sensible,
due to the rounding errors this is going to introduce.
The non-arbitrary IEEE 754-2008 specification cannot describe the decimal
123457.1467 exactly, which is frankly pointless.

Decimals, or more broadly rational numbers that, outside numerical
computations, that are going to be used are not going to need huge
denominators.

I've been thinking about this for a while, and I personally think that
rational numbers are just better than trying to support only decimals.
And considering we have 64 bits to play with for a new zval type splitting
the bits into a 32-bit unsigned integer for the numerator and an 32-bit
signed integer for the denominator should provide us with enough reasonable
rational numbers.
As any number that do not fit in this structure seems to be something
relegated to the world of numerical computation where floats are what are
mostly used anyway.


> All of the points above are subject to discussions, it is not an RFC
> candidate right now. So please share your opinions.
>
> I know that the implementation of this will require a lot of work. But I
> don't think this is a stopper from formulating the requirements.
> Sometimes,
> any project requires big changes to move forward. I'm pretty sure this
> 

Re: [PHP-DEV] Inconsistency mbstring functions

2023-12-01 Thread G. P. B.
On Fri, 1 Dec 2023 at 09:31, Stefan Schiller via internals <
internals@lists.php.net> wrote:

> Hi,
>
> I would like to raise attention to an inconsistency in how mbstring
> functions handle invalid multibyte sequences. When, for example,
> mb_strpos encounters a UTF-8 leading byte, it tries to parse the
> following continuation bytes until the full byte sequence is read. If
> an invalid byte is encountered, all previously read bytes are
> considered one character, and the parsing is started over again at the
> invalid byte. Let's consider the following example:
>
> mb_strpos("\xf0\x9fABCD", "B"); // int(2)
>
> The leading byte 0xf0 initiates a 4-byte UTF-8 sequence. The following
> byte (0x9f) is a valid continuation byte. The next byte (0x41) is not
> a valid continuation byte. Thus, 0xf0 and 0x9f are considered one
> character, and 0x41 is regarded as another character. Accordingly, the
> resulting index of "B" is 2.
>
> On the other hand, mb_substr, for example, simply skips over
> continuation bytes when encountering a leading byte. Let's consider
> the following example, which uses mb_substr to cut the first two
> characters from the string used in the previous example:
>
> mb_substr("\xf0\x9fABCD", 2); // string(1) "D"
>
> Again, the leading byte 0xf0 initiates a 4-byte UTF-8 sequence. This
> time, mb_substr just skips over the next three bytes and considers all
> 4 bytes one character. Next, it continues to process at byte 0x43
> ("C"), which is regarded as another character. Thus, the resulting
> string is "D".
>
> This inconsistency in handling invalid multibyte sequences not only
> exists between different functions but also affects single functions.
> Let's consider the following example, which uses mb_strstr to
> determine the first occurrence of the string "B" in the same string:
>
> mb_strstr("\xf0\x9fABCD", "B"); // string(1) "D"
>
> The principle is the same, just in a single function call.
>
> This inconsistency may not only lead to an unexpected behavior but can
> also have a security impact when the affected functions are used to
> filter input.
>
>
> Best Regards,
> Stefan Schiller
>
> [1]: https://www.php.net/manual/en/function.mb-strpos.php
> [2]: https://www.php.net/manual/de/function.mb-substr.php
> [3]: https://www.php.net/manual/de/function.mb-strstr.php
>

This might have been better to raise as a bug, but in any case I am CCing
Alex who's the main maintainer of the mbstring extension so he's aware of
this and can possibly provide some explanations.

Best regards,

Gina P. Banyard


Re: [PHP-DEV] [RFC] [VOTE] Add 4 new rounding modes to round() function

2023-12-01 Thread G. P. B.
On Fri, 1 Dec 2023 at 09:35, Jakub Zelenka  wrote:

> On Fri, Dec 1, 2023 at 9:28 AM G. P. B.  wrote:
>
>> On Thu, 30 Nov 2023 at 23:45, Jakub Zelenka  wrote:
>>
>>> Hi,
>>>
>>> On Thu, Nov 30, 2023 at 10:13 PM Jorg Sowa  wrote:
>>>
>>>>
>>>> Creating aliases for Intl extension constants has been accepted with 10
>>>> votes yes and 6 votes no.
>>>>
>>>>
>>> This means that it is declined as you need 2/3 of votes to pass...
>>>
>>
>> This seems to be a secondary vote which only requires 50% of votes to be
>> accepted.
>>
>>
> Ah I see now. It's a bit strange to have a secondary vote about sort of
> unrelated thing in different extension but no one objected before or during
> the vote so let's keep it accepted.
>

I think so, however, I do agree that this was unclear, and we probably
should make sure future RFCs explicitly state if a vote is primary or not
and what the approval rate for the vote to be accepted should be.

Best regards,

Gina P. Banyard


Re: [PHP-DEV] [RFC] [VOTE] Add 4 new rounding modes to round() function

2023-12-01 Thread G. P. B.
On Thu, 30 Nov 2023 at 23:45, Jakub Zelenka  wrote:

> Hi,
>
> On Thu, Nov 30, 2023 at 10:13 PM Jorg Sowa  wrote:
>
>>
>> Creating aliases for Intl extension constants has been accepted with 10
>> votes yes and 6 votes no.
>>
>>
> This means that it is declined as you need 2/3 of votes to pass...
>

This seems to be a secondary vote which only requires 50% of votes to be
accepted.

Best regards,

Gina P. Banyard


Re: [PHP-DEV] [RFC] [Discussion] Resource to object conversion

2023-11-22 Thread G. P. B.
On Wed, 22 Nov 2023 at 07:36, Mike Schinkel  wrote:

> On Nov 21, 2023 at 11:33 PM, > wrote:
>
> What is the point of a major release if we cannot even do such a BC break?
> We don't even know when PHP 9.0 is going to happen yet.
>
>
> I have been using Go for about four years now and it seems they have
> gotten the backward compatibility issue nailed, and that pays great
> dividends in developer confidence in the language, i.e.:
>
>
> https://www.reddit.com/r/golang/comments/17v4xja/anyone_face_issues_when_updating_version_of_go/
>
> They recently explained in depth how they do it:
>
> https://go.dev/blog/compat
>
> Also see:
>
> https://thenewstack.io/how-golang-evolves-without-breaking-programs/
>
> Although Go is compiled and PHP is not, I think there still may be
> significant insight that can be gained for PHP by studying how Go is
> handling it and applying any lessons learned.
>

Go is a "new" programming language, with its 1.0.0 version being from 2012.
It was also designed from the ground up.

PHP on the other hand wasn't designed, but the language grew organically,
and is 28 years old.
Comparing it to Go, in my opinion, makes no sense.

We should be comparing ourselves to languages of that age or older, the
most famous example being Python, which did a major BC break between its
version 2 and 3.
But Fortran, C, Perl (with Raku), and for sure others have all made changes
to the language, recent or not, that break compatibility.

Go even has a cave out that they *may* release a Go 2 specification, which
does not guarantee any backwards compatibility with Go 1. [1]
Even if the current lead engineer says this is "never" going to happen, the
cave out still exists.

More importantly, it is possible to write cross compatible code, even
without changing anything about is_resource(), if we convert streams to
opaque objects.
It might be tedious and one might need to have redundant instanceof checks
with is_resource() if one does not want to check for a false return, or
duplicate checks for closed resources.
But it is possible, which was *not* the case for Python 2 and 3 as it
changed fundamentally how strings behaved.

Finally, I think people would have more confidence in the language if it
stopped coming with various foot guns included that need to be explicitly
kept in check by using external tools such as static analysis tools, or
code style tools.
And removing those, or making the language overall more coherent and
consistent, requires us to break backwards compatibility.

Sincerely,

Gina P. Banyard

[1]  https://go.dev/doc/go1compat:

> It is intended that programs written to the Go 1 specification will
> continue to compile and run correctly, unchanged, over the lifetime of that
> specification. At some indefinite point, a Go 2 specification may arise,
> but until that time, Go programs that work today should continue to work
> even as future "point" releases of Go 1 arise (Go 1.1, Go 1.2, etc.).


Re: [PHP-DEV] [RFC] [Discussion] Resource to object conversion

2023-11-21 Thread G. P. B.
On Thu, 16 Nov 2023 at 16:13, Jakub Zelenka  wrote:

> On Sun, Nov 12, 2023 at 4:48 PM Máté Kocsis 
> wrote:
>
> > Hi Internals,
> >
> > Following my straw poll about the Process resource name, I would like to
> > present an RFC which clarifies the rough timeline and the BC promises of
> > the "resource to object conversion" project.
> >
> > Link: https://wiki.php.net/rfc/resource_to_object_conversion
> >
> >
> Please could you add a separate vote for primary streams if the resource to
> object conversion should be done at all (requiring 2/3 votes to be
> accepted). I will personally vote against this if there is no is_resource
> change as I think it's just too big BC break even for 9.0 - it will likely
> require massive update of many code bases.
>

What is the point of a major release if we cannot even do such a BC break?
We don't even know when PHP 9.0 is going to happen yet.

I will also state that I am against changing the semantics of is_resource()
*unless* we remove support for resources altogether from the engine at the
same time so that there is no possible ambiguity.
Which frankly is probably something we should be doing, and that people
paid by the foundation should convert resources to opaque objects in PECL
extensions, and other extensions brought to our attention.
And yes, that also means helping Oracle with OCI8 that is getting unbundled.

I will try to get round to do ext/dba somewhat soon.

Best regards,

Gina P. Banyard


Re: [PHP-DEV] PHP-8.1 open for bug fixes UNTIL 25 Nov

2023-11-21 Thread G. P. B.
On Tue, 21 Nov 2023 at 21:10, Ben Ramsey  wrote:

> We’ve discussed this among release managers, and I’m updating this
> thread to clarify that I was mistaken about what the dates for supported
> versions mean. These are the dates up until we will accept patches to a
> version branch. So, for example, we will accept bug fix patches in the
> PHP-8.1 version branch until 25 November. This means there may be
> another release following the end-of-active-support date, if there are
> bug fixes in the version branch.
>
> The PHP-8.1 branch is open for bug fixes UNTIL 25 November. I’m very
> sorry for the confusion.
>
> Cheers,
> Ben
>

Does this mean any bug-fix merged into 8.2 already needs to be backported?
Or will 8.1 RMs do this?

Best regards,

Gina P. Banyard


[PHP-DEV] Request PHP 8.3 GA release push back

2023-11-20 Thread G. P. B.
Hello internals,

As far as I am aware, the 8.3.0 release is meant to be tagged tomorrow.

However, there are at least three known bugs in RC6:

- An ext/posix bug [1], which has a fix [2]
- Two ext/pgsql bugs around the new pipeline support:
  - A constant not being exposed [3]
  - The overall implementation having issues [4]

Sincerely,

Gina P. Banyard

[1] https://github.com/php/php-src/issues/12725
[2] https://github.com/php/php-src/pull/11774
[3] https://github.com/php/php-src/issues/9344
[4] https://github.com/php/php-src/pull/12731


Re: [PHP-DEV] [RFC] [VOTE] Add 4 new rounding modes to round() function

2023-11-15 Thread G. P. B.
On Wed, 15 Nov 2023 at 23:43, Levi Morrison  wrote:

> On Wed, Nov 15, 2023 at 2:26 PM Jorg Sowa  wrote:
> >
> > Hello internals!
> > I have just opened voting on the RFC to add 4 new rounding modes to
> round()
> > function.
> >
> > Voting will end November 30th, 00:00 GMT.
> >
> > Link:
> > https://wiki.php.net/rfc/new_rounding_modes_to_round_function
> >
> > I'm sorry for previous message, but completely missed the title of the
> > message.
> >
> > Kind regards,
> > Jorg
>
> I didn't pay attention to the discussion. Sorry.
>
> I object to changing the intl code to add aliases. The intl extension
> is a wrapper around ICU, and I don't think it's a wise choice to
> modify it because you think "ROUND_TOWARD_ZERO" is better than
> "ROUND_DOWN". I agree the name is better, but that's not really how
> extension maintenance should work. I definitely don't think we should
> then _deprecate_ the ROUND_DOWN/ROUND_UP constants in ICU, given that
> the extension itself hasn't deprecated them and is still maintained. I
> am surprised nobody pointed this out before.
>
> I don't have any objection to the rounding modes themselves.
>

Personally I am fine with adding aliases to ext/intl, it is not because the
extension is a wrapper that we can't enhance it to be somewhat consistent.
However, I do agree that deprecating the existing constants is a bad idea.
But the RFC vote, as far as I can understand, is not about deprecating it
in this release.

Best regards,

Gina P. Banyard


Re: [PHP-DEV] Set register_argc_argv to Off by default

2023-11-09 Thread G. P. B.
On Wed, 8 Nov 2023 at 16:59, Thomas Chauchefoin <
thomas.chauchef...@sonarsource.com> wrote:

> Thank you for your early feedback on this suggestion. Without rushing
> things, I assume that the next step for me would be to draft an RFC to
> formalize this change?
>

The change seems to be uncontroversial, so an RFC feels unnecessary here.
Just make a PR and I think we can merge this into master.

Best regards,

Gina P. Banyard


Re: [PHP-DEV] Set register_argc_argv to Off by default

2023-11-07 Thread G. P. B.
On Tue, 7 Nov 2023 at 10:33, Thomas Chauchefoin via internals <
internals@lists.php.net> wrote:

> Hey,
>
> I recently opened an issue on GitHub [1] to discuss setting
> register_argc_argv to Off by default for all SAPIs but cli, embed, and
> phpdbg. Ilija Tovilo suggested including this change in 8.4.0.
>
> Even though most downstream distributions already turn it off, that's
> not the case everywhere. For instance,  the official Docker image has
> it on [2]. Outside of performance reasons, this also has a security
> impact because it eases the exploitation of limited LFI bugs [3] and
> CLI tools stored under the web root [4].
>
> -Thomas
>
> [1]: https://github.com/php/php-src/issues/12344
> [2]: https://hub.docker.com/_/php
> [3]: https://www.youtube.com/watch?v=yq2rq50IMSQ
> [4]: https://github.com/advisories/GHSA-jm6m-4632-36hf
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>

This sounds sensible to me.

Best regards,

Gina/George P. Banyard


Re: [PHP-DEV] New RFC : empty() function

2023-10-31 Thread G. P. B.
On Tue, 31 Oct 2023 at 11:23, Alessandro Rosa 
wrote:

> Hi Marcos,
>
> thanks for feedback.
> Could you be clearer about your advice and gimme an example please?
>

I have edited the RFC content to add highlighting, hopefully you can now
use that to improve the RFC text.

However, I'm not very convinced at the moment that such a function is truly
useful.

Best regards,

Gina/George P. Banyard


Re: [PHP-DEV] Re: Basic Type Alias

2023-10-30 Thread G. P. B.
On Sun, 29 Oct 2023 at 20:36, Robert Landers 
wrote:

> My personal opinion is that file-based type aliases would be the best.
> It solves an immediate problem while introducing a problem most people
> will never have (needing to reuse the type aliases elsewhere). It
> allows them to be used in functions and classes, while also making
> code more readable but not opaque outside the file. For example, if
> there is a type called BigNumber that was an alias of
> GMP|int|float|string, outside the file, knowing the actual types in my
> IDE is more useful than "BigNumber" and doesn't require me to dig into
> the definition or even agree with the other file's definition of
> "BigNumber." I just need to know that I need to pass it one of those
> types from my own code. However, reading that file's code, I don't
> need to know the intimate details of the aliases to make it more
> readable.
>
> If we find ourselves constantly writing the same type aliases in a
> bunch of files, we only need to wait less than a year to implement
> "global" (as in, available once include'ed) in the very next version
> of PHP.
>

No, this is actually ridiculous. File based type alias are dumb and I will
vote against this.
My main motivation to work on function autoloading in the first place was
to create a mechanism so that adding a new type autoloader is very easy.
As I already find the semantics of one interface per file dumb, let's not
make it even more ridiculous of having a single line in a file, especially
as a library/project will likely want to declare more than one type.

An IDE should very much have no struggle to resolve what the type alias
actually resolves too, they already do this with classes and interfaces.
And needing to declare every alias in every single file of a project is a
ludicrous idea.

Best,

Gina/George P. Banyard


Re: [PHP-DEV] Custom object equality

2023-10-25 Thread G. P. B.
I am just going to put this out there, but I will vote against any RFC
which provides access to userland to overload == and <=> until the base
semantics of PHP comparisons are fixed and the necessary engine
prerequisite work is done.

I am working on such an RFC, as I frankly do not trust people to think
stuff through and handle things like polymorphic comparisons.

Also, introducing some new kind of weird operator is just... bad.

Best regards,

George P. Banyard


Re: [PHP-DEV] Peculiar behaviour of ext/xml set_X_handler() functions

2023-10-16 Thread G. P. B.
On Tue, 10 Oct 2023 at 14:52, G. P. B.  wrote:

> Hello internals,
>
> While doing some refactoring of ext/xml to bring it up to modern engine
> standards, I discovered some peculiar behaviour of the functions which set
> callable handlers.
>
> The PR in question is: https://github.com/php/php-src/pull/12340
>
> The issue is that this function doesn't just accept the usual callabes,
> but also passing a method name that should be called on an object provided
> by xml_set_object().
>
> Moreover, contrary to the usual semantics of checking if the value is
> callable when passed to the function, it is checked when attempting to call
> it.
> The PR changes this to validate that the function or method name provided
> is actually callable.
> This requires a BC break in that xml_set_object() MUST be called prior to
> setting the method names, otherwise a ValueError is now thrown.
>
> The only project I could find that called xml_set_object() after already
> setting method names for handlers is semsol/arc2
> <https://github.com/semsol/arc2> to which I have sent a PR to update this.
>
> The existence of the already set methods is now also checked when calling
> xml_set_object() again with a different object to ensure that the handlers
> are always well-defined.
>
> The plan is to also deprecate this feature out right, but this can be done
> by adding it to the mass 8.4 deprecation RFC. [1]
>
> Point of this email is to know if anyone has any complaints about this BC
> break before merging the PR into master.
>
> Best regards,
>
> George P. Banyard
>
> [1] https://wiki.php.net/rfc/deprecations_php_8_4
>

If no one complains I will merge the PR at the end of the week.

Best regards,

George P. Banyard


Re: [PHP-DEV] Better name for method Randomizer::nextFloat()

2023-10-16 Thread G. P. B.
On Mon, 16 Oct 2023 at 09:34, Christian Schneider 
wrote:

> > Side note: The implementation of nextFloat() is much more efficient than
> that of getFloat(0.0, 1.0) and the output will differ even for the same
> seeded engine. The domain of legal return values is identical.
>
> This sounds like an implementation detail as I'm pretty sure you could
> switch to that fast path easily enough if neither of the arguments were
> given.
>

This is not how the engine works, it would require an if-clause on every
single call of getFloat() that would probably defeat the point of it.

Best regards,

George P. Banyard


[PHP-DEV] Peculiar behaviour of ext/xml set_X_handler() functions

2023-10-10 Thread G. P. B.
Hello internals,

While doing some refactoring of ext/xml to bring it up to modern engine
standards, I discovered some peculiar behaviour of the functions which set
callable handlers.

The PR in question is: https://github.com/php/php-src/pull/12340

The issue is that this function doesn't just accept the usual callabes,
but also passing a method name that should be called on an object provided
by xml_set_object().

Moreover, contrary to the usual semantics of checking if the value is
callable when passed to the function, it is checked when attempting to call
it.
The PR changes this to validate that the function or method name provided
is actually callable.
This requires a BC break in that xml_set_object() MUST be called prior to
setting the method names, otherwise a ValueError is now thrown.

The only project I could find that called xml_set_object() after already
setting method names for handlers is semsol/arc2
 to which I have sent a PR to update this.

The existence of the already set methods is now also checked when calling
xml_set_object() again with a different object to ensure that the handlers
are always well-defined.

The plan is to also deprecate this feature out right, but this can be done
by adding it to the mass 8.4 deprecation RFC. [1]

Point of this email is to know if anyone has any complaints about this BC
break before merging the PR into master.

Best regards,

George P. Banyard

[1] https://wiki.php.net/rfc/deprecations_php_8_4


Re: [PHP-DEV] Can we deprecate socket_set_blocking() ?

2023-10-09 Thread G. P. B.
On Mon, 9 Oct 2023 at 18:39, Hans Henrik Bergan 
wrote:

> The only thing socket_set_blocking() is good for is to confuse people
> looking for socket_set_block()
>

Shouldn't we then also deprecate the socket_* alias functions?
But this can probably be added to
https://wiki.php.net/rfc/deprecations_php_8_4

Best regards,

George P. Banyard


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

2023-10-05 Thread G. P. B.
On Thu, 5 Oct 2023 at 07:40, Marc Bennewitz  wrote:

> I don't see a bug or broken behavior here as these functions were
> processing floating point numbers since  forever.
>
> https://3v4l.org/PrrmO
>
That's not my point, the point is about the function being broken for large
64bit integers, which your RFC is fixing.

I would also expect most people that actually use ceil/floor/round to dump
the return value of it into a property/value that is expected to be an int.
Which is also compatible for floats that fit in an int (and since 8.2 for
those that have a fractional part a warning is emitted).

I see no benefit in doing this weird approach instead of just changing the
behaviour.
Yes this is a theoretical BC break [1], but unless you can show me an
actual use case that breaks (and even then) I don't think the approach of
this RFC is good.

Best regards,

George P. Banyard

[1] And this is coming from me, who does love finding weird edge cases and
remove them from the language, just look at the 8.3 range() RFC


Re: [PHP-DEV] proc_open() resource to opaque object migration

2023-10-04 Thread G. P. B.
On Thu, 28 Sept 2023 at 21:20, Máté Kocsis  wrote:

> Hi Everyone,
>
> I'm writing in connection with a question coming up lately during the
> "resource to opaque object migration" project (
> https://github.com/php/php-tasks/issues/6) which we have been working on
> for quite a long while.
>
> During the review of my PR migrating the resource returned by proc_open()
> to an object (https://github.com/php/php-src/pull/12098), it quickly
> became
> evident that there was no consensus about the new class name, since the
> originally proposed "Process" name has a non-negligible BC break
> likelihood.
>
> That's why we should find the best class name in accordance with Nikita's
> namespace naming convention RFC (
> https://wiki.php.net/rfc/namespaces_in_bundled_extensions). Even though my
> PR currently implements "Standard\Process", this name is not a good
> candidate according to the policy:
>
> Because these extensions combine a lot of unrelated or only tangentially
> > related functionality, symbols should not be namespaced under the Core,
> > Standard or Spl namespaces. Instead, these extensions should be
> considered
> > as a collection of different components, and should be namespaced
> according
> > to these.
>
>
> Does anyone have a good suggestion?
>

If there needs to be a namespace, we could take inspiration from Python and
use OS\Process,
as everything relating to processes seem to reside in the os module.

Best regards,

George P. Banyard


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

2023-10-04 Thread G. P. B.
On Tue, 26 Sept 2023 at 11:39, Marc Bennewitz  wrote:

> Hi internals
>
> I'd like to put a new RFC under discussion:
> https://wiki.php.net/rfc/integer-rounding


I don't understand the point of the deprecation phase at all.
Frankly, I consider this RFC a bug fix of the current "broken"
implementation, which was broken with 7.0 and the move to 64bit integers.
As such, I would just make the behaviour of returning integers the default
behaviour in 8.4.
If, for some weird reason, people want less accurate numbers, they can
always cast the input to float.

Best regards,

George P. Banyard


Re: [PHP-DEV] Re: [RFC] [Discussion] Add 4 new rounding modes to round() function

2023-10-04 Thread G. P. B.
On Wed, 4 Oct 2023 at 23:23, Jorg Sowa  wrote:

> Thank you all for the discussion. I modified description of the RFC and
> added deprecation of constants ROUND_UP and ROUND_DOWN from Intl extension
> as an additional voting.
>
> Thanks also for input about possible mode ROUND_STOCHASTIC, however I think
> it's the idea for the extension and not the addition to the standard
> functions.
>
> I would like to start the voting in coming days.
>

I don't think deprecating the constants in the version where the aliases
are introduced is a good idea.
There needs to be at least one version where both variants exist to allow
for cross version compatibility.

I will also echo Tim's idea of using an enum instead and a union type.
For the namespace, I think it would make sense to use Maths\RoundingMode

Best regards,

George P. Banyard


Re: [PHP-DEV] Add bcround, bcfloor, bcceil to BCMath.

2023-09-29 Thread G. P. B.
On Tue, 26 Sept 2023 at 11:10, Saki Takamachi  wrote:

> Hi, internals.
>
> I'm currently working on renovating the `round()` function.
>
> The current `round()` function rounds a value like 0.285 (0.28499) to
> 0.29 using pre-round.
> This is a mistake for FP, so discussions are underway to abolish such
> pre-round behavior.
>
> However, there is definitely a user request to round 0.285 to 0.29, so I
> thought why not add a function similar to the round function to BCMath.
>
> Similarly, I'm considering adding similar functionality to `floor()` and
> `ceil()`. In rare cases, when dealing with very large values, these will
> return a different value than expected as a decimal number.
>
> I would love to hear everyone's opinions.
>

I think regardless of the issues around round(), ceil(), floor(), etc.
having the equivalent functions for BCMath makes a lot of sense to me.

Best regards,

George P. Banyard


Re: [PHP-DEV] Deprecate PDO::ATTR_ERRMODE [Proposed RFC]

2023-09-06 Thread G. P. B.
On Tue, 29 Aug 2023 at 14:52, Saki Takamachi  wrote:

> Hi, internals.
>
> I thought about various things to improve the current situation where
> `PDO::ATTR_ERRMODE` is not working very smartly.
>
> Exceptions may be thrown regardless of the setting of `PDO::ATTR_ERRMODE`.
> https://www.php.net/manual/en/pdo.rollback.php#refsect1-pdo.rollback-errors
>
> Another annoyance is that `PDO::ERRMODE_SILENT` sometimes gives a warning.
> This is an undocumented phenomenon, and it's a 19-year-old vintage bug.
>
> https://github.com/php/php-src/blob/223fb08819967b3063610289a5783944a85d6d65/ext/pdo/pdo_dbh.c#L74
>
> Based on these, I feel that the reliability of the attribute value
> PDO::ATTR_ERRMODE is low and there is not much meaning in its existence as
> an attribute value.
>
> Since the default behavior became `PDO::ERRMODE_EXCEPTION` in PHP8.0.0, I
> think it would be better to abolish it rather than leave it halfway.
>
> I think this is a big change, so I'm assuming 9.x+ even if it's
> implemented.
> I will do the implementation myself.
>
> Please let me know what you think.
> Thank you.
>
> Saki
>

I don't know about removing the silent mode altogether, but I think the
warning version is pretty useless.
However, if we are going to remove it, it would be nice to have this done
consistently across all bundled extensions.
>From the top of my head, I know SQLite3 and the Intl extension also have
such flags, and possibly the DOM extension too.

Best regards.

George P. Banyard


Re: [PHP-DEV] Removing support for the disable_classes INI setting

2023-08-14 Thread G. P. B.
On Mon, 14 Aug 2023 at 20:03, Larry Garfield  wrote:

> However, I think it should not get an exception for code-freeze.  At best,
> I could see a deprecation warning on it in 8.3 and remove in 8.4/9, but I
> defer to the RMs on that front.
>

In its current state, this INI setting does nothing other than cause
segfaults.
This functionality is broken, and frankly has always been.
The only reason I am asking for an exception is to remove broken
functionality that doesn't work and cannot be easily fixed.

George P. Banyard


[PHP-DEV] Removing support for the disable_classes INI setting

2023-08-14 Thread G. P. B.
Hello internals,

While working on some DNF type bugs, I discovered some major issues around
the disable_classes INI setting implementation. Such as:

- A double-free of the type of a typed property
- A use after free (which segfaults) when trying to access a property
defined on a disabled class that was extended (e.g. disabling Exception)
- A use after free when var_dumping a disabled class that has its own
handler (as it will assume properties to be allocated)
- And likely more considering the lack of tests surrounding this feature

This feature seems of dubious nature, and the only justification given when
adding support for this in the changelog of PHP 4.3.2 is:

New "disable_classes" php.ini option to allow administrators to disable
certain classes for security reasons. [1]

However, only classes defined by extensions can be disabled, and such a
class is critical for the correct operation of said extension.
As such, what one should do for security reasons is to not enable the
extension in the first place.

Moreover, compared to the behaviour of disable_functions which, as of PHP
8.0, removes the function declaration completely, disable_classes does not
remove the declaration of the class, but just "overloads" the object
creation process to not initialize the object and emit a warning.
Meaning, it is totally valid to instantiate a disabled class and pass it
around to functions for them to blow up when they try to use the object as
intended.

Considering the major flaws in the implementation of said feature, the
dubious nature of it, and the seeming lack of usage of it (considering none
of the above breaking bugs have been reported).
I would like to remove this feature in PHP 8.3, even though I know we are
past feature freeze and close to the first RC.

I have CCed the RMs for 8.3, and would like the opinion of other people on
if this removal makes sense to the majority of people

Sincerely,

George P. Banyard

[1] https://externals.io/message/2076


Re: [PHP-DEV] New reflection methods for working with attributes

2023-07-25 Thread G. P. B.
On Tue, 25 Jul 2023 at 21:55, Robin Chalas  wrote:

> FY  I started working on the implementation., hopefully it will bring more
> arguments in favor of that RFC which I’m willing to submit asap as well.
>

For future reference, please just open a PR with the implementation before
writing an RFC for those kinds of things. As self-contained, small features
do not necessarily require them.

Best regards,

George P. Banyard

PS: Side note, please do not bottom post.


Re: [PHP-DEV] New reflection methods for working with attributes

2023-07-25 Thread G. P. B.
On Tue, 25 Jul 2023 at 21:24, David Gebler  wrote:

> What's the difference between this and what was proposed in
> https://externals.io/message/120799 ? I don't get why this wouldn't
> require
> an RFC.
>

Because it frankly doesn't require an RFC.
I think people are getting confused as to when one is required.

Adding methods for completeness that should just be there and no-one
complains can just get added.
The email is a courtesy to know if some people are going to bikeshed.
Now, adding an interface is more of a change that probably warrants one,
but even then it is fuzzy.

Best,

George P. Banyard


Re: [PHP-DEV] [RFC] [VOTE] Deprecate remains of string evaluated code assertions

2023-07-12 Thread G. P. B.
On Wed, 28 Jun 2023 at 17:09, G. P. B.  wrote:

> Hello internals,
>
> I'm opening the vote for the
> Deprecate remains of string evaluated code assertions RFC:
> https://wiki.php.net/rfc/assert-string-eval-cleanup
>
> It will last for two weeks and end on Wednesday the 12th of July
>

The vote has now ended.
The RFC has been unanimously accepted, with 24 votes in favour.

Thank you for everyone that has voted.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] [Vote] Path to Saner Increment/Decrement operators

2023-07-12 Thread G. P. B.
On Wed, 28 Jun 2023 at 17:07, G. P. B.  wrote:

> Hello internals,
>
> I'm opening the vote for the Path to Saner Increment/Decrement operators
> RFC:
> https://wiki.php.net/rfc/saner-inc-dec-operators
>
> It will last for two weeks and end on Wednesday the 12th of July
>

The vote has now ended.

The RFC was accepted unanimously, with 25 votes in favour.

Thank you to everyone who has voted.

Best regards,

George P. Banyard


Re: [PHP-DEV] [VOTE] Interface Default Methods

2023-07-11 Thread G. P. B.
On Mon, 3 Jul 2023 at 01:11, Levi Morrison  wrote:

> Chatter on the [Interface Default Methods RFC][1] has been quiet for
> the past 6 days, and the feature freeze deadline is fast approaching
> for PHP 8.3, so I'm moving this to vote. It'll be open for two weeks
> as usual.
>
> Thanks to everyone who discussed weaknesses in the RFC during the
> discussion phase.
>
>   [1]: https://wiki.php.net/rfc/interface-default-methods
>

Although I like the idea, I think the main reason for the pushback is how
close this is being voted on before feature freeze when the RFC +
implementation was updated very recently before.
And personally I think, considering this, I would also delay implementing
this.
We have at least 2 other major RFCs that are being pushed back (Property
Hooks and Function autoloading) due to time constraints.

Maybe it's time for a more meta discussion about the absurdly long release
process PHP has of releasing many intermediate versions that seem to get no
testing from userland.

For everyone against this feature, I would urge you to understand the
distinction between "type classes" and Java-like "interfaces" (which is
effectively what PHP interfaces are).
A good article is the following one:
https://diogocastro.com/blog/2018/06/17/typeclasses-in-perspective/

I also find it baffling the lack of understanding around this feature.
Generic programming exists, and it operates on a level where a method can
have a "concrete" default representation and still represent the genericity
to its fullest.
Examples have already been given, such as the Comparable, Equatable, or
Iterator type classes.
Considering, Haskell, Rust, Scala, and modern PL theory sees no issue with
that, I struggle to understand the resistance here.

Moreover, saying this capability should not be added to PHP because we have
inferior tools X + Y + Z to achieve what this does is counterproductive
and, IMHO, shows a clear lack of understanding of the issues PHP's model
currently has.
Traits are, and have always been, terribly designed. Assisted copy-paste
that has no relation to the type the implementation it provides fulfils.
Abstract classes allowing default implementations, but enforcing hierarchy
thus type relations, meaning they are unsuitable for when one wants
genericity (for separating concerns).
Interfaces which allow the creation of new types, but no generic
implementation which depend on a function specified by said interface.

Objectively, the current situation is suboptimal.
Maybe the resistance to the proposal would be far less if the RFC, and
implementation, would check at compile time that the default
implementations only rely on known existing functions available to the
interface.
Or that at least one of the methods would remain "abstract".
However, that does not seem what the discourse is.

I find the Rust Seek trait(/type class) a very concise example of the
usefulness, and where the implementation of a default method is far from
concrete.
Requiring an implementation to provide the implementation for seek(int
$position)
But being able to provide the trivial implementation of rewind() which is
seek(0);

Sincerely,

George P. Banyard


[PHP-DEV] [RFC] [VOTE] Deprecate remains of string evaluated code assertions

2023-06-28 Thread G. P. B.
Hello internals,

I'm opening the vote for the
Deprecate remains of string evaluated code assertions RFC:
https://wiki.php.net/rfc/assert-string-eval-cleanup

It will last for two weeks and end on Wednesday the 12th of July

Best regards,

George P. Banyard


[PHP-DEV] [RFC] [Vote] Path to Saner Increment/Decrement operators

2023-06-28 Thread G. P. B.
Hello internals,

I'm opening the vote for the Path to Saner Increment/Decrement operators
RFC:
https://wiki.php.net/rfc/saner-inc-dec-operators

It will last for two weeks and end on Wednesday the 12th of July

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] Deprecate remains of string evaluated code assertions

2023-06-27 Thread G. P. B.
I am going to group the response from the 3 emails.

On Tue, 27 Jun 2023 at 09:17, Claude Pache  wrote:

> Although the specific comment in the php.ini file talks about runtime
> switching, it is in fact irrelevant whether it is set at runtime or not.
> More concretely; I use currently the settings:
>
> * `zend.assertion=1` as set in the global php.ini;
> * `assert.active = on` or `off` in my per-directory .user.ini or .htaccess
> file, depending on whether I want to enable assertions by default;
> * optionally  `ini_set('assert.active', '1')` at runtime when I want to
> enable debug mode temporarily.
>
> I have established the above settings several years ago (at the time of
> PHP 7), after having carefully read the documentation (and having been
> slightly confused by the redundancy between `assert.active` and
> `zend.assertion`). I might have end up to switch between `zend.assertion =
> 0/1` instead of switching between `assert.active = on/off` instead; I can’t
> say exactly why I chose the former option instead of the latter, but I
> guess that both the comment in the `php.ini` file, and the existence of
> `assert_options(ASSERT_ACTIVE, ...)` as an alternative to
> `ini_set('assert.active', ...)`, have influenced my choice.
>
> Note also that the above settings minus `zend.assertion` was the way to do
> it in PHP 5 (again it is irrelevant whether it is at runtime or not), and
> many people that used assertions in PHP 5 may have continued to use that
> way without modification, as there is currently no strong reason to change
> (only `zend.assertions=-1` is relevant if you are especially concerned
> about micro-optimisation).
>
> Now, of course, people will adapt to the new settings. But given the
> confusing state of the options (both `zend.assertion` and `assert.active`
> must be enabled, and I am not even speaking about `assert.exception`), you
> ought to be super-clear (in the deprecation notice, in the php.ini file, in
> the docs), what is the the expected state: paradoxically leave
> `assert.active` and `assert.exceptions` enabled (the default value) even
> when you want to disable assertions, and playing exclusively with
> `zend.assertion`.
>

I would argue that zend.assertions=-1 is more than a micro-optimisation,
but that not the point.
Yes, you would need to switch to toggle zend.assertions between 1 and 0 if
you would want to enable/disable assertions at run-time.
However, considering that code should behave the same even when assertions
are not compiled (with the -1 value) I don't really see the point of
toggling between both modes.

The expected state is to leave all the assert.* INI settings to their
default value and only use the zend.assertions INI setting.

The state of the assert() docs were in very bad shape, and I hope that my
recent changes to it has improved it and made them clearer.
However, I will make sure that the path forward is very clear be that in
the INI setting docs, the assert() docs, the assert_options() docs, the
migratrion guide, and in the deprecation message.


On Tue, 27 Jun 2023 at 09:37, Claude Pache  wrote:

> I don’t see the RFC listed under https://wiki.php.net/rfc#under_discussion
> .
>

Fixed.


> The RFC is imprecise in what is meant by “deprecating”. I guess that a
> deprecation notice (E_DEPRECATED) will be triggered at least under the
> following conditions:
>
> * at startup when one of the assert.* setting has not the default value;
> * at runtime when `assert_options(...)` is used;
> * at runtime when `ini_set(...)` is used to set an `assert.*` option to a
> non-default value?
>
> It is unclear to me what will happen when:
>
> * `ini_set(...)` is used to set an `assert.*` option to its default value,
> either as a no-op or not?
>

Clarified.


> Moreover, by removing `assert.callback` (and other options), are you
> effectively removing a feature? (I don’t really know, I have never
> considered it even in my worst dreams.) If so, it should be noted in a
> “Backward Incompatible Changes”.
>

Yes, I don't really see the point of such a section as a deprecation, and
thus removal is clearly a BC Break.
I have tried to improve the wording to convey this more clearly.

On Tue, 27 Jun 2023 at 09:47, Claude Pache  wrote:

> changing the return value of `assert(...)` from `bool` (true) to `void` is
> also a BC break, (and it is unclear to me what is the effective advantage
> of the break).
>

Considering that any failed assertion is going to abort execution, having
it return a value is pointless.
Moreover, one must not rely on the expression being asserted to be executed
(thus it should have no side effects) and changing it to void very clearly
conveys this meaning that one must not store the result of the assert() as
it is not a totally normal function call.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] Path to Saner Increment/Decrement operators

2023-06-26 Thread G. P. B.
On Tue, 17 Jan 2023 at 14:28, G. P. B.  wrote:

> Hello Internals,
>
> I would like to start the discussion on the Path to Saner
> Increment/Decrement operators RFC:
> https://wiki.php.net/rfc/saner-inc-dec-operators
>
> The goal of this RFC is to reduce language complexity by making $v++
> behave like $v += 1 and $v-- behave like $v -= 1;
>
> I am expecting the contentious part of the proposal to be the deprecation
> of the PERL string increment feature to achieve the aforementioned goal.
> However, I believe the benefits of aligning the behaviour of the
> increment/decrement operators with addition/subtraction are larger than
> keeping support for the PERL increment, which in its current state has
> various shortcomings.
>

I have added a section about impact on the PERL increment deprecation to
the RFC.
https://wiki.php.net/rfc/saner-inc-dec-operators

I am also planning on opening the vote on this on Wednesday the 28th of
June.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] Deprecate remains of string evaluated code assertions

2023-06-26 Thread G. P. B.
On Wed, 31 May 2023 at 13:08, G. P. B.  wrote:

> Hello internals,
>
> I would like to start the discussion about deprecating various remains
> from the now removed string code evaluated assertions functionality of
> assert().
>
> The RFC is located on the wiki at the following address:
> https://wiki.php.net/rfc/assert-string-eval-cleanup
>
> Initially, this was part of the mass PHP 8.3 deprecation RFC, but only the
> assert_options() function was part of it.
>

Head's up, I'm planning on opening the vote on this on Wednesday the 28th
of June.

Best regards,

George P. Banyard

>


Re: [PHP-DEV] [RFC] Deprecate remains of string evaluated code assertions

2023-06-26 Thread G. P. B.
On Wed, 31 May 2023 at 23:20, Claude Pache  wrote:

> Although your RFC says that the `zend.assertions` ini setting has
> superseded `assert.active` for a while, the “official” php.ini file still
> advises to modify the value of `assert.active` rather than the one of
> `zend.assertion` in order to switch behaviour at runtime:
>
>
> https://github.com/php/php-src/blob/91fd5641cde138b8894a48c921929b6e4abd5c97/php.ini-development#L1604
>
> I bet that many people (myself included) follows the advice given in
> `php.ini`.
>

This talks about run-time modification, which is something that I don't
think should be done in practice, nor is often done.


> Another point: Your RFC is missing `assert.quiet_eval`...
>

I have clarified that this was indeed removed in PHP 8.0.
https://wiki.php.net/rfc/assert-string-eval-cleanup

Best regards,


Re: [PHP-DEV] [RFC] [VOTE] Define proper semantics for range() function

2023-06-15 Thread G. P. B.
On Thu, 1 Jun 2023 at 17:16, G. P. B.  wrote:

> Hello internals,
>
> The vote for the Define proper semantics for range() function RFC is now
> open and will last for two weeks until the 15th of June 2023:
>
> https://wiki.php.net/rfc/proper-range-semantics
>

The vote has now ended with 20 votes in favour and 0 against.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] [VOTE] Define proper semantics for range() function

2023-06-08 Thread G. P. B.
On Thu, 8 Jun 2023 at 07:35, Marc  wrote:

> sorry for speaking up so late but I think the following change is a
> mistake and unexpected:
>
>  > Calls to range() that have integer boundaries but a float step that
> is compatible as an integer will now return an array of integers instead
> of an array of floats:
>
> var_dump( range(1, 5, 2.0) );
> /* New Behaviour */
> array(3) {
>[0]=>
>int(1)
>[1]=>
>int(3)
>[2]=>
>int(5)
> }
> /* Current Behaviour */
> array(3) {
>[0]=>
>float(1)
>[1]=>
>float(3)
>[2]=>
>float(5)
> }
>
> The problem I see with this is that the return type changes from float
> to int "magically" based on the value of the step.
> range(1, 5, 2.0) // list range(1, 5, 2.1) // list
>
> In my opinion, no matter, if start, end or step is a float the result
> should be a list.
>

A list is compatible and interpretable as a list so this only I
slight change in output and has effectively no BC break when using the
values generated by range().
Moreover, this change is needed to make calls such as range("a", "z", 3.0)
work as intended.
I.e. generating a list of characters instead of generating the list [0.0]
by attempting to cast the strings to floats.

Moreover, considering the return type is already pretty magical, supporting
this specific case would add even more complexity to the implementation,
and this has effectively no BC break, I will not change this detail.
More so that the RFC is currently in voting and cannot be amended.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

2023-06-05 Thread G. P. B.
On Tue, 30 May 2023 at 09:54, Claude Pache  wrote:

> Any change that introduce a BC break or a migration burden must be
> justified. (There has been enough rant around justified BC breaks, so let’s
> not introduce unjustified ones.)
>
> What is the purpose of changing the return convention of IntlCalendar
> methods, and is it worth?
>

We are talking about adding a new method here, so I don't really see how
this is a BC break.
One would hope that people do not just blindly change function names,
especially considering that one would need to look at the migration guide,
where it can be explicitly noted that the return type is different from the
previous method.

Moreover, the return convention of IntlCalendar (and most other Intl
classes) has for the most par always been wrong.
Prior to PHP 8.0, passing invalid types to functions/methods was considered
undefined behaviour, and in general the value returned when a TypeError
wasn't thrown was NULL.
However, Intl (and some other extensions) didn't follow this and returned
false on ZPP errors and always true otherwise.
As such, the change to *always* throw TypeErrors on ZPP violations has
brought to light those sorts of extensions.
This was one of the main motivation to add the true singleton type other
than consistency, as this allows tools like static analysis, but not
limited to them, to detect dead code and bring this up to people's
attention.

As effectively, there is no point in storing and/or comparing the value of
a method/function that always returns the same value (be that null(/void),
true, false, or other).
Considering this fact, that one should not compare the return value of most
(if not all) of these methods, using void as the type seems very much
appropriate rather than using true.

Best regards,

George P. Banyard


[PHP-DEV] [RFC] [VOTE] Define proper semantics for range() function

2023-06-01 Thread G. P. B.
Hello internals,

The vote for the Define proper semantics for range() function RFC is now
open and will last for two weeks until the 15th of June 2023:

https://wiki.php.net/rfc/proper-range-semantics

Best regards,

George P. Banyard


[PHP-DEV] [RFC] Deprecate remains of string evaluated code assertions

2023-05-31 Thread G. P. B.
Hello internals,

I would like to start the discussion about deprecating various remains from
the now removed string code evaluated assertions functionality of assert().

The RFC is located on the wiki at the following address:
https://wiki.php.net/rfc/assert-string-eval-cleanup

Initially, this was part of the mass PHP 8.3 deprecation RFC, but only the
assert_options() function was part of it.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] Define proper semantics for range() function

2023-05-31 Thread G. P. B.
Hello Internals,

Round 2, I'm planning to open voting tomorrow, the only change being is
handling of string digits to behave like doing an ASCII string range:
https://wiki.php.net/rfc/proper-range-semantics

Any final comments or complaints should be raised now.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] Define proper semantics for range() function

2023-05-16 Thread G. P. B.
On Tue, 16 May 2023 at 17:49, Jorg Sowa  wrote:

> Thank you. That makes sense. I have last question about case with integer
> and string digit, i.e. range('5', 10) or range('1', 9). What would be in
> this case expected output? I couldn't find any test cases covering this
> example.
>

I've added test cases, but the RFC already mentions that those numeric
strings would be interpreted as integers and thus generate a list of ints.

Best regards,

George P. Banyard

PS: Just an etiquette reminder to bottom post instead of top posting to not
carry around the previous bits of emails (see
https://github.com/Danack/RfcCodex/blob/master/etiquette/mailing_list.md#why-should-i-place-my-response-below-the-quoted-text
)


Re: [PHP-DEV] [RFC] Define proper semantics for range() function

2023-05-16 Thread G. P. B.
On Mon, 15 May 2023 at 23:53, Jorg Sowa  wrote:

> Hello!
>
> I have one concern about the part:
>
> > Emit an E_WARNING when $start or $end is cast to an integer because the
> other boundary input is a number or numeric string. (e.g. range
> ('5', 'z'); or range (
> 5, 'z');)
>
> Doesn't it limit the functionality of the function for the numbers as
> characters? Currently when we call range('/','z') we get full range of
> characters. https://onlinephp.io/c/9cb12
>
> But when we change argument $start to next character which is zero ('0')
> then we get array with only one element. https://onlinephp.io/c/a0cda
>
> Casting numerical string in this function may be confusing.
>
> Sorry for making fuss just before voting, but didn't see this topic before
> and wanted to share my insights with you thinking it may be relevant.
>

No worries, this is the point of giving a heads-up.
Someone else brought this to my attention again as well.
And the concern makes sense, I've updated the implementation and RFC to
adjust the behaviour with string digits:
https://wiki.php.net/rfc/proper-range-semantics

Please let me know if this addresses the issue and is also clear.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

2023-05-16 Thread G. P. B.
On Mon, 15 May 2023 at 19:39, Larry Garfield  wrote:

> Tangent: If I were to put together an RFC that set out such a 5 year cycle
> expectation with reasonable guidelines around when things could be
> deprecated, would anyone actually support it?
>

No, as this doesn't solve the problem.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

2023-05-16 Thread G. P. B.
On Mon, 15 May 2023 at 19:36, Rowan Tommins  wrote:

> On 15 May 2023 09:54:41 BST, "G. P. B."  wrote:
>
> >Why are we assuming that PHP 9.0 is going to come after PHP 8.4?
>
> Historically, PHP has had a major release roughly every five years. The
> main exception is PHP 6, which was never released - but whose major
> features became PHP 5.3, five years after 5.0, and six before 7.0
>
> I think planning a rough timeline is more useful to users and contributors
> than waiting until there's some exciting headline feature. Otherwise, it
> becomes tempting to sneak in breaking changes in 8.x because "we don't know
> how soon 9.0 is", or to have a rush of changes because "we've only just
> decided 9.0 is soon".
>
> It also helps avoid putting a release number on an experimental feature
> that might never arrive, as with Unicode strings in 6.0; or that might turn
> out to be less important to most users than other changes, like the JIT in
> 8.0.
>

I totally agree that we shouldn't use a major number as an exciting feature
flag.
However, if what we want is a consistent timeline for how long something
gets deprecated before being removed, then let's just do like Python and
state that the deprecation exists for two consecutive releases and will be
removed in the third one.

Because this proposal of "let's do majors every 5 years" now reduces what
we can do, as we cannot (or should not) deprecate stuff in X.0, as it makes
it harder for people to upgrade to the new major (see how we postponed the
8.0 deprecation RFC to 8.1 after various comments).
But apparently we cannot deprecate anything in X.3 or X.4 because there
will "never" be a X.5 again and this is "too short" of a time frame.
This, IMHO, is terrible for the project's maintenance. As this basically
says to someone who wants to start contributing to the project by
deprecating some weird behaviour that they may be told off with "you should
have done this X years ago" or "wait another Y years".
People cannot magically know when they should shove aside any other
priorities in life just to be able to make *some* change to the language.
Considering, this was partially the argument used against Max Kellerman
with the header changes: "this should have been done for PHP 7.0 or PHP
8.0".
Those sorts of statements alienate new (and old) contributors, and looking
at the state of the projects where we are still very much thin on
maintainers, anything that does this is from my POV a no-go.

My understanding as to *why* the JIT caused the major bump was because of
major changes within the Zend Engine, not necessarily the feature in itself.
And again, the only "good" reason I see for a major bump is something like
converting streams from resources to objects, as this is a massive shift
and has the possibility to cause issues for a lot of codebases.

Best regards,

George P. Banyard


Re: [PHP-DEV] [VOTE] Use exceptions by default in SQLite3 extension

2023-05-15 Thread G. P. B.
On Mon, 15 May 2023 at 15:41, someniatko  wrote:

> The `PDOException` class is actually extending the `RuntimeException`,
> not just `Exception`.
>

Yes, and this is, IMHO, a mistake.
Outside SPL, there are only 4 extensions which extend from RuntimeException:
 - PDO
 - MySQLi
 - SNMP
 - Phar

See:
https://heap.space/search?project=php-src===spl_ce_RuntimeException===

The vast majority of extensions that define their own custom exceptions
extend Exceptions.
The reason being that depending on a separate extension (here SPL) instead
of core API is less than ideal.

Moreover, SPL exceptions are meant for userland.
And I personally find it extremely strange that if I attempt to catch a
RuntimeException I might as well catch a PDO failure.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] Define proper semantics for range() function

2023-05-15 Thread G. P. B.
Hello Internals,

I plan to put the RFC to a vote tomorrow in its current state, which has
not been changed since the 30th of March:
https://wiki.php.net/rfc/proper-range-semantics

Any final comments or complaints should be raised now.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

2023-05-15 Thread G. P. B.
On Sat, 13 May 2023 at 00:08, Larry Garfield  wrote:

> That means it's impossible to write code that works from 8.2 to 9.0
> without version checks.  The overlap period is only 2 releases (8.3 and
> 8.4).  That's too short of a window.
>

Why are we assuming that PHP 9.0 is going to come after PHP 8.4?
There has been no decision as to when the new major is going to be released.
And as far as I'm concerned, the only good reason to move from 8.x to 9.0
is when we convert streams from resources to objects, which is going to be
a large enough BC that those sorts of minor BC breaks should frankly be a
non-issue.
So could we not speculate on an arbitrary timeline of the new major version
being tagged and go on the merit of this proposal in isolation?

I don't see much of an issue of, introduce the new functions/methods in PHP
8.3, deprecate them in PHP 8.3+x (for x > 0) and remove in PHP 9.0.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] Define proper semantics for range() function

2023-05-08 Thread G. P. B.
On Fri, 5 May 2023 at 15:49, Derick Rethans  wrote:

> On Mon, 27 Mar 2023, G. P. B. wrote:
>
> > While working on analysing the impact of the changes proposed by
> > amending the behaviour of the increment and decrement operators (
> > https://wiki.php.net/rfc/saner-inc-dec-operators) I discovered that
> > the range() function has some rather lax behaviour that is very
> > unintuitive.
> >
> > I therefore propose the "Define proper semantics for range() function"
> > RFC to address the unintuitive behaviour that sees no usage and/or
> > hide bugs: https://wiki.php.net/rfc/proper-range-semantics
>
> | If $step is a float but is compatible with int interpret it as an
> | integer.
>
> I guess you mean with that "the fraction is '.0'" and "-2^52 < number <
> 2^52" ? What is also going to be playing up is the range of $start and
>
$end itself.
>

Yes, this is what I mean by "compatible with int".


> | Introduce and use a proper ZPP check for int|float|string $start and
> | $end parameters, this will cause TypeErrors to be thrown when passing
> | objects, resources, and arrays to range().
>
> I am not sure whether it wise to disallow resources. These are often
> file descriptors and having a range on those *could* make sense? Not
> fussed much either way though.
>

This is not how file/stream resources work. The resource number is not
correlated to the file descriptor.
The only cases where this is the case are the predefined streams STDIN,
STDOUT, and STDERR.
This is part of the reason for needing a new function like the
file_descriptor() function I'm proposing. [1]


> | Throw a ValueError when passing a negative $step for increasing
> | ranges.
>
> I think that's a BC break that is not worthy of making.
>

There is no impact shown, and it doesn't make sense to pass a negative
$step.
This can be changed to a deprecation but considering there is no impact I
don't see why we should wait.

| Emit an E_WARNING when $start or $end has more than one byte.
>
> Surely only if the string doesn't represent an int or float?
>

Yes, this was implied. As we are only concerned about the "pure string"
case here.
But I have clarified this.

| var_dump(range(null, 2));
>
> I think that safely can be interpreted as range(0, 2) — I wouldn't throw
> a deprecation warning for that... but then of course, we do already have
> a similar (deprecation) warning for arguments.
>

The deprecation is just a consequence of finally using ZPP, thus invoking
the previously accepted RFC.


> > The change propose to throw TypeErrors and ValueErrors for case where I
> > couldn't find occurrences in the wild and hide bugs, and emit some
> > E_WARNINGs for cases that are hard to detect via static analysis.
>
> | Target Version: PHP 8.3
>
> In my opinion that's not a good thing. There are a fair amount of BC
> breaks (beyond the one that I oppose to with nagative steps throwing an
> ValyeError), without first having deprecations. IMO, the actual BC
> breaks here should become deprecations in 8.X and BC can only be really
> broken in 9.0.


Disagree, the implementation is already complicated, and trying to support
the current absurd behaviour is impractical, especially as no impact has
been found.

Best regards,

George P. Banyard

[1] https://wiki.php.net/rfc/file-descriptor-function


Re: [PHP-DEV] [RFC] New core autoloading mechanism with support for function autoloading

2023-04-11 Thread G. P. B.
On Tue, 11 Apr 2023 at 09:48, Rowan Tommins  wrote:

> > As a result of the pinning, function_exists() will return true for
> functions in namespaces that have been pinned to a global function.
>
> The lookup pinning seems like the right way to optimise the
> implementation, but I don't think it should be visible to user code like
> this - a lookup for "strlen" in namespace "Foo" is not the same as defining
> the function "Foo\strlen". Consider this code:
>
> namespace Bar {
> if ( function_exists('Foo\strlen') ) {
> \Foo\strlen('hello');
> }
> }
> namespace Foo {
>  strlen('hello'); // triggers name pinning
> }
> namespace Bar {
> if ( function_exists('Foo\strlen') ) {
> \Foo\strlen('hello');
> }
> }
>
> If I'm reading the RFC correctly, the second function_exists will return
> true. I'm less clear if the call to \Foo\strlen will actually succeed - if
> it gives "undefined function", then function_exists is clearly broken; if
> it calls the global strlen(), that's a very surprising side effect.
>

It *should* indeed call the global strlen() but I hadn't actually created
such a test as ... I hadn't thought of doing that.
However, we *already* do function pinning which can result in this
behaviour via the function cache, see the following bug which defines a new
function via eval():
https://bugs.php.net/bug.php?id=64346

I am not sure that it calling the global strlen() is that surprising, as it
is basically aliasing the function \Foo\strlen() to \strlen().

For bonus points, the call to strlen that triggers pinning could be inside
> an autoloader, making even the first function_exists call return true.
>

If it is in the same namespace as the autoloader, then yes. However, if the
autoloader is in Bar then only Bar\strlen() is being aliased to \strlen().
Ilija mentioned this off-list, and I hadn't considered this, but this could
lead to a large increase of symbols being defined in the function symbol
table, as every nonqualified call (either by using the "use" statement, or
writing the full FQN) will get aliased to a global function and take an
entry in the symbol table.


> Similarly, I think it should be possible to "unpin" a function lookup with
> a later definition, even if no autoloading would be triggered. That is,
> this should not be a duplicate definition error:
>
> namespace Foo;
> if ( strlen('magic') != 42 ) {
> function strlen($string) { /* ... */ }
> }
>

There are some larger technical issues at play, as mentioned in the
previous bug.
The function cache will pin the call and there is no way of unpinning it. I
tried looking into fixing this, but it turns out to be too complicated
(/impossible?).
More so, disabling the function cache is a massive performance penalty.
As such, the RFC follows the current de facto behaviour.



> > The use of the word class in the API is currently accurate
>
> This isn't actually true: classes, interfaces, traits, and enums all share
> a symbol table, and thus an autoloader. I don't know of a good name for
> this symbol table, though.
>

They do share a symbol table indeed but using class is probably the least
confusing one.


> Regarding the API, would it be possible to take advantage of nearly all
> autoloaders only being interested in particular namespace prefixes?
>
> Currently, every registered autoloader is run for every lookup, and most
> immediately check the input for one or two prefixes, and return early if
> not matched. I suspect this design is largely because autoloading came
> before namespaces, so the definition of "prefix" wasn't well-defined, but
> going in and out of userland callbacks like this is presumably rather
> inefficient.
>
> Perhaps the "register" functions should take an optional list of namespace
> prefixes, so that the core implementation can do the string comparison, and
> only despatch to the userland code if the requested class/function name
> matches.
>

That is actually interesting, hadn't thought about taking an array of
prefixes.
And yes, every callback call requires a VM re-entry, which is expensive.

Should the prefix be with or without the trailing backlash?

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] New core autoloading mechanism with support for function autoloading

2023-04-11 Thread G. P. B.
On Mon, 10 Apr 2023 at 20:12, Anton Smirnov  wrote:

> Hello George,
>
> I'm not sure I'm 100% correct but I think that this RFC still allows to
> call different functions for the same code, just not in the same run.
>
> Consider this pseudocode:
>
> //- funcs.php
>
> namespace My;
>
> function myfunc(...) { ... }
> function array_map(...) { ... }
>
> //- code1.php
>
> namespace My;
>
> myfunc(1); // autoload triggered
> // bonus points if it's a different file or some obscure method
> array_map(strlen(...), []); // My\array_map
>
> //- code2.php
>
> namespace My;
>
> // no prior autoload
> array_map(strlen(...), []); // global array_map
>

No, array_map will trigger the autoloader (actually strlen() will trigger
it once first) and the autoloader should then load the array_map function
from the My namespace.
However, if the autoloader does not correctly load the My\array_map()
function then it will be bound to the global function.

I can add some test cases for that if needed.


> And if I understand the current order of execution correctly, here is a
> more extreme example:
>
> //- moreextreme.php
>
> namespace My;
>
> if (some_random()) {
>   array_map('My\myfunc', []); // global array_map
> } else {
>   array_map(myfunc(...), []); // My\array_map
> }
>
> --
> Anton
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>


[PHP-DEV] [RFC] New core autoloading mechanism with support for function autoloading

2023-04-10 Thread G. P. B.
Hello Internals,

Dan and I would like to propose a new core autoloading mechanism that fixes
some minor design issues with the current class autoloading mechanism and
introduce a brand-new function autoloading mechanism:
https://wiki.php.net/rfc/core-autoloading

The existing SPL autoloading functions would become aliases to the new Core
ones and will continue to work without any migrations needing to be
performed.

Hope to hear your opinions about this!

Best regards,

George P. Banyard


Re: [PHP-DEV] Array spread append

2023-04-06 Thread G. P. B.
On Thu, 6 Apr 2023 at 14:04, mickmackusa  wrote:

> I think it would be more intuitive to implement an array "appending"
> functionality with "concatenating" syntax.  The `.=` syntax already exists,
> but is forbidden from use on non-strings.  If you want to implement this
> preexisting syntax as an array concatenation operator, then it is a blank
> slate -- you can decree whatever behaviors you wish for it without blurring
> what the combined operator does with strings.
>

This would be the idea situation, but this is not correct.
Arrays, still in PHP 8, get automatically cast to strings during
concatenation or echo:


Results in:
Warning: Array to string conversion

Warning: Array to string conversion
string(10) "ArrayArray"

See: https://3v4l.org/26Jba

I'm expecting that in PHP 9 this is going to throw a TypeError similarly to
how non-string castable objects throw a TypeError in PHP 8.
But because this was "only" a notice in PHP 7, this specific change only
got promoted to an E_WARNING.

Thus, this suggestion is not possible until the major version, but I do
prefer this compared to the weird $a[...] syntax.

Bbet regards,

George P. Banyard


Re: [PHP-DEV] [RFC] Define proper semantics for range() function

2023-03-30 Thread G. P. B.
On Thu, 30 Mar 2023 at 03:50, Mark Baker  wrote:

> On 28/03/2023 00:36, G. P. B. wrote:
> > Hello internals,
> >
> > While working on analysing the impact of the changes proposed by amending
> > the behaviour of the increment and decrement operators (
> > https://wiki.php.net/rfc/saner-inc-dec-operators) I discovered that the
> > range() function has some rather lax behaviour that is very unintuitive.
> >
> > I therefore propose the "Define proper semantics for range() function"
> RFC
> > to address the unintuitive behaviour that sees no usage and/or hide bugs:
> > https://wiki.php.net/rfc/proper-range-semantics
> >
> > The change propose to throw TypeErrors and ValueErrors for case where I
> > couldn't find occurrences in the wild and hide bugs, and emit some
> > E_WARNINGs for cases that are hard to detect via static analysis.
>
> Unlike your changes to the increment operator, I'd love to see this
> rationalisation put in place, though like many here I don't see problems
> with using a negative step with decreasing ranges, but would consider it
> strange for increasing ranges.


I still find it somewhat odd, but this is not a hill I'm going to die on.
I've changed the behaviour to throw a ValueError if a negative step is
provided with increasing range and accept negative steps for decreasing
ranges.
Furthermore, I've also made passing an empty string an E_WARNING with a
cast to 0, same as the current behaviour.

See new version:
https://wiki.php.net/rfc/proper-range-semantics


> And I do want to see some
> case-consistency when working with string ranges.
>
>
> I'd love to see it taken a stage (or two) further; returning an iterable
> rather than an array (although that would be a bc break); and working
> with strings (ASCII only) in the same way that the increment operator
> does, so that range('A', 'IV') would be valid, and return `Z` then `AA`,
> `AZ` then `BA`, etc.
>

Frankly I was also surprised that the behaviour with strings was to do an
ASCII code point increment.
As I would agree that range("Y", "AC") returning ["Y", "Z", "AA", "AB",
"AC"] would have been more intuitive than the silently discarding
everything past the 1st byte.
However, I don't think there is much point in breaking BC to return a
possible generator or fix the unfortunate string behaviour.
I would rather that PHP creates dedicated syntax to creates ranges (e.g.
$s..$e seems to be what most other programming languages settles on,
although it might be slightly confused as concatenation) à la Ruby which
allows objects that implement certain methods to also be used to generate
ranges.
This is IMHO way more powerful as it would allow the creation of Date
ranges or other custom ranges.
And part of this proposal could be to support the aforementioned
alphabetical string ranges natively without needing to break BC on range()
and let this function just fade away into obscurity.

There is also this C++ talk from over a decade ago that argues that Ranges
are better than iterator, so this might be an additional motivation as to
why we would want this:
https://accu.org/conf-docs/PDFs_2009/AndreiAlexandrescu_iterators-must-go.pdf


> I am slightly surprised that you make no mention of the odd behaviour of
> mixed alphameric strings, e.g. var_dump(range('A1', 'C5')) which returns
> a purely alpha array 'A' to 'C'; or var_dump(range('3c', '5e')) which
> returns numeric (3, 4, 5); or var_dump(range('1', '1e2')) which treates
> `1e2` as scientific and returns 1..100.
>

Because I didn't think of this and was just well usual numeric string
behaviour or non-numeric string behaviour that truncates the string.
But that range('3c', '5e') is the only way to get an array of digits as
strings, and it makes me want to shout into the abyss.
I'm not sure it super worth to mention those cases, but I can add examples
of this to the RFC after crying about the even more insane behaviour
range() currently has.

Best regards,

George P. Banyard


Re: [PHP-DEV] [IDEA] allow extending enum

2023-03-29 Thread G. P. B.
On Wed, 29 Mar 2023 at 09:31, Rokas Šleinius  wrote:

> Enums were a very useful addition to PHP, however one aspect of them is
> neither
> explicitly documented - or seemingly even talked about.
>
> Enums were implemented as final so they cannot be extended nor can extend
> anything else.
>

This is by design.
Enumerations are in type theory parlance sum types.
Objects in PHP, in general, are what are called product types.


> From a user perspective it's surprising - and actually limiting.
>

The point of enums is to be limiting.


> USAGE EXAMPLE:
> I am making an error management system: each error presented to the user
> must have a unique code visible.
>
> ```php
> class SystemError
> {
> public function __construct(
> private string $errorText,
> private ErrorCode $code
> ) {
> }
>
> public function __toString():
> {
> return $this->errorText . ' ' . $this->code->toString();
> }
> }
> // ...
>
> enum ErrorCode
> {
> case Code_1;
> case Code_2;
>
> public function toString(): string
> {
> return 'Error code:' . substr($this->name, strlen('Code_'));
> }
> }
> ```
>
> Now I want to modify it to support different modules with different
> namespaces for
> errors, e.g. an ApiError, simple enough:
>
> ```php
> enum BaseErrorCode
> {
> // ...
> }
>
> enum ErrorCode extends BaseErrorCode
> {
> case Code_1;
> case Code_2;
>
> // ...
> }
>
> enum ApiErrorCode extends BaseErrorCode {
> // ...
> function toString(): string
> {
> return 'Error code:API-' . substr($this->name, strlen('Code_'));
> }
> }
> ```
>
> This results in a syntax error.
>

You are approaching the design of this in a fundamentally wrong way.
Enums, as sum types, are meant to enclose a *finite* number of states.
If you want to expand those finite number of states, the solution is to use
a union type.

Let's say you have those 3 enums:

enum GenericErrors {
case ErrorType1;
case ErrorType2;
}

enum FileErrors {
case FileErrorType1;
case FileErrorType2;
case FileErrorType3;
}

enum NetworkErrors {
case NetworkErrorType1;
case NetworkErrorType2;
case NetworkErrorType3;
}

And you have a function that can fail with either a generic error or a file
error then you can be explicit by having:

function foo(): T|GenericErrors|FileErrors { /* Do something or Fail */ }

This signature provides you with great type safety and can be checked via
static analysis that *all* error cases are handled.
Moreover, you know you don't need to handle NetworkErrors.
Extending an Enum *loses* you this type safety, because now ff you say
something returns a GenericErrors well who knows if you cover all cases
because someone might have ad-hock added a new one.

PHP's type system is not perfect yet, but it has become powerful enough
that you can do things like what I just described.
Ideally we would have generics, and you could use a Result/Either Monad
(fancy word for "wrapper") to have a return type of Result

Use the type system properly instead of trying to shove everything into an
"Inheritance" view of things.
Union types are not "bad", they are another way of creating a sum type.
(You could, with a lot of effort, create "proper" enumerations since PHP
8.0 by using union types and singleton classes.)


PROPOSAL:
>
> Enums should be able to extend other enums.
>
> For a complete wishlist, add:
>  * abstract enums;
>
 * enums allowed to implement interfaces;
>

Enums can already implement interfaces: https://3v4l.org/tKtQ0


> However since I have no experience in PHP source code, I can only
> provide the test suite for a possible PR this might have :(
>
> Do you think this is likely to get implemented?
>

I dear hope so not.
Breaking fundamental type theoretical assumptions is terrible.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] Define proper semantics for range() function

2023-03-28 Thread G. P. B.
On Tue, 28 Mar 2023 at 08:29, Tim Düsterhus  wrote:

> Hi
>
> On 3/28/23 00:36, G. P. B. wrote:
> > I therefore propose the "Define proper semantics for range() function"
> RFC
> > to address the unintuitive behaviour that sees no usage and/or hide bugs:
> > https://wiki.php.net/rfc/proper-range-semantics
>
> The "ASCII code point range" example is confusing, because it is a
> decreasing range. However decreasing ranges are only introduced in the
> next example.
>
> Best regards
> Tim Düsterhus
>

I've reordered the example and added a more descriptive ASCII code point
range example first!

I've also added an example which highlights the behaviour with null and how
it would emits deprecation notices.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] Define proper semantics for range() function

2023-03-28 Thread G. P. B.
On Tue, 28 Mar 2023 at 08:19, Christian Schneider 
wrote:

> Am 28.03.2023 um 00:36 schrieb G. P. B. :
> > I therefore propose the "Define proper semantics for range() function"
> RFC
> > to address the unintuitive behaviour that sees no usage and/or hide bugs:
> > https://wiki.php.net/rfc/proper-range-semantics
> >
> > The change propose to throw TypeErrors and ValueErrors for case where I
> > couldn't find occurrences in the wild and hide bugs, and emit some
> > E_WARNINGs for cases that are hard to detect via static analysis.
>
> I think it makes sense to clean up the range() function, thanks!
>
> There are two cases I would handle differently:
> - I'm not sure why a negative step for $start > $end is considered wrong,
> I consider range(10, 0, -2) at least as logical/readable as using a
> positive decrement of 2. Not requiring a sign for steps seems weirder to me
> but that's something we cannot change. *BUT* if it is the result of a
> calculation it seems wrong to require an abs() around it. I do see the
> reason for a warning/error when $start < $end and $step < 0.
>

Considering the only other programming language that I know of that has a
range() function that accepts a step argument is Python, and its behaviour
is IMHO worse.
For increasing ranges it requires a positive step, and if not just
generates an empty range.
For decreasing ranges it requires a negative step, and if not just
generates an empty range (this applies even if using the default step value
of 1 which is bonkers).

Making it a requirement to pass a negative step is definitely out of the
question.
Making it okay to use negative steps *only* for decreasing ranges could be
sensible, but we check for the step parameter way before we look into the
boundary values because those are different for int, float and string
boundaries.
Moreover, I personally find it weirder to require a sign for negative steps
as for me a step is something that *must* be positive, and at least Kotlin
seems to somewhat agree with me looking around at
https://kotlinlang.org/docs/ranges.html#progression and playing with the
source code
Namely:
for (i in 4 downTo 1 step 2) print(i)

> 42

for (i in 4 downTo 1 step -2) print(i)

> Exception in thread "main" java.lang.IllegalArgumentException: Step must
be positive, was: -2.


> - Values of '' or null in integer context (e.g. range(null, 10, 2)) should
> IMHO emit a warning first, not directly be changed to a TypeError. The
> usual BC / migration concern :-)
>

When null is used, no TypeError is emitted, just the "usual" null to scalar
deprecation that happens since
https://wiki.php.net/rfc/deprecate_null_to_scalar_internal_arg got accepted.
But I'll add an example to the RFC and a test case in the PR.

Trying to figure out if an empty string was used with another string
boundary is tedious, as this information needs to somehow get carried
around.
A previous iteration of the PR used to convert empty strings to 0 with a
warning, but considering the analysis I decide to just make this a
ValueError as it doesn't seem that empty strings are actually used in
practice.
But this is an easy revert, and I'm not really bound to this decision.

Best regards,

George P. Banyard


[PHP-DEV] [RFC] Define proper semantics for range() function

2023-03-27 Thread G. P. B.
Hello internals,

While working on analysing the impact of the changes proposed by amending
the behaviour of the increment and decrement operators (
https://wiki.php.net/rfc/saner-inc-dec-operators) I discovered that the
range() function has some rather lax behaviour that is very unintuitive.

I therefore propose the "Define proper semantics for range() function" RFC
to address the unintuitive behaviour that sees no usage and/or hide bugs:
https://wiki.php.net/rfc/proper-range-semantics

The change propose to throw TypeErrors and ValueErrors for case where I
couldn't find occurrences in the wild and hide bugs, and emit some
E_WARNINGs for cases that are hard to detect via static analysis.

Best regards

George P. Banyard


Re: [PHP-DEV] [RFC] [VOTE] Saner array_(sum|product)()

2023-03-06 Thread G. P. B.
Hello internals,

I am pleased to announce that the RFC was accepted unanimously, with 27 yes
votes and 0 no votes.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] Saner array_(sum|product)()

2023-02-20 Thread G. P. B.
On Mon, 20 Feb 2023 at 14:38, Andreas Hennings  wrote:

> Hello,
> the RFC seems like a good idea to me.
> However, I do not see any mention of plus operator on arrays, e.g. ['a' =>
> 'A'] + ['b' => 'B']. The array_reduce() would support this, but I think
> array_sum() would not, because of the return type. I just think this should
> be made more explicit.
> -- Andreas
>

Arrays are mentioned in the RFC, and they are already not supported in the
array_(sum|product) functions and remain unsupported.

Best,

George P. Banyard


[PHP-DEV] [RFC] [VOTE] Saner array_(sum|product)()

2023-02-20 Thread G. P. B.
Hello internals,

I've just opened the vote for the Saner array_(sum|product)() RFC:
https://wiki.php.net/rfc/saner-array-sum-product

Best regards,

George P. Banyard


Re: [PHP-DEV] What's the purpose of zend_result?

2023-02-20 Thread G. P. B.
On Sun, 19 Feb 2023 at 08:45, Nikita Popov  wrote:

> On Sun, Feb 19, 2023, at 09:21, Max Kellermann wrote:
> > On 2023/02/19 08:56, Nikita Popov  wrote:
> > > If you have a function like zend_stream_open_function(), SUCCESS and
> FAILURE are directly meaningful values.
> >
> > Agree, but that doesn't explain why FAILURE needs to be negative.
>
> I expect that there are two main reasons for that:
>  - There are probably some places that return a (non-negative) value or
> FAILURE.
>  - There are probably some places that check for success/failure using >=
> 0 and < 0. Another POSIX-ism.
>
> I don't think we endorse such usage, but it likely exists.
>
> Let me turn the question around: Is there some reason to change the value
> of FAILURE at this point?
>
> > > The current guideline for use of bool and zend_result in php-src is
> that bool is an appropriate return value for "is" or "has" style functions,
> which return a yes/no answer. zend_result is an appropriate return value
> for functions that perform some operation that may succeed or fail.
> >
> > What does the return value of these functions mean?
> >
> > - zend_make_printable_zval()
> > - zend_make_callable()
> > - zend_parse_arg_bool()
> > - zend_fiber_init_context()
> > - zend_observer_remove_begin_handler()
> > - php_execute_script()1
> >
> > If I understand the guideline correctly, then those examples (and
> > countless others) are defective and should be fixed, correct?
>
> At least in principle, yes. Of course, actually doing a change may not
> always be worthwhile, especially as this might result in a silent behavior
> change for extensions (as putting the return value into an "if" would
> invert behavior now).
>
> I believe Girgias has done extensive work on making the int vs bool vs
> zend_result situation more consistent, so you might want to coordinate with
> him.
>
> Regards,
> Nikita


Yeah, I spent a lot of time around the release of PHP 8.0 and a bit in
between releases to convert various int types to bool or zend_result just
to make the API clearer.
One of the main cases I missed was to change the return value of object
handlers, as those, for the most part, are meant to return zend_result.
I didn't push forward with that one as when I realized we were already in
beta releases or maybe RC and even changing it for a minor version did feel
a bit disruptive.
However, it may make sense to tackle this sooner than later if we are doing
some other object handler changes.

There are however definitely still cases where FAILURE is assumed to be -1
either because this is what the function is expected to return on failure
or checking the value of a POSIX like API.
Ideally, all cases where zend_result is used would actually indicate this
to make it possible to *maybe* convert SUCCESS to true and FAILURE to false
and typedef zend_result to bool.

Best,

George P. Banyard


Re: [PHP-DEV] [RFC] Saner array_(sum|product)()

2023-02-12 Thread G. P. B.
Hello internals,

If there are no further feedback I intend on opening the vote for this
tomorrow.

Best regards,

George P. Banyard


Re: [PHP-DEV] [VOTE] include cleanup

2023-02-12 Thread G. P. B.
On Thu, 9 Feb 2023 at 22:09, Matthew Weier O'Phinney <
mweierophin...@gmail.com> wrote:

> I'm not directly involved in maintenance, but my take on the scenario was
> that these were rejected and reverted because they caused breakage, whether
> that was in compiling a spare PHP build, or in extensions that were
> assuming that using certain headers would slurp in everything they needed.
> This breakage was unacceptable without an RFC. I saw chatter from a number
> of folks after the changes were merged about builds no longer compiling;
> considering the stability of the php-src tree, inability to build will
> always be a source of alarm.
>
> [...]
>
> As I pointed out earlier, the changes previously merged led to breakages
> when compiling the project. How is that not enough? And dumping a huge
> bunch of PRs with such changes without first discussing it with maintainers
> means a lot of effort reviewing  [...].
>
The other point that has been brought up multiple times is that it
> introduces breaking changes for extension maintainers.
>
> Should these extensions be relying on one or more "god" headers instead of
> the specific headers for the symbols they use? Probably not. Will forcing
> the issue without giving them a chance to review and understand the
> changes, and have a roadmap for when and how those changes occur be a net
> positive? No; it will cause a lot of busy work for a lot of people, almost
> all of whom are volunteers and most of whom would rather be building out
> user-requested features or fixing user-reported bugs.
>
> I'm unsure why that's unclear or not enough for you.
>
> --
> Matthew Weier O'Phinney
> mweierophin...@gmail.com
> https://mwop.net/
> he/him
>

I'm going to ignore Max's subpar behaviour and frustration in the aftermath
of this.

However, as the reviewer of these PRs, they did NOT completely break the
build, I was working on php-src and compiling master without ANY issues
during the weekend after landing the changes.
The *only* build being broken was the DTrace build, which would have been
fixed by a follow-up PR.
We have had completely broken builds for longer days due to some other
random changes, and we didn't revert them but fixed them as a follow-up.
We still, for over 6 months now, have a "broken" ASAN build due to phpdbg
messing up the analyser and crashing the test runner on 8.2 and master,
something that multiple core devs, me included, need to work around by
monkey patching the run-test.php file.

These changes were initially agreed upon and multiple people were in
favour, even by Dmitry.

I will also say, that I spend a significant amount of time reviewing those
PRs, and I would have continued, as I think they are beneficial to the
project.
But my opinion and the work I put into the reviews is apparently worthless.
Therefore, I maintain that, IMHO, these commits should not have been
reverted.
The unique complaint, that should be addressed to me, is in how I merged
the PRs in that I didn't squash them.

The fact external extensions were broken and that we should add all
relevant headers to php.h is a fair complaint and would have been fixed
quite easily in a single commit and did not affect anyone effectively at
this stage because we are, checks notes, 6 months at minimum before the
first alpha builds of PHP 8.3 are released.

The only reason they were reverted is that Dmitry demanded them as they
broke the DTrace build, which he is apparently the only one to use and is
not tested on CI, a failure of the project's CI infrastructure.

As I previously complained, the RFC process for this kind of change is
completely inadequate and demonstrates that the PHP project has a massive
issue with governance and how to handle cases like this.

As a final note, if the complaint had been made by anyone else other than
Dmitry, I doubt these changes would have been reverted, and can we please
stop pretending otherwise.

Sincerely,

George P. Banyard


Re: [PHP-DEV] Deprecate ldap_connect with host and port as separate arguments

2023-02-07 Thread G. P. B.
On Tue, 7 Feb 2023 at 12:56, Côme Chilliet  wrote:

> Le vendredi 27 janvier 2023, 10:00:35 CET Andreas Heigl a écrit :
> > Hey Folks.
> >
> > I think it would be a good idea to deprecate calling ldap_connect with 2
> > parameters host and port.
>
> Hello,
>
> My long term plan was to replace it by a constructor for the new
> \LDAP\Connection class that only accepts the URI syntax. Which would also
> be better because ldap_connect is a really confusing name as it does not
> actually connect to anything.
>
> But I’m unfamiliar with how to write object methods into PHP modules, and
> I do not have much time to allocate to php-ldap.
>
> If you are interested into working on some OO methods for php-ldap
> classes, I have a ton of ideas on how to make it awesome.
>
> Côme
>

Please let me know and I can spend some time on it.
I also think working with objects was made significantly easier now that we
have proper stubs and derive a lot of code generation from it.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] Saner array_(sum|product)()

2023-01-26 Thread G. P. B.
Hello internals,

I've slightly updated the RFC:
https://wiki.php.net/rfc/saner-array-sum-product

I've added an FFI example which overloads addition but cannot be cast to a
numeric type.
And changed the behaviours around objects. An object needs to implement a
numeric cast for it to be added/multiplied, this solves the issue around
having the return value being cast to an object as this cannot happen
anymore.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] Path to Saner Increment/Decrement operators

2023-01-23 Thread G. P. B.
On Fri, 20 Jan 2023 at 19:44, Jordan LeDoux  wrote:

> I don't see a section in the RFC about JIT or anything related to OpCache,
> but I know from experience with the Operator Overloads RFC that there are
> several architecture specific assembly optimizations for ++ and --. Have
> these been considered, and how will they be impacted?
>

The only assembly specific code is for integer increments/decrements, as
those are not affected by this RFC there is no impact to them.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] Path to Saner Increment/Decrement operators

2023-01-23 Thread G. P. B.
On Fri, 20 Jan 2023 at 18:28, Mark Baker  wrote:

> The documentation page consistently uses the word Increment and
> Decrement, not Add 1 and Subtract 1.
>
> Developers who read the documentation should be aware of the Perl
> convention when dealing with alphabetic strings, and should expect that
> behaviour. Alphanumeric strings are certainly more problematic, less
> well documented, and less well understood, and I'll agree that they're
> inconsistent in their behaviour.
>

The PHP documentation has never been the source of truth about the PHP
implementation,
if it was dynamics properties should have been removed without any notice
as until them being deprecated there was no documentation.
So arguing *something* should behave a certain way because the docs are
written in a certain way holds no value to me.
The state of the PHP docs could certainly be improved, as it is blatantly
lying at various core sections.

However, the whole point of this RFC is to *remove* cognitive burden for
developers, so they don't even need to be aware of this "feature" and not
get surprised when it kicks in.
Moreover, by your logic, you wouldn't care if we removed support for
alphanumeric strings and only let the PERL increment kick in for purely
alphabetical.
While convenient for you, someone might actually use this feature on
alphanumeric strings, and we're back to "why is my use case being removed
while that other just as weird one remains".

I even went initially for such a proposal (see previous revision [1]),
however, I realized this provides minimal benefit as it doesn't reduce at
all the cognitive burden or the overall design specification of the
language.



> Deprecating the Increment operator for strings will create extra work
> for me, will affect many of the users of my library, and I'm certain it
> will also have a performance impact on the library (replacing that
> operation with a more expensive function call for alpha increments, but
> still having the operation for numeric increments). So yes, I am willing
> to die on this hill because that deprecation will have a very direct and
> adverse affect on my work.
>

If the issue is about performance, it is possible to enhance the optimizer
to inline those two functions (something the optimizer already attempts to
do for userland functions [2]).
The other aspect goes back to the typical conundrum: Do we think PHP is in
decline?
 - If yes, then not doing breaking changes and catering to legacy projects
is definitely the course of action to take.
 - If no, improving PHP for the next generation of developers and software,
so that it is easier to learn and reason about, should be the course of
action to take.

As I personally think we are in the second situation, that's why I'm
tackling this subject in this manner.
It's the same belief that made us deprecate dynamic properties, convert
warnings to Errors, etc.

Now, if we are in the former case, then you are totally justified, and I
probably should find another job as I see no point in not improving the
overall language semantics.

George P. Banyard

[1] https://wiki.php.net/rfc/saner-inc-dec-operators?rev=1669977388
[2] See zend_try_inline_call() function


Re: [PHP-DEV] [RFC] Path to Saner Increment/Decrement operators

2023-01-20 Thread G. P. B.
On Thu, 19 Jan 2023 at 00:23, Mark Baker  wrote:

>
> On 18/01/2023 21:25, G. P. B. wrote:
> >> I would like to start the discussion on the Path to Saner
> >> Increment/Decrement operators RFC:
> >> https://wiki.php.net/rfc/saner-inc-dec-operators
> >>
> >> The goal of this RFC is to reduce language complexity by making $v++
> >> behave like $v += 1 and $v-- behave like $v -= 1;
> > If that is the goal, then I would agree with this RFC.
> >
> > However, changing the PERL string increment feature does not IMO fit
> > into that goal, and it also a useful*feature*. On that base I would
> > vote against this. And I suspect many others would as well.
>
> However, the ++ and -- are the "Increment" and "Decrement" operators,
> not the Add1 and Subtract1 operators; while they behave in that way when
> used with variables containing numeric values, they are special
> operators and not simply a syntactic sugar for +=1 and -=1. As long as
> their behaviour is consistent, and definition of what "Increment" and
> "Decrement" mean is clearly defined for different datatypes, then I feel
> that the PERL-style alpha string increment has enough valid use cases to
> justify itself.
>

That's a strange hill to die on, most people would expect that those
operators do indeed behave like Add1 and Sub1, and clearly you are not
having any issue with making -- act like Sub1.
There is even a user note on the manual page from 21y ago where someone was
expecting booleans to be incremented. [1]
Moreover, the alphanumeric string increment feature is fundamentally broken
and unsound due to the simple fact that PHP supports converting numeric
strings written in scientific notation to float.
And this behaviour of casting a numeric string in scientific notation
*always* takes precedent over the alphanumeric increment.
The following code samples show this perfectly:
$s = "5d9";
var_dump(++$s); // string(3) "5e0"
var_dump(++$s); // float(6)
$s = "5e9";
var_dump(++$s); // float(51)
var_dump(++$s); // float(52)

Behaviour that *also* has a user note on the manual page. [2]

It is possible to have a sound implementation of it in userland, via the
following function:
function polyfill(string $s): string {
if (is_numeric($s)) {
$offset = stripos($s, 'e');
if ($offset !== false) {
/* Using increment operator would cast the string to float
 * Therefore we manually increment it to convert it to an
"f"/"F" that doesn't get affected */
$c = $s[$offset];
$c++;
$s[$offset] = $c;
$s++;
$s[$offset] = match ($s[$offset]) {
'f' => 'e',
'F' => 'E',
'g' => 'f',
'G' => 'F',
};
return $s;
}
}
return ++$s;
}


> We might also discuss consistency of datatype changes when these
> operators are used.
>
> $a = PHP_INT_MAX;
>
> ++$a;
>
> or
>
> $a = '10';
>
> ++$a;
>
> both change the datatype of $a; which isn't documented behaviour either.
>

Those are just "regular" well documented type coercions due to addition and
int to float promotions.
And yes, the PHP documentation could be better, but no one is paid to work
on it.
However, it turns out that I only work part-time for The PHP Foundation,
and therefore I am available to be hired to do some technical writing for
the PHP Documentation.
(If anyone does want to take me up on this offer, do feel free to email me).

In any case, I've updated the RFC to version 0.3, [3] with a whole section
about the current behaviour of the PERL increment implementation in PHP, a
native implementation of str_increment() and str_decrement() that handles
strings that can be interpreted as numbers written in scientific notation.
It also changes the timeline slightly, as this seems the preferred course
of action.

Best regards,

George P. Banyard

[1] https://www.php.net/manual/en/language.operators.increment.php#16149
[2] https://www.php.net/manual/en/language.operators.increment.php#109621
[3] https://wiki.php.net/rfc/saner-inc-dec-operators


Re: [PHP-DEV] [RFC] Path to Saner Increment/Decrement operators

2023-01-18 Thread G. P. B.
On Wed, 18 Jan 2023 at 14:35, Björn Larsson 
wrote:

> Since the alpanumeric_decrement RFC was rejected january 2014 9 years ago,
> could it be an option to bring it up again in conjunctione with your RFC?
>
> Maybe the added value of your RFC could swing the opinion. I mean there has
> been RFC's that required multiple tries to fly.
>

Possibly, and I could wait for the result of such an RFC, but I do not
intend on pushing this forward.

On Wed, 18 Jan 2023 at 15:32, Derick Rethans  wrote:

> On Tue, 17 Jan 2023, G. P. B. wrote:
>
> > I would like to start the discussion on the Path to Saner
> > Increment/Decrement operators RFC:
> > https://wiki.php.net/rfc/saner-inc-dec-operators
> >
> > The goal of this RFC is to reduce language complexity by making $v++
> > behave like $v += 1 and $v-- behave like $v -= 1;
>
> If that is the goal, then I would agree with this RFC.
>
> However, changing the PERL string increment feature does not IMO fit
> into that goal, and it also a useful *feature*. On that base I would
> vote against this. And I suspect many others would as well.
>

I do not understand how this does *not* fit into that goal.
$s = "a10";
$s += 1;
var_dump($s);
Results in a TypeError whereas
$s = "a10";
$s++;
var_dump($s);
Results in string(3) "a11"

Therefore, $s++ does not behave like $s += 1; and thus in scope.

Is there a way to avoid this single useful feature from being
> deprecated, while to good parts of this RFC stay?
>

Yes, but at that point, I don't see why we should unify the behaviour if it
is going to remain inconsistent. Might as well make incrementing on bool,
and decrementing null a TypeError in PHP 9 to make it stricter.


> I am also unsure as how much actual breakage this would cause, and
> before this gets up to a vote, I would like to see how bad (or not) this
> would affect already existing code bases.
>

Fair point, I can try and run Nikita's script on the top composer packages,
but that won't show the state of private codebases.

On Wed, 18 Jan 2023 at 16:03, Levi Morrison 
wrote:

> It seems to me that if you truly want to clean up this specific part
> of the language, you are going to have to play the long game:
>  1. New functions are added for the perl behavior of string increment
> and decrement. No warnings are given in code, but call-outs are made
> in upgrading and other documentation about this behavior changing.
> Note that in the past I would have used an E_STRICT for this, but
> people seem opposed to adding new E_STRICT warnings.
>  2. In the next minor version, we add a warning about the behavior
> when string increment/decrement is used.
>  3. In the next major version, we finally clean up the behavior.
>
> But this gets muddy if we do PHP 8.3 for step 1, and then we decide to
> go for PHP 9.0 instead of 8.4, and it messes with the "ideal" cycle.
>
> Note that I support this sort of plan, and would support it for
> cleaning up many other parts of PHP as well. It's just unfortunate it
> takes so long, but that's how it goes sometimes :/


I don't think we need such a long timeline because the function is easily
poly filled.
Moreover, if people jump a version in an upgrade, they are still going to
immediately receive a warning/deprecation.
But if such a timeline is preferred, I do not mind changing it.

On Wed, 18 Jan 2023 at 18:33, Alex Wells  wrote:

> Classes and methods is the expected way of implementing standard library in
> an OO language. New APIs (such as the new Random api) use OO instead of
> functions and it makes more sense to use OO in this case too: there's
> likely a place for other methods too, like toBase(int $otherBase) etc. It
> would also be possible to use overloaded operators if needed.


Until we have strings that can invoke methods, I don't see the point of
having an OO API.
PHP is a multi paradigm language, and creating a class with two methods
seems very useless to me.
OOP is favoured in PHP because using functions is just an overall terrible
experience that needs improvements, but using functional patterns is
totally doable (and can produce elegant code) in PHP.

George P. Banyard


Re: [PHP-DEV] [RFC] Saner array_(sum|product)()

2023-01-18 Thread G. P. B.
On Wed, 18 Jan 2023 at 15:06, Derick Rethans  wrote:

> On Tue, 17 Jan 2023, G. P. B. wrote:
>
> > I would like to start the discussion about the Saner
> > array_(sum|product)() RFC:
> > https://wiki.php.net/rfc/saner-array-sum-product
> >
> > Currently, the array_sum() and array_product() behave differently than
> > their userland implementation as they ignore arrays and objects, and
> > cast the remaining types to int/float. This is in contrast with the
> > behaviour of arithmetic operators that throw TypeErrors on types and
> > values that are not interpretable as int/float.
> >
> > This RFC aims to resolve this discrepancy.
>
> I think I agree with the premise of this RFC, but I do think a few
> details are wrongly addressed.
>
> First of all, a clarification why this produces int(4) would be useful:
>
> $input = [true, STDERR, new stdClass(), [], gmp_init(6)];
> $output = array_sum($input);
>
> I had to look up that STDERR would cast to int(3) :-)
>

ACK will clarify this.


> I don't understand why this would result in a warning and a return of
> int(50):
>
> $a = [10, 15.6, gmp_init(25)];
> var_dump(array_sum($a));
>
> Why doesn't this return float(50.6) instead? I realise that the
> array_reduce variant (below) does the same, but it's not what I would
> expect:
>
>  $input = [10, 15.6, gmp_init(25)];
> $output = array_reduce($input, fn($carry, $value) => $carry +
> $value, 0);
>

The issue is that now when encountering an object, if it defines a
do_operation handler, it will use that and not cast the value to a numeric
type.
I suppose it's possible to first check if the entry is an object and do a
manual cast, which might also negate the next issue you mentioned.


> I think the phrase "If traversing the array transforms the return value
> into an object, if that object is not numerically castable an E_WARNING
> is emitted and the initial return value is returned instead, which may
> change the return value if scalar values were present in the array. "
> should come with an example :-)
>

Within php-src we don't really have any "practical" examples, FFI\CData can
possibly satisfy this condition by doing something like:
$x = FFI::new("int[2]");
$x[0] = 10;
$x[1] = 25;
var_dump($x + 1);
/* dumps:
object(FFI\CData:int32_t*)#2 (1) {
  [0]=>
  int(25)
}
*/

I constructed a special class and added it to zend_test to be able to test
this behaviour, but will see if I can wrangle FFI to also show this case.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] Path to Saner Increment/Decrement operators

2023-01-18 Thread G. P. B.
On Tue, 17 Jan 2023 at 18:28, Mark Baker  wrote:

> On 17/01/2023 17:28, Craig Francis wrote:
> > I've seen this used a few times, e.g. starting with a numerical value
> (Passport number, NHS number, Social Security Number, Date of Birth
> 20230117), and the developer simply appends an incrementing letter on the
> end to get a unique reference; e.g. a person having multiple assessments...
> especially if it's more than 26 (A-Z), and you need to move to multiple
> letters, which `chr(90 + 1)` cannot help you with.
>
> Being able to increment alpha strings is incredibly useful when working
> with Excel spreadsheets (as I do on a daily basis), because the column
> Ids match this pattern; and I would hate to see this deprecated. Having
> to replicate that logic for traversing column Ids in userland code would
> be inconvenient (to say the least), would affect many of the users of my
> libraries, and would have a performance impact on my libraries. If
> anything, I'd rather like to see the decrement operator work with alpha
> strings as well for more consistency.
>
> I don't have the karma for a vote; but if I did then it would be a "No"
> for this alone, because I can see the problems that it will cause me and
> the users of my libraries.
>
>
> > That said, I appreciate that incrementing some strings can be a bit
> unusual (e.g. "A9" to "B0", vs "A 9" to "A 0").
>
> Agreed. While incrementing works in a very logical manner with mixed
> alphanumeric strings, it's not well documented behaviour, and most
> developers take a long time before they understand what it's actually
> doing. While there might be use cases for incrementing alphanumerics, I
> suspect that it would be better implemented in the business logic of an
> application, because the component parts of that string are likely to
> have business meaning; and also to provide better code readability.
>

I appreciate being shown concrete cases about the useful ness of this
operation.
The reason I didn't go with adding support for decrementing alphanumeric
strings is that it was unanimously rejected.
However, if Rowan's suggestion of adding
string_increment()/string_decrement() with more rigorous behaviour (that we
can flesh out together) would be part of this proposal, would you be more
inclined to accept deprecating ++ from performing this feature?

I truly believe having $v++ behave like $v += 1 and $v-- behave like $v -=
1; is something to strive for *because* it allows us to remove one
dedicated type juggling context people need to be aware of and simplifies
the overall semantics of the language.
Keeping support for string increments means that one cannot interchange $v++
and $v += 1 and that one needs to be aware about using it when a value
might hold a string.
As such, if it needs to remain its own type juggling context, the question
is why not make it stricter by having it warn and then throw a TypeError on
bool, reopening the can of worms that is the null handling between both
operators and what to do with the empty string case.
These questions are already answered by making those operators behave just
like addition/subtraction.

My order of preference for the semantics are as follows:
1. The behaviour described in the RFC (with or without function for string
in/decrement)
2. (with a massive gap, but I could live with it) adding support for string
decrements and tiding up the behaviour of the alphanumeric string to make
it stricter and less error-prone.
3. The current asymmetry (again with a massive gap between this and option
2)

But because option 2 seems out of the question due to the unanimous
rejection of https://wiki.php.net/rfc/alpanumeric_decrement, the only
viable options to me seem like 1 and 3.
As I hate option 3 I am pushing for option 1 as I think it has various
benefits.

Moreover, I do not want to split this into its own proposal (deprecating
string increments/figuring out what to do with them) as I feel it will make
any attempt to improve the situation harder.

Best regards,

George P. Banyard


Re: [PHP-DEV] RFC: rules for #include directives

2023-01-18 Thread G. P. B.
On Wed, 18 Jan 2023 at 09:01, Max Kellermann  wrote:

> On 2023/01/16 13:48, "G. P. B."  wrote:
> > Moreover, having those sorts of changes be RFCs seems counterproductive
> as
> > the only people who care about this are actual core and extensions
> > developers and this opens the gate for petty RFCs to resolve coding style
> > disagreements.
>
> How shall we proceed from here?  Shall I create an official RFC or
> not?
>
> George said no, which I understand; but I don't know what else to do
> to produce a decision.
>
> I asked Dmitry to post his GitHub arguments in this thread, so you see
> both sides of the story, and you can discuss his arguments.  (I
> already replied to him on GitHub, see
> https://github.com/php/php-src/pull/10345)
>

I still don't think the RFC process is a good vehicle for those sorts of
decisions, but it's the only process we have and there is some clear
disagreement that needs to get resolved here.
So I think creating an official RFC is the only way.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] Add SameSite cookie attribute parameter

2023-01-17 Thread G. P. B.
On Mon, 16 Jan 2023 at 15:36, Christian Schneider 
wrote:

> Am 16.01.2023 um 14:39 schrieb G. P. B. :
> > On Sun, 15 Jan 2023 at 20:58, Christian Schneider 
> wrote:
> > Some comments:
> > - I am not convinced that we should introduce a third way of providing
> parameters to setcookie(). I don't think this function is used often enough
> in common code to add yet another iteration of the API. Assuming there is 1
> to 2 places in your framework using this I don't think many bugs will go
> unnoticed. Adding a warning to illegal 'samesite' values in $options would
> IMHO be enough if stricter checking is wished for.
> >
> > How is this providing a third way of providing parameters to
> setcookie()? As it is, if I want to use named arguments and the typed
> parameters, I cannot set the SameSite attribute.
> > I've heard multiple people waste time due to a SameSite bug because they
> made a typo, and it took them way too long to figure out the issue as they
> thought they had a server configuration problem.
> > Adding a warning for invalid values is effectively turning that option
> into an Enum, which at this point one might as well have a proper Enum.
>
> Maybe third API was misleading, what I mean is that we had the (old) way
> of positional arguments for options which did not include something like
> samesite and the (new) way with the options array. So I would assume most
> code was migrated to $options.
> Now you are suggesting of updating the old API with an additional
> positional argument which would mean changing the array-form back to
> positional.
> Is your plan to deprecate the $options-API at some point in the future?
> Having two APIs offering the same functionality seems a bit confusing to me.
>

I personally don't plan on deprecating the array $options API. If two APIs
that provide the same functionality are too confusing, then the current
incomplete way of passing parameters should be deprecated at some point.
However, until then, having symmetry between both APIs is better for people
who prefer to have parameters and use named arguments than the $options
array.


> Alternatively the 'samesite' option in $options could accept
> SameSite::Lax. This would accomplish (almost) the same with a smaller
> change. Disclosure: I like arrays for options as they allow for array
> addition etc. Something similar can be done with ...$options, true :-)
>

That is actually a good idea to add support for the array version to accept
the new enum, I'll amend the implementation.


> Side-Note: Isn't SameSite a very generic name in the global namespace? I'm
> not sure what the PHP policy is here.
>

AFAIK the global namespace is "owned" by PHP so that shouldn't be an issue
per the usual policy.


> >   - I don't like the camelCase of $sameSite as this is different from
> all the other parameters, e.g. $expires_or_options (yes, this is a
> pseudo-parameter name, I know) and $httponly. Looking at a couple of
> functions in the standard PHP set I didn't see any $camelCase.
> >
> > ACK, it should probably be in snake case and be $same_site
>
> Looking at $httponly I would have expected $samesite.
>

I suppose it can be either or, but trying to improve the name of a not yet
existing parameter seems better than following the weird naming scheme. But
I'm not hard set on the name here.


> >   - A more generic question: How are Enums handled concerning future
> additions of values vs. BC compatibility? What is the migration plan there
> if one wants to support both old and new PHP versions?
> >
> > The parameter is optional, so I don't understand what migration plan you
> need?
> > If in the, unlikely, event that a new value is added this should be
> added as a case in the enum in the next minor version of PHP.
> > In the, again unlikely, event that a value is deprecated, the
> corresponding enum case should also be deprecated following the normal PHP
> deprecation process.
> >
> > I only decided to make this an enum because I deem it very unlikely for
> a new valid attribute value to be introduced, the new IETF RFC clarifies
> and amends the behaviour of the 3 valid attribute values that have always
> been the same since 2016.
>
> I was talking about extending Enums, not the parameter and not SameSite
> specifically.
> Are there any Enums in core PHP APIs where new values could be added in
> the future and how is the plan for code supporting multiple PHP version
> there? This was just something which crossed my mind when thinking about
> APIs with Enums. This is not really related to this RFC so I understand if
> you want to ignore this part :-)
>

I might be again misunderstanding, but one cannot extend an enum as they
are final classes under the hood.
Currently, the only other native enum is the one that was added with the
Randomizer Additions RFC [1] so this topic hasn't come up yet as the enum
for ext-random is definetely complete.

Best,

George P. Banyard


[PHP-DEV] [RFC] Saner array_(sum|product)()

2023-01-17 Thread G. P. B.
Hello internals,

I would like to start the discussion about the Saner array_(sum|product)()
RFC:
https://wiki.php.net/rfc/saner-array-sum-product

Currently, the array_sum() and array_product() behave differently than
their userland implementation as they ignore arrays and objects, and cast
the remaining types to int/float.
This is in contrast with the behaviour of arithmetic operators that throw
TypeErrors on types and values that are not interpretable as int/float.

This RFC aims to resolve this discrepancy.

Best regards,

George P. Banyard


[PHP-DEV] [RFC] Path to Saner Increment/Decrement operators

2023-01-17 Thread G. P. B.
Hello Internals,

I would like to start the discussion on the Path to Saner
Increment/Decrement operators RFC:
https://wiki.php.net/rfc/saner-inc-dec-operators

The goal of this RFC is to reduce language complexity by making $v++ behave
like $v += 1 and $v-- behave like $v -= 1;

I am expecting the contentious part of the proposal to be the deprecation
of the PERL string increment feature to achieve the aforementioned goal.
However, I believe the benefits of aligning the behaviour of the
increment/decrement operators with addition/subtraction are larger than
keeping support for the PERL increment, which in its current state has
various shortcomings.

Best regards,

George P. Banyard


[PHP-DEV] [PHP_DEV] [RFC] Add file_descriptor() function

2023-01-16 Thread G. P. B.
Hello internals,

I would like to start the discussion about the "Add file_descriptor()
function" RFC:
https://wiki.php.net/rfc/file-descriptor-function

This RFC proposes the addition of the file_descriptor() function to
retrieve the underlying file descriptor of stream if it exists. This is
useful when interacting with a USB device.

Best regards,

George P. Banyard


Re: [PHP-DEV] [RFC] Add SameSite cookie attribute parameter

2023-01-16 Thread G. P. B.
On Sun, 15 Jan 2023 at 16:40, Nicolas Grekas 
wrote:

> Hi George,
>
> There's quite some activity on the HTTP cookies side.
> I read about SameParty and Partitioned attributes recently, see:
> - https://developer.chrome.com/docs/privacy-sandbox/chips/
> - https://github.com/cfredric/sameparty
>
> Maybe we should have a plan that works for these too?
>
> Nicolas
>

Considering those cookie attributes are still in an experimental design
phase and are not universally agreed upon by user agents, I wouldn't want
to add a specific parameter for them.
However, a more generic solution by overloading the options array to
accepts arbitrary key:value pairs might be something to pursue, but this is
out of scope for this RFC as just adding this to the current option array
forces people to use that signature which is less type safe.


On Sun, 15 Jan 2023 at 20:58, Christian Schneider 
wrote:

> Some comments:
> - I am not convinced that we should introduce a third way of providing
> parameters to setcookie(). I don't think this function is used often enough
> in common code to add yet another iteration of the API. Assuming there is 1
> to 2 places in your framework using this I don't think many bugs will go
> unnoticed. Adding a warning to illegal 'samesite' values in $options would
> IMHO be enough if stricter checking is wished for.
>

How is this providing a third way of providing parameters to setcookie()?
As it is, if I want to use named arguments and the typed parameters, I
cannot set the SameSite attribute.
I've heard multiple people waste time due to a SameSite bug because they
made a typo, and it took them way too long to figure out the issue as they
thought they had a server configuration problem.
Adding a warning for invalid values is effectively turning that option into
an Enum, which at this point one might as well have a proper Enum.


> - I don't like the camelCase of $sameSite as this is different from all
> the other parameters, e.g. $expires_or_options (yes, this is a
> pseudo-parameter name, I know) and $httponly. Looking at a couple of
> functions in the standard PHP set I didn't see any $camelCase.
>

ACK, it should probably be in snake case and be $same_site


> - A more generic question: How are Enums handled concerning future
> additions of values vs. BC compatibility? What is the migration plan there
> if one wants to support both old and new PHP versions?
>

The parameter is optional, so I don't understand what migration plan you
need?
If in the, unlikely, event that a new value is added this should be added
as a case in the enum in the next minor version of PHP.
In the, again unlikely, event that a value is deprecated, the corresponding
enum case should also be deprecated following the normal PHP deprecation
process.

I only decided to make this an enum because I deem it very unlikely for a
new valid attribute value to be introduced, the new IETF RFC clarifies and
amends the behaviour of the 3 valid attribute values that have always been
the same since 2016.


> Regards,
> - Chris


On Sun, 15 Jan 2023 at 21:08, Claude Pache  wrote:

> Hi,
>
> Some technical remarks:
> * The new parameter name should be `$samesite` (all lowercase), in order
> to match with the casing of the corresponding key in `$options`.
>

I don't agree with this, the name $samesite all lowercase is far from
optimal, however it should be in snake case ($same_site) in accordance with
the other parameters.


> * I think that you should add the case `SameSite::Omit` (which is the
> current default). This is not only for BC, but also for FC if, for some
> reason, `SameSite: Lax` is replaced by some attribute that supersedes it.
> Or if `SameSite: Lax` becomes the default, and therefore redundant. —
> Having `SameSite::Omit` instead of `null` would mean that it would be
> difficult to miss it by accident.
>

Same idea as in my reply to Chris, it is extremely unlikely that a new
attribute value will be added, moreover the point of using SameSite::Lax as
the default is that it will become the default (it already is in Chrome,
and is gated between a flag in Firefox) as currently in any case you *must*
provide a SameSite values for your cookies as the behaviour end-users are
going to have depends on their browser (e.g. Chrome defaulting to Lax,
while Firefox and Safari still default to None currently) and thus omitting
the value is, IMHO, a bug waiting to happen.


> That said, I am much more interested in being able to add custom cookie
> attributes. Back when SameSite was introduced (on the web, not in PHP), I
> recall that I had to use some hack in order to include them in my session
> cookie (before upgrading to PHP 7.3). The new cookie attributes mentioned
> by Nicolas in the other mail are probably too experimental in order to
> support them officially, but it could be interesting to be able to include
> them nonetheless, e.g. using some `customAttributes` parameter.
>

As said to Nicolas, this is definitely interesting, but 

Re: [PHP-DEV] RFC: rules for #include directives

2023-01-16 Thread G. P. B.
On Mon, 16 Jan 2023 at 12:03, Max Kellermann  wrote:

> Hi,
>
> in the past weeks, I submitted four PRs for cleaning up the #includes
> in the PHP code base:
>
>  https://github.com/php/php-src/pull/10216
>  https://github.com/php/php-src/pull/10220
>  https://github.com/php/php-src/pull/10279
>  https://github.com/php/php-src/pull/10300
>
> I saw that the existing #include directives were inconsistent,
> incomplete and bloated; things just worked by chance, not by design,
> because there were a few headers which just included everything.  I
> wanted to help clean up this mess that had accumulated over two
> decades.
>
> All PRs were welcomed by different reviewers and were merged; there
> was just one minor criticism by Dmitry Stogov who thought the code
> comments explaining many non-obvious #includes should be removed:
>
> https://github.com/php/php-src/pull/10216#issuecomment-1375140255
>
> > I don't think we should include the comments like // for
> > BEGIN_EXTERN_C (and similar). The are good for review only.  I'm
> > indifferent to these changes and don't object.
>
> Yesterday, when a DTrace-specific regression was reported
> (https://github.com/php/php-src/pull/10220#issuecomment-1383035139),
> after which Dmitry asked to revert the whole PR:
>
>  https://github.com/php/php-src/pull/10220#issuecomment-1383658247
>
> Instead, I submitted a trivial fix for the regression
> (https://github.com/php/php-src/pull/10334), which was rejected by
> Dmitry
> (https://github.com/php/php-src/pull/10220#issuecomment-1383706602)
> but confirmed by the original reporter
> (https://github.com/php/php-src/pull/10220#issuecomment-1383802334).
>
> In spite of that, Dmitry demanded to revert all of my #include
> cleanups
> (https://github.com/php/php-src/pull/10220#issuecomment-1383739816):
>
> > I'm asking to revert all these include cleanup commits!  This is
> > just a useless permutation. e.g. 68ada76 adds typedef struct _* that
> > we didn't need before. How is this clearly?
>
> ... which so far only Derick Rethans agreed to:
>
> https://github.com/php/php-src/pull/10220#issuecomment-1383784480
>
> > FWIW, I agree with Dmitry here, and these should all be reverted. It
> > adds nothing but clutter.
>
> Christoph M. Becker performed the revert and suggested doing an RFC
> (https://github.com/php/php-src/pull/10220#issuecomment-1383789100)
> and vote on it.
>
> So this is a first draft of my proposal which I'd like to discuss with
> you:
>
>  https://github.com/php/php-src/pull/10338
>
> Max
>

As the person who has signed off on the PRs and merged them, I am in favour
of these changes and think the revert is a total over reaction.
Before merging the first set, it was asked ahead of time if those changes
were OK, and multiple people were in favour of these changes.
The fact the DTrace build was broken inadvertently and only found out today
is a failure of our CI Build matrix and myself for not knowing this option
nor compiling with it.
Not of the changes.
Moreover, having those sorts of changes be RFCs seems counterproductive as
the only people who care about this are actual core and extensions
developers and this opens the gate for petty RFCs to resolve coding style
disagreements.

Finally, some of the wording used in those PR replies are far from civil
and can push away new contributors to the language.

Best regards,

George P. Banyard


[PHP-DEV] [RFC] Add SameSite cookie attribute parameter

2023-01-14 Thread G. P. B.
Hello Internals,

I would like to start the discussion about the Add SameSite cookie
attribute parameter RFC:
https://wiki.php.net/rfc/same-site-parameter

This proposes to add an optional same site parameter to the setrawcooki(),
setcookie() and session_set_cookie_params() that takes a a value a new
SameSite enum:

enum SameSite {
case None;
case Lax;
case Strict;}


Best regards,

George P. Banyard


Re: [PHP-DEV] Re: A set of 18 functions/changes to improve PHP core

2023-01-06 Thread G. P. B.
On Fri, 6 Jan 2023 at 16:19, Christoph M. Becker  wrote:

> On 06.01.2023 at 17:01, Thomas Hruska wrote:
>
> > Pre-implemented as an extension (sort of) w/ a preliminary test suite to
> > validate the implementations:
> >
> > https://github.com/cubiclesoft/php-ext-qolfuncs
> >
> > I probably violated some coding style guidelines and a few things are
> > bound to ruffle some feathers.  Be gentle?  But all input is generally
> > useful.
>
> Thank you!  At least some of that looks interesting to me.  I'll try to
> have a closer look; maybe filing issues on GH is preferable to
> discussing all the details on this ML.
>
> > I attempted to sign in with my existing PECL credentials on wiki.php.net
> > (user:  cubic) but that didn't work.  Attempting to create a Wiki
> > account for my possibly already existing user resulted in two error
> > messages:  An empty message box and another message box saying "The user
> > could not be created."
> >
> > Requesting RFC karma for user 'cubic' (cu...@php.net) if possible.
> Thanks.
>
> That is not possible, since you already have a PHP account.  Note that
> these are completely separate from PECL accounts (although users usually
> have the same nicknames for both accounts).  To log in to the Wiki, you
> need your PHP account password.  In case you have it forgotten, use
> .
>
> > I assume I'll need to create up to 18 separate RFCs unless one RFC is
> > somehow fine?  Not sure how this should be divvied up yet.
>
> Maybe a compromise is in order, and you can split it into a couple of
> RFCs.  Maybe not; I haven't checked closely so far.
>
> --
> Christoph M. Becker
>

I think splitting into "component" RFCs might make more sense as there are
some string, image, 2D matrices, variable and hash things which are logical
delimitations between these function additions.

I've had a cursory read of the README most seem reasonable, I have my
doubts about the matrix functions (see below) and str_realloc() (but that
might just be the naming and how it's meant to be use with str_splice which
is confusing me)

About str_splice(), why not call it str_modify() as the behaviour seems to
be subtly different from its array equivalent array_splice()?

About matrices:
We already have a couple of issues supporting them that we probably should
address those first, namely that multiplication is assumed to be
commutative, something which is not true for matrices (see
https://github.com/php/php-src/pull/9218)
Hoping I get back to argue about the former, it would make it possible to
use objects and the fact extensions can overload operators to make dealing
with matrices easier.
This is IMHO a better approach than using multidimensional arrays, which
those functions also seem to limit to the (albeit very useful) 2*2 case.

Anyhow, best of luck with the RFCs.

Best regards,

George P. Banyard


[PHP-DEV] What to do with opaque GMP objects which allows instantiation

2022-12-30 Thread G. P. B.
Hello internals,

While working on something, I was using GMP for a test case and creating it
by doing akin to
$o = new GMP(5);

However, this does *not* create a GMP object initialized to 5, rather it
creates and sets the value to 0. This is because it does not define a
constructor, and the proper way to create GMP objects is to use the
procedural gmp_init() API.

GMP was one of the first resource to object conversions which was performed
in PHP 5.6.
More recent resource to object conversions prevent the instantiation of the
opaque object via new by defining a constructor that throws an Error to
point to the procedural API.
(something which would have saved me at least 30 minutes of debug time)

Opened an issue [1] and a PR was made [2] which I requested to target 8.1
as I was considering this a bugfix, however it seems that some people to
initialize the object by just writing new GMP().

Thus, the question, what should we do?
 - Merge as a bug fix for 8.1.
 - Target master (or 8.2.1) and align the behaviour with other opaque
objects.
 - Add a proper constructor that does the same as gmp_init() in master/8.2.1
 - Add a mock constructor that allows instantiating without arguments, but
throws an error if trying to provide an argument.
 - Do nothing

Best regards,

George P. Banyard

[1] https://github.com/php/php-src/issues/10155
[2] https://github.com/php/php-src/pull/10158


Re: [PHP-DEV] PHP build for the wasm32-wasi target

2022-12-28 Thread G. P. B.
On Thu, 22 Dec 2022 at 16:04, Rafael Fernández López 
wrote:

> Hello community,
>
> I am reaching out on behalf of the WasmLabs team at the Office of the CTO
> at VMware.
>
> We have been working on building PHP for WebAssembly, specifically for the
> wasm32-wasi target. WASI [1] is a system interface that allows WebAssembly
> to run on the server side by safely being able to access operating
> system-like features, including files and filesystems, Berkeley sockets,
> clocks and random numbers [2].
>
> WebAssembly in the server side is experiencing a big growth both in
> interest and users. There are other programming language interpreters that
> can be compiled to wasm32-wasi as of today; for example: Python [3] and
> Ruby [4].
>
> We have patches for PHP 7.3.33 and 7.4.32 at the time of writing. Porting
> PHP 8.2.0 is work in progress at this time.
>
> In the interest of getting feedback from the PHP community I am providing
> links to the patches at GitHub. Please, let us know what would be the next
> step to contribute and continue to maintain this work upstream. We look
> forward to work with the PHP community.
>
> If you want to try it out, you can download the binaries we are producing
> in the CI/CD pipeline for 7.3.33 [5] and 7.4.32 [6]. You can use a
> WASI-enabled WebAssembly runtime such as Wasmtime [7] to run PHP compiled
> to wasm32-wasi.
>
> Patches: 7.3.33 [8], 7.4.32 [9][10].
>
> Example execution:
>
> ```
> $ wasmtime -- --dir ~/php-example php-cgi-7.4.32.wasm
> ~/php-example/hello/index.php
> X-Powered-By: PHP/7.4.32
> Content-type: text/html; charset=UTF-8
>
> Hello, world!
> ```
>
> We have already presented some of this work at OSS conferences and events
> like Kubecon and Docker Community days, and have written a few articles
> providing the background to the port, its limitations and what can
> currently be accomplished (including running WordPress!). Please find below
> some links to the talks and articles:
>
> - Porting PHP to WebAssembly using WASI  [11]
> - Running WordPress with WebAssembly using mod_wasm and Apache  [12]
> - WebAssembly: Docker without containers! [13]
> - mod_wasm: Bringing WebAssembly to Apache [14]: this talk builds on top
> of PHP 7.3.33 and shows WordPress running with sqlite on top of Apache,
> thanks to an Apache module (mod_wasm) that is able to execute WebAssembly
> modules.
> - Docker and WebAssembly, better together [15]: this presentation features
> PHP 7.4.32 and explains a bit of the context of this work.
>
>
> Thank you,
> Rafael Fernández López.
>
> [1] https://wasi.dev/
> [2]
> https://github.com/bytecodealliance/wasmtime/blob/03463458e426d4bd0601ebd82e95b668fc982443/docs/WASI-intro.md
> [3] https://docs.python.org/3/whatsnew/3.11.html
> [4] https://www.ruby-lang.org/en/news/2022/12/06/ruby-3-2-0-rc1-released/
> [5]
> https://github.com/vmware-labs/webassembly-language-runtimes/releases/tag/php%2F7.3.33%2B20221124-2159d1c
> [6]
> https://github.com/vmware-labs/webassembly-language-runtimes/releases/tag/php%2F7.4.32%2B20221124-2159d1c
> [7] https://wasmtime.dev/
> [8]
> https://github.com/vmware-labs/webassembly-language-runtimes/tree/main/php/php-7.3.33/patches
> [9]
> https://github.com/vmware-labs/webassembly-language-runtimes/blob/main/php/php-7.4.32/patches/0001-Initial-port-of-7.3.33-patch-to-7.4.32.patch
> [10]
> https://github.com/vmware-labs/webassembly-language-runtimes/blob/main/php/php-7.4.32/patches/0002-Fix-mmap-issues.-Add-readme.patch
> [11] https://wasmlabs.dev/articles/php-wasm32-wasi-port/
> [12] https://wasmlabs.dev/articles/running-wordpress-with-mod-wasm/
> [13] https://wasmlabs.dev/articles/docker-without-containers/
> [14] https://www.youtube.com/watch?v=jXe8kulUscQ
> [15] https://youtu.be/yo30oF1Gflo?t=7361


Hello Rafael,

While this is impressive, this looks very similar to
https://github.com/seanmorris/php-wasm in that it compiles the whole
interpreter to WASM.
Something I personally find of rather limited utility, even more so when in
this iteration when the current interpreter already works server side.
Mainly because I don't really see what benefits this gives to the language.
Wouldn't this be just slower than executing the interpreter currently?

I spent some time a couple of months ago thinking about trying to actually
compile PHP code to WASM directly *without* compiling the whole
interpreter, be that either by only compiling the existing opcodes to WASM
or compiling directly from PHP to WASM. Now, this is clearly several
magnitudes harder as a problem to solved, but is one that seems to have
more benefits than just compiling the interpreter.
Having a lot of the standard library, actually written in PHP, would be a
big benefit here. And would make this project similar to RPython (what PyPy
uses to compile its Python subset into C).

The other issue is that I don't know of a way to run WASM binaries from PHP
at the moment, from a quick search and from what I remembered Tuleap [1]
achieved this by executing the 

Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments

2022-11-27 Thread G. P. B.
On Sat, 26 Nov 2022 at 23:06, Marco Pivetta  wrote:

> On Sat, 26 Nov 2022, 23:56 G. P. B.,  wrote:
>
>> As all the invariants will be maintained in the child classes
>>
> The invariant here is the immutability from a behavioural PoV.
> LSP has 100% been broken in that example.
>

I've gone back to the example, and if we are going to use bad code to
"argue" against certain features, then let's use this argument to its
natural conclusion.

But before that, we really need to get everyone clear on what LSP is.
It is a subtyping relationship that states:
  if a property P on an object X of type T is provable to be true, then for
an object Y of type S to be considered a proper subtype of T, this property
P must also be true for all object Y of type S.
Let's rephrase this slightly in terms more appropriate to PHP:
  if a characteristic P on an instance X of class T is provable to be true,
then for an instance Y of class S to be considered a proper subclass of T,
this characteristic P must also be true for all instances Y of class S.

Having this in mind, let me show how easy it is to break LSP:
class T {
public function __construct(public readonly int $i) {}
public function add(self $t): self { return new self($this->i + $t->i);
}
}

$op1 = new T(5);
$op2 = new T(14);
$r   = $op1->add($op2);
var_dump($r);

class S extends T {
public function add(parent $t): self { return new self($this->i -
$t->i); }
}

$op1 = new S(5);
$op2 = new T(14);
$r   = $op1->add($op2);
var_dump($r);

Clearly, the characteristic that we can "add two T together" has been
broken in class S.
This is all to say, the type system can only provide a level of code
*correctness*, and that remains even in highly sophisticated type systems.
If you bear with me and the introduction of the following imaginary type
dependent syntax: Matrix which declares a Matrix type which is an n by
m matrix.
Thus, we can write the signature of a function that does define matrix
multiplication in the following way:
  function multiplication(Matrix $op1, Matrix $op2): Matrix
Which would guarantee us that the 2 matrices provided can indeed be
multiplied together as they have compatible dimensions, and it even gives
us the dimensions of the output matrix.
However, the one thing it cannot guarantee is that the operation we are
actually doing is a matrix multiplication, I could just as well write a
function that returns a new n by k matrix filled with zeroes.

How can one be sure that a subclass will not alter the behaviour? By making
the method final.
That's mainly why personally I think final by default, and using
composition over inheritance, is "better".
And I think from what we've seen in language design over the last couple of
decades is the OO inheritance is just a bad way to do subtyping and
something like Haskell type-classes are way better. [1] But that ship has
somewhat sailed for PHP.
The LSP checks PHP (and for the matter any type system) can do and does, is
checking the logical correctness of the implementation, not behavioural
correctness that onus still remains on the developer.

Now, let's have another look at your example:
/* readonly */ class ImmutableCounter {
public function __construct(private readonly int $count) {}
public function add1(): self { return new self($this->count + 1); }
public function value(): int { return $this->count; }
}

IMHO the fact that the count property is private is the biggest issue, just
changing it to protected would solve most of the issues, as now you've
given the tools to PHP to verify and ensure this constraint holds for the
whole class hierarchy.
But more importantly, it means I don't need to reimplement the *whole*
parent class and (risk me messing up)/(keeping in sync) the behaviour of
the parent class if I want to do something like:

class SubstractableImmutableCounter extends ImmutableCounter {
public function sub1(): self { return new self($this->count - 1); }
}

As it's easy to mess up even just writing a function properly that does the
expected behaviour, I suppose we should remove the ability for users to
write them, as they cannot be trusted with.
So please, can we stop using the argument that this proposal breaks LSP if
you write dumb code, as that's already the case, and it doesn't if you
provide PHP with the actual information it needs to verify in child classes.

Moreover, if one expects a readonly class to be immutable, then that is
already not the case if one of the properties is a non-readonly object.
It is also not necessarily stateless, as can be seen in the following
example: https://3v4l.org/A2Lbs

readonly class ImmutableBadNumberGenerator {
public function __construct(private readonly int $i) {}

public function generate(): int {
static $j = 1;
++$j;
return $this->i + $j;
}
}

$o = new ImmutableBadNumberGenerator(10);
var_dump($o->generate()

Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments

2022-11-26 Thread G. P. B.
On Thu, 17 Nov 2022 at 14:47, Marco Pivetta  wrote:

> Hey Máté,
>
> On Tue, 15 Nov 2022 at 07:30, Máté Kocsis  wrote:
>
> > Hi Everyone,
> >
> > Following Nicolas' thread about "Issues with readonly classes" (
> > https://externals.io/message/118554), we created an RFC to fix two
> issues
> > with the readonly behavior: https://wiki.php.net/rfc/readonly_amendments
> >
> >
> Since I was reviewing https://github.com/lcobucci/jwt/pull/979 today, I
> had
> to put the "why immutability is an LSP requirement" in examples.
>
> As explained in
> https://github.com/lcobucci/jwt/pull/979#discussion_r1025280053, a
> consumer
> relying on a `readonly` implementation of a class probably also expects
> replacement implementations as read-only.
> I am aware that we lack the `readonly` marker at interface level (would be
> really neat), but the practical example is as follows:
>

>
> ```php
> 
> /* readonly */ class ImmutableCounter {
> public function __construct(private readonly int $count) {}
> public function add1(): self { return new self($this->count + 1); }
> public function value(): int { return $this->count; }
> }
>
> // assuming the proposed RFC is in place, this is now possible:
> class MutableCounter extends ImmutableCounter {
> public function __construct(private int $count) {}
> public function add1(): self { return new self(++$this->count); }
> public function value(): int { return $this->count; }
> }
>
> $counter1 = new ImmutableCounter(0);
> $counter2 = $counter1->add1();
> $counter3 = $counter2->add1();
>
> var_dump([$counter1->value(), $counter2->value(), $counter3->value()]);
>
> $mutableCounter1 = new MutableCounter(0);
> $mutableCounter2 = $mutableCounter1->add1();
> $mutableCounter3 = $mutableCounter2->add1();
>
> var_dump([$mutableCounter1->value(), $mutableCounter2->value(),
> $mutableCounter3->value()]);
> ```
>
> This prints ( https://3v4l.org/IDhRY )
>
> ```
> array(4) {
>   [0]=>
>   int(0)
>   [1]=>
>   int(1)
>   [2]=>
>   int(2)
> }
> array(4) {
>   [0]=>
>   int(1)
>   [1]=>
>   int(2)
>   [2]=>
>   int(2)
> }
> ```
>
> I took a simplified example on purpose, just to demonstrate the problem
> with `readonly` disappearing in child classes.
>
> That's really really confusing, buggy, and, from a consumer PoV, broken
> (LSP violation too, transitively).
>

Removing the readonly restriction for a child class is *not* an LSP
violation.
As all the invariants will be maintained in the child classes (i.e. all the
properties of the parent class remain readonly).
Moreover, a consumer of the base class can only rely on the API defined by
said class, as such it will not be aware of any mutable properties defined
by a child class.

LSP is really not about the class hierarchy/subtypes itself, but the
relation of input data and output data of the operations allowed on a type
(i.e. method calls or property access), and because readonly is not the
same as immutability there is no theoretical issue.

Now, I can agree that it can be slightly surprising that a class which has
a writeable property extends from one which only has readonly ones, but one
can already immitate this by declaring every property readonly but not the
class.

Best regards,

George P. Banyard


[PHP-DEV] Removing OS2 specific code

2022-11-15 Thread G. P. B.
Hello internal,

While working on a bug, I've encountered some compatibility code for OS2,
which I've planned to remove, as the last release of OS2 was in December
2001.

This is to ask if anyone has any objections to removing compatibility for
OS2.

Best regards,

George P. Banyard


[PHP-DEV] Should the engine be able to pass variables by reference of userland functions

2022-10-19 Thread G. P. B.
Hello internals,

While working on cleaning up the FCI/FCC API I stumbled upon the
zend_fcall_info_args_ex()
function.
This function allows the engine to pass "variables" to userland functions
that expect a by-ref parameter.
This is currently only used within php-src in PDO when calling the
constructor of classes where the result of a statement should be stored in
PDO::FETCH_CLASS mode.

I am wondering if this makes any sense to continue support in general, and
thus in this specific case. As every other engine call to userland function
passes by value.

A PR of this change is located:
https://github.com/php/php-src/pull/9725/files

What are your opinions?

Best regards,

George P. Banyard


Re: [PHP-DEV] Experimental features

2022-10-07 Thread G. P. B.
On Fri, 7 Oct 2022 at 07:57, Christian Schneider 
wrote:

> But now to my main point: You are talking about the most simple and
> isolated case, a new function.
>

I agree, and for most intent and purposes a function can be polyfilled in
userland. So this is the least interesting case.

However, I don't think this approach is that useful. To give an example,
for DNF types the implementation is based on a change in how iterable works
in PHP.
It went from a pseudo-type to a compile time alias, as this massively
simplified variance and LSP checks.
I would not want such a change to land in a patch release especially as it
turned out there was an unintended BC break with the compile time
redundancy which only got fixed in PHP8.2RC2.


> Your model may work fine with that but as soon as we are talking something
> like a syntax change we have some additional problems:
> - Enabling it with a prefix obviously does not work, something like
> declare() is definitely needed
> - Maintaining such changes to the engine / parser could turn out messy,
> e.g. lots of "if (experimental_x)"-like code in the core
> - The test suite would possibly have to deal with an exponential explosion
> of experimental feature combinations to make sure they don't interfere with
> each other.
>
> All in all I'm not sold yet that the benefit outweighs the cost of this
> approach.
>

Ditto.

When Nikita proposes editions back in 2020 (
https://github.com/nikic/php-rfcs/blob/language-evolution/rfcs/-language-evolution.md)
the point about them was mainly about backwards-incompatible changes, not
new features.
Because in general new features don't have the same issues, especially
since the RFC process (although not without its issues) does force thinking
about the design of it instead of just shoving something in php-src
randomly as in the before days.

Best regards,

George P. Banyard


Re: [PHP-DEV] Re: Increase maximum size of an uploaded file to 50Mbyte

2022-09-07 Thread G. P. B.
On Wed, 7 Sept 2022 at 19:37, juan carlos morales <
dev.juan.mora...@gmail.com> wrote:

> By the Way... This needs an RFC right?
>

No it does not.

Best regards,

George P. Banyard


Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

2022-09-06 Thread G. P. B.
On Mon, 5 Sept 2022 at 18:20, Tim Düsterhus  wrote:

> Hi
>
> I've now written up an RFC as a follow-up for the "What type of
> Exception to use for unserialize() failure?" thread [1]:
>
> 
>
> RFC: Improve unserialize() error handling
> https://wiki.php.net/rfc/improve_unserialize_error_handling
>
> Proof of concept implementation is in:
>
> https://github.com/php/php-src/pull/9425
>
> Discussion period for that RFC is officially opened up.
>
> 
>
> The primary point of discussion in the previous mailing list thread and
> in the PR comments is whether unserialize() should continue to emit
> E_WARNING or whether that should consistently be changed to an
> Exception. As of now I plan to explicitly vote on this and the RFC
> contains some opinions on that matter.
>
> Best regards
> Tim Düsterhus
>
> [1] https://externals.io/message/118311
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>
Thank you Tim for the thorough investigation.
I didn't know how bad the situation was in regards to unserialization.
So I'm now tending in favour of promoting the notice/warnings to exceptions
as it is currently extremely hard to handle the behaviour correctly.

Best regards,

George P. Banyard


Re: [PHP-DEV] What type of Exception to use for unserialize() failure?

2022-08-01 Thread G. P. B.
On Fri, 29 Jul 2022 at 18:17, Tim Düsterhus  wrote:

> Hi!
>
> I'm currently in the process of cleaning up the error handling of the
> new ext/random and now came across the implementation of __unserialize().
>
> Looking through the source code for existing examples I found that the
> following is used:
>
> - ext/gmp: Exception
> - ext/hash: Exception
> - ext/date: Error
> - ext/spl: UnexpectedValueException
> - ext/random: Currently Exception.
>
> … all of those with different error messages. unserialize() itself will
> emit a regular 'Notice' (yes, you read that right) when encountering
> garbage data.
>
> In the end I've opted to follow ext/date's lead and created a PR
> changing ext/random to use an Error, as unserialization failures likely
> mean that you are unserializing untrusted data which you should not do
> in the first place. Also any errors during unserializing are not really
> recoverable: You can't go and fix up the input string.
>
> The PR can be found here:
>
> https://github.com/php/php-src/pull/9185
>
> During review cmb noted that using an 'Error' here might not be the best
> choice, as while it is likely to be a programmer error if unserializing
> fails, we do not want to educate users to catch(Error).
>

I probably have a very different philosophy on this as I don't mind users
catching Errors, but it probably be mostly done by Framework/libraries
doing some funky stuff.
And passing user input which can fail to unserialize is not really a
programming error and something that can be "expected" to happen, I agree
that an Exception does make more sense here.


> As the existing behavior already is inconsistent and the Notice really
> should be upgraded to something … more alarming in the future, I'm
> putting this topic up for discussion on the list.
>
> My suggested path forward would be:
>
> For 8.2:
>
> - Update ext/random to use the ext/date wording (I like that one best):
>
> "Invalid serialization data for  object."
>
> - Revert the change from Exception to Error in my ext/random PR #9185.
>

Those make sense to me


> - Update ext/date to Exception (the Error for __unserialize() was added
> in 8.2, but there already was an Error for __wakeup() before).
>

I'm not a super fan of this, both __wakeup and __unserialize should use the
same one.
As far as I can tell the reason __wakeup() is still defined is if someone
called it explicitly.
But in any case if someone was handing an unserialisation failure (that was
handled by __wakeup prior to 8.2) and caugh an Error explicitly this would
now break.
Thus I'd prefer handling this with the rest after 8.2


> For whatever comes after 8.2:
>
> - Add a new UnserializationFailureException extends Exception (or
> whatever color the bikeshed should have). Any existing catch blocks
> should still match, as Exception is more general. It would break for
> ext/spl, though. Unless UnserializationFailureException extends
> UnexpectedValueException.
>

As we would be breaking BC with ext/date anyway I think having it just
extend Exception would be fine


> - Add a helper function that throws this Exception with a consistent
> wording.
> - Upgrade unserialize() from Notice to UnserializationFailureException.
>

Notice to Exception might be a big jump, E_WARNING definitely an then
promote to an Exception in 9.0 is probably more in line with what we did
for 7.x/8.0

George P. Banyard


  1   2   3   4   >