Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:126608 X-Original-To: internals@lists.php.net Delivered-To: internals@lists.php.net Received: from php-smtp4.php.net (php-smtp4.php.net [45.112.84.5]) by qa.php.net (Postfix) with ESMTPS id E59891A00BC for ; Thu, 6 Mar 2025 21:39:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=php.net; s=mail; t=1741296999; bh=afYyIMgiJsvhd8wIlvvpBt2y+DPWR9qX259Z1y/G3Zc=; h=Date:From:Cc:In-Reply-To:References:Subject:From; b=VFxJwH3JGYFHomMv5VVC56/jjqzK/e59+1YGL7LQLptPnvxhaA0jFCJYDY5RVFzJy QrGd20t+crJpq0Snoznu4CzkIwRh/riKZOgLkcp9n8I9VgjUNSvWxxvxmzX0VHQe5N Wd2KWZP6vruLkaT3iAqx91f7bzrxxnCXSPjYKtQXxHiTGm+zHV1s3fnf60g+OSAsaN Rn5niwx8t/RSMJ+QIQcL5Vfk89y96nRDtdANlgR90U8kJ/RfKJvnCO93sC6sjqgl3f m5fARm/YjEHpNaznjbT416OcFS6DKb4WWgZe/thfw5EesXhBP/3+YcRffG1Fz80/wI TevmrUWcJy8qg== Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id DD526180086 for ; Thu, 6 Mar 2025 21:36:37 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=-3.6 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,DMARC_MISSING,HTML_MESSAGE, MISSING_HEADERS,RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS, SPF_PASS autolearn=no autolearn_force=no version=4.0.0 X-Spam-Virus: No X-Envelope-From: Received: from fout-b8-smtp.messagingengine.com (fout-b8-smtp.messagingengine.com [202.12.124.151]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Thu, 6 Mar 2025 21:36:37 +0000 (UTC) Received: from phl-compute-11.internal (phl-compute-11.phl.internal [10.202.2.51]) by mailfout.stl.internal (Postfix) with ESMTP id A8D5A1140175 for ; Thu, 6 Mar 2025 16:39:12 -0500 (EST) Received: from phl-imap-09 ([10.202.2.99]) by phl-compute-11.internal (MEProxy); Thu, 06 Mar 2025 16:39:12 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bottled.codes; h=cc:cc:content-type:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:subject:subject:to; s=fm2; t=1741297152; x=1741383552; bh=rIP7LoocbPBlovFQEscKW95Qw+AJ++1iY9m46u2EJD0=; b=CcBWj7Gmws/l 6XzgWZG9X8G1Z6pG635oz0bKj1OKYZCm0ka5PYVD058tahb1ghuCcqLhGYag/xwx CRlEe9UGBS3PEjdAOZhEAHTbfS8nZuXwezv5c508zoFDRQoUrgXxYCKt1r7pzvay /akE7DXMPwJEbcCiGCfYQLQp62vjNsHtKPiK1lS1sq1kTIlSRW4nwfYX/Kw9R+p4 /ONvYnf3q14NzCc5uX4HhpqLo4wkkIV/CcN3NoL9Se2cHzbqCl59wzJfRBT3+2AF nFuCWCr9bXsf8s9Hdisl9YgEGcDFCaFSrGQF8bMqcDi9qubXTlr0pD7iVqdaIzAl ApY1r1C81A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t= 1741297152; x=1741383552; bh=rIP7LoocbPBlovFQEscKW95Qw+AJ++1iY9m 46u2EJD0=; b=R9bIp3Y620V1BPExUylIj5UMZfQhPQL9erbJf+1FXW+VhV+8ynu c2r7fSu9xRDY7DNgQ0gxYtJRtfkClUV0q5ugupEcW3WvGeszXwEDwuZZKRq4YVTG b4QxcVPaRHQyzpunc02gKKuufGWSj3bYjWtbNYn0jVDig6yWdZAOjfXcseX8UHp9 bhY4R62iCH/+dQm7PaPeOOljBCw+kdcCC88VfL0u8UJvqCdRMqdyKytbVuyGyi7q hVPDUKma1/KY3frUFgk7+qKJEEqC6Gd1TOpEVLrFkzJnO92ZXstXZ4umCDB6TAMQ O0TvVuV8EDTSOQ+UcYaSSV9VSeEavd7Yttg== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgddutdekkeehucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenuchmihhsshhinhhguc fvqfcufhhivghlugculdeftddmnecujfgurhepofggfffhvefkjghfufgtsegrtderreer tdejnecuhfhrohhmpedftfhosgcunfgrnhguvghrshdfuceorhhosgessghothhtlhgvug drtghouggvsheqnecuggftrfgrthhtvghrnhepteeffefhvefhjeeuiedtueegkeeivefh hfehheeiffdtfeelffekvdeujefftddunecuffhomhgrihhnpegvgihtvghrnhgrlhhsrd hiohenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehr ohgssegsohhtthhlvggurdgtohguvghspdhnsggprhgtphhtthhopedupdhmohguvgepsh hmthhpohhuthdprhgtphhtthhopehinhhtvghrnhgrlhhssehlihhsthhsrdhphhhprdhn vght X-ME-Proxy: Feedback-ID: ifab94697:Fastmail Received: by mailuser.phl.internal (Postfix, from userid 501) id 276BB780068; Thu, 6 Mar 2025 16:39:12 -0500 (EST) X-Mailer: MessagingEngine.com Webmail Interface Precedence: bulk list-help: list-post: List-Id: internals.lists.php.net x-ms-reactions: disallow MIME-Version: 1.0 Date: Thu, 06 Mar 2025 22:38:51 +0100 Cc: internals@lists.php.net Message-ID: <678f17ad-345c-452a-9636-f4c9c1a7acf2@app.fastmail.com> In-Reply-To: References: <076001db8e34$41cab990$c5602cb0$@glaive.pro> Subject: Re: [PHP-DEV] RFC: short and inner classes Content-Type: multipart/alternative; boundary=a1985e0130764294ba37b7ce223a1f10 From: rob@bottled.codes ("Rob Landers") --a1985e0130764294ba37b7ce223a1f10 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On Thu, Mar 6, 2025, at 09:04, Tim D=C3=BCsterhus wrote: > Hi >=20 > Am 2025-03-06 07:23, schrieb Rob Landers: > > So, technically, they aren=E2=80=99t required to be in the same RFC;= but also,=20 > > they complement each other very well. >=20 > They really should be separate RFCs then. Your RFC text acknowledges=20 > that in the very first sentence: =E2=80=9Ctwo significant enhancements= to the=20 > language=E2=80=9D. Each individual proposal likely has sufficient bike= -shedding=20 > potential on its own and discussion will likely get messy, because one=20 > needs to closely follow which of the two proposals an argument relates=20 > to. I put a lot of thought into this issue off and on, all day. I've decided= to remove short syntax from the RFC and focus on inner classes. If this= passes, then I will propose it as a separate RFC. Introducing them conc= urrently makes little sense in light of the feedback I have gotten so fa= r, and it is turning out that there is much more to discuss than I initi= ally expected. Thus, I will skip replying about short classes. >=20 > As for the =E2=80=9CInner classes=E2=80=9D proposal: >=20 > - =E2=80=9Cabstract is not allowed as an inner class cannot be parent = classes.=E2=80=9D=20 > - Why? This is mostly a technical reason, as I was unable to determine a gramma= r rule that didn't result in ambiguity. Another reason is to ensure enca= psulation and prevent usages outside their intended scope. We can always= add it later. > - =E2=80=9Ctype hint=E2=80=9D - PHP does not have type hints, types ar= e enforced. You=20 > mean =E2=80=9CType declaration=E2=80=9D. Thank you for pointing this out! I learned something new today! I've upd= ated the RFC. > - =E2=80=9Cthis allows you to redefine an inner class in a subclass, a= llowing=20 > rich hierarchies=E2=80=9D - The RFC does not specify if and how this i= nteracts=20 > with the LSP checks. It doesn't affect LSP. I've updated the RFC accordingly. On Thu, Mar 6, 2025, at 20:08, Niels Dossche wrote: > Hi Rob >=20 > Without looking too deep (yet) into the details, I'm generally in favo= r of the idea. > What I'm less in favor of is the implementation choice to expose the i= nner class as a property/const and using a fetch mode to grab it. > That feels quite weird to me honestly. How did you arrive at this choi= ce? >=20 > Kind regards > Niels It's a slightly interesting story about how I arrived at this particular= implementation. If you noticed the branch name, this is the second impl= ementation. The first implementation used a dedicated list on the class-= entry for inner classes. Since I wanted to prevent static property/const= s from being declared with the same name, I had just set it to a string = of the full class name as a placeholder. That implementation also requir= ed some pretty dramatic OPcache changes, which I didn't like. At one poi= nt, I went to add the first test that did `new Outer::Inner()` and the t= est passed...=20 You can imagine my surprise to see a test pass that I had expected to fa= il, and it was then that I went into the details of what was going on. A= ny `new ClassName` essentially results in the following AST: ZEND_AST_NEW -- ZEND_AST_ZVAL -- "ClassName" -- (... args) The original grammar, at the time, was to reuse the existing static prop= erty access AST until I could properly understand OPcache/JIT. My change= had resulted in (approximately) this AST: ZEND_AST_NEW -- ZEND_AST_ZVAL -- ZEND_AST_STATIC_PROP -- "Outer::Inner" -- (... args) Which, effectively resulted in emitting opcodes that found the prop + st= ring value I happened to put there as a placeholder until I figured out = a better solution, handling autoloading properly and everything. This pr= etty much negated all efforts up to that point, and I was stunned. So, I branched off from an earlier point and eventually wrote the versio= n you see today. It's 1000x simpler and faster than the original impleme= ntation (literally), since it uses all pre-existing (optimized)) infrast= ructure instead of creating entirely new infrastructure. It doesn't have= to check another hashmap (which is slow) for static props vs. constants= vs. inner classes.=20 In essence, while the diff can be improved further, it is quite simple; = the core of it is less than 500 lines of code. I'd recommend leaving any comments about the PR on the PR itself (or via= private email if you'd prefer that). I'm by no means an expert on this = code base, and if it is not what you'd expect, being an expert yourself,= I'd love to hear any suggestions for improvements or other approaches. On Thu, Mar 6, 2025, at 20:33, Larry Garfield wrote: > My biggest concern with this is that it makes methods and short-classe= s mutually incompatible. So if you have a class that uses short-syntax,= and as it evolves you realize it needs one method, sucks to be you, now= you have to rewrite basically the whole class to a long-form constructo= r. That sucks even more than rewriting a short-lambda arrow function to= a long-form closure, except without the justification of capture semant= ics. I literally fell out of my chair laughing. Thanks for that, and it is tr= ue. I look forward to discussing this further! > ## Inner classes >=20 > I'm on board with the use case. What I'm not sure on is inner classes= vs file-private visibility, something that Ilija was working on at one = point and Micha=C5=82 Brzuchalski suggested in his post. Both solve lar= gely the same problem with different spelling. >=20 > Arguably, inner classes have fewer issues with current autoload conven= tions. I must ponder this further. Indeed! See: https://externals.io/message/126331#126337 >=20 > > However, no classes may not inherit from inner classes, but inner cl= asses may inherit from other classes, including the outer class.=20 >=20 > I think you have one too many negatives in that sentence. Thank you, this is fixed! =E2=80=94 Rob --a1985e0130764294ba37b7ce223a1f10 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable
On Thu, Mar 6, = 2025, at 09:04, Tim D=C3=BCsterhus wrote:
Hi

Am 2025-0= 3-06 07:23, schrieb Rob Landers:
> So, technically, the= y aren=E2=80=99t required to be in the same RFC; but also, 
> they complement each other very well.

They really should be separate RFCs then. Your RFC text acknowled= ges 
that in the very first sentence: =E2=80=9Ctwo si= gnificant enhancements to the 
language=E2=80=9D. Eac= h individual proposal likely has sufficient bike-shedding 
potential on its own and discussion will likely get messy, because= one 
needs to closely follow which of the two propos= als an argument relates 
to.

I put a lot of thought into this issue off and on, all= day. I've decided to remove short syntax from the RFC and focus on inne= r classes. If this passes, then I will propose it as a separate RFC. Int= roducing them concurrently makes little sense in light of the feedback I= have gotten so far, and it is turning out that there is much more to di= scuss than I initially expected.

Thus, I wi= ll skip replying about short classes.


As for the =E2=80= =9CInner classes=E2=80=9D proposal:

- =E2=80= =9Cabstract is not allowed as an inner class cannot be parent classes.=E2= =80=9D 
- Why?

<= div>This is mostly a technical reason, as I was unable to determine a gr= ammar rule that didn't result in ambiguity. Another reason is to ensure = encapsulation and prevent usages outside their intended scope. We can al= ways add it later.

- =E2=80=9Ctype hint=E2=80=9D - PHP does not have t= ype hints, types are enforced. You 
mean =E2=80=9CTyp= e declaration=E2=80=9D.

Thank = you for pointing this out! I learned something new today! I've updated t= he RFC.

- =E2=80=9Cthis allows you to redefine an inner class in a s= ubclass, allowing 
rich hierarchies=E2=80=9D - The RF= C does not specify if and how this interacts 
with th= e LSP checks.

It doesn't affec= t LSP. I've updated the RFC accordingly.

On= Thu, Mar 6, 2025, at 20:08, Niels Dossche wrote:
Hi Rob

Without looking too deep (yet) into the details, I'm generally in fav= or of the idea.
What I'm less in favor of is the implement= ation choice to expose the inner class as a property/const and using a f= etch mode to grab it.
That feels quite weird to me honestl= y. How did you arrive at this choice?

Kind = regards
Niels

It= 's a slightly interesting story about how I arrived at this particular i= mplementation. If you noticed the branch name, this is the second implem= entation. The first implementation used a dedicated list on the class-en= try for inner classes. Since I wanted to prevent static property/consts = from being declared with the same name, I had just set it to a string of= the full class name as a placeholder. That implementation also required= some pretty dramatic OPcache changes, which I didn't like. At one point= , I went to add the first test that did `new Outer::Inner()` and the tes= t passed... 

You can imagine my surpri= se to see a test pass that I had expected to fail, and it was then that = I went into the details of what was going on. Any `new ClassName` essent= ially results in the following AST:

ZEND_AS= T_NEW
-- ZEND_AST_ZVAL
    -- "Cla= ssName"
-- (... args)

The ori= ginal grammar, at the time, was to reuse the existing static property ac= cess AST until I could properly understand OPcache/JIT. My change h= ad resulted in (approximately) this AST:

ZE= ND_AST_NEW
-- ZEND_AST_ZVAL
    --=  ZEND_AST_STATIC_PROP
        -- = "Outer::Inner"
-- (... args)

= Which, effectively resulted in emitting opcodes that found the prop + st= ring value I happened to put there as a placeholder until I figured out = a better solution, handling autoloading properly and everything. This pr= etty much negated all efforts up to that point, and I was stunned.

So, I branched off from an earlier point and eve= ntually wrote the version you see today. It's 1000x simpler and faster t= han the original implementation (literally), since it uses all pre-exist= ing (optimized)) infrastructure instead of creating entirely new infrast= ructure. It doesn't have to check another hashmap (which is slow) for st= atic props vs. constants vs. inner classes.

In essence, while the diff can be improved further, it is quite simple= ; the core of it is less than 500 lines of code.

I'd recommend leaving any comments about the PR on the PR itself (= or via private email if you'd prefer that). I'm by no means an expert on= this code base, and if it is not what you'd expect, being an expert you= rself, I'd love to hear any suggestions for improvements or other approa= ches.

On Thu, Mar 6, 2025, at 20:33, Larry = Garfield wrote:
My biggest conce= rn with this is that it makes methods and short-classes mutually incompa= tible.  So if you have a class that uses short-syntax, and as it ev= olves you realize it needs one method, sucks to be you, now you have to = rewrite basically the whole class to a long-form constructor.  That= sucks even more than rewriting a short-lambda arrow function to a long-= form closure, except without the justification of capture semantics.
=

I literally fell out of my chair = laughing. Thanks for that, and it is true. I look forward to discussing = this further!

## Inner classes

I'm on b= oard with the use case.  What I'm not sure on is inner classes vs f= ile-private visibility, something that Ilija was working on at one point= and Micha=C5=82 Brzuchalski suggested in his post.  Both solve lar= gely the same problem with different spelling.

<= div>Arguably, inner classes have fewer issues with current autoload conv= entions.  I must ponder this further.


> However, no classes may not inherit from inner classes, but= inner classes may inherit from other classes, including the outer class= . 

I think you have one too many negat= ives in that sentence.

Thank y= ou, this is fixed!

=E2=80=94= Rob
--a1985e0130764294ba37b7ce223a1f10--