Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:115653
MIME-Version: 1.0
References: <CAMrTa2EO-5nyoCYkHhMVxQ4nucRoHJj0=M=W22kUaW0jpHkHLg@mail.gmail.com>
In-Reply-To: <CAMrTa2EO-5nyoCYkHhMVxQ4nucRoHJj0=M=W22kUaW0jpHkHLg@mail.gmail.com>
Date: Sat, 7 Aug 2021 18:41:52 +0100
Message-ID: <CA+kxMuRo5WOd1Brdr-eXvc-XWJ3j-Ju1_cZ1+1uBDgio5Exgkw@mail.gmail.com>
To: Jordan LeDoux <jordan.ledoux@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Revisiting Userland Operator Overloads
From: Danack@basereality.com (Dan Ackroyd)

On Fri, 6 Aug 2021 at 17:34, Jordan LeDoux <jordan.ledoux@gmail.com> wrote:
>
> I was considering working from Jan's previous
> implementation with these basic changes:

That may not be the best place to start, and that something more
similar to the previous (unvoted) RFC may be better:
https://wiki.php.net/rfc/operator-overloading

In particular, the problems with the approach in
https://wiki.php.net/rfc/userspace_operator_overloading is that having
a function return a special constant when unsupported types are used
is...just not the right approach. The function shouldn't be run at all
if the types aren't supported.

Having to understand a block of code, to figure out what types it
accepts, makes code hard to reason about.

Although the union types RFC had passed when the userspace operator
overloading RFC went to vote, the ability to use union types wasn't
taken advantage of in the RFC.

Listing the types supported as a union type for the parameter like:

class Order implements __AddInterface {
    public function __add(Product|Discount $rhs) {...}
}

is easier to reason about, and would throw a type-error consistent
with how unacceptable types would behave for other function calls.

I personally find having the functions be defined as static, with the
first parameter being the left hand side, a bit weird. It makes the
code quite a bit harder to read imo*.

> I feel like the right place to start is to limit
> the RFC to only the mathematical operators at first.
> ...something like MathObjectInterface

I agree with Larry and Scott, one method per interface is a better
approach. But defining all the interfaces in one go is probably
better, even if the implementation only supports using some of them.

Doing it like that means that when new operators are implemented as
overloadable in PHP 8.3, people would be able to write library code
that takes advantage of those new operators that works on both 8.2 and
8.3, without users having to define the missing interfaces themselves,
even if their app code isn't able to take advantage of the newly
overloadable operators until they upgrade to 8.3.

cheers
Dan
Ack

* This was mentioned in the RFC: "By passing both operands to the
handler, it can decide between the cases on non-commutative operators
($a / 2 vs. 2 / $a), which would be more difficult when only the
=E2=80=9Cother=E2=80=9D operand (besides $this) is passed.", but I can't se=
e an
example of that, and I don't understand it. Perhaps a better
explanation would be enough.

Though also the behaviour for associativity mentioned in the previous
RFC  https://wiki.php.net/rfc/operator-overloading#associativity isn't
mentioned in the 2020 RFC and there may be a can of worms in that.