Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:127909 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 lists.php.net (Postfix) with ESMTPS id CFDA91A00BC for ; Sat, 5 Jul 2025 21:03:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=php.net; s=mail; t=1751749315; bh=cW5LKegZ73GvcDIhRsvIJ7wpevTTpTZRNLgaIesdVAo=; h=Date:From:To:Cc:In-Reply-To:References:Subject:From; b=gAQ9AsxxqUwkKhTWymKkHL18+jlmC8jA6cYbDYjQB/UsSbjKw/Al3M8RYC6xJN49L mbQ0gcUijA/xMoLqGv73pyUwkplbbx2dpxjfEClTdm7dRJWoME5KETrRmOo8QstETI HOEtuTpV9e1/B0zYcAsyP0bRgWQ8GEInVjBKXET4DZEGDZ1GjuDTPwBXR9hhebnFw1 gKWGj1OnE5RdE2GfSfZAsj1l6Qe0zXQghIb3sxmKDUfVOatpQLFPwysdLaHQA9a+1E 2SFTeZF72ByJ3i+kRx7G2QPtoGYvPf3TVyoaLymBqYrBi9Lnn+czofADlX4ttI8Qy+ QiOmUgExVsxKg== Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id B5C0018020B for ; Sat, 5 Jul 2025 21:01:54 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=-3.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,DMARC_MISSING,HTML_MESSAGE, RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=no autolearn_force=no version=4.0.1 X-Spam-Virus: Error (Cannot connect to unix socket '/var/run/clamav/clamd.ctl': connect: Connection refused) X-Envelope-From: Received: from fout-a3-smtp.messagingengine.com (fout-a3-smtp.messagingengine.com [103.168.172.146]) (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 ; Sat, 5 Jul 2025 21:01:54 +0000 (UTC) Received: from phl-compute-05.internal (phl-compute-05.phl.internal [10.202.2.45]) by mailfout.phl.internal (Postfix) with ESMTP id 0A108EC0241; Sat, 5 Jul 2025 17:03:45 -0400 (EDT) Received: from phl-imap-06 ([10.202.2.83]) by phl-compute-05.internal (MEProxy); Sat, 05 Jul 2025 17:03:45 -0400 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:to; s=fm3; t=1751749425; x= 1751835825; bh=cW5LKegZ73GvcDIhRsvIJ7wpevTTpTZRNLgaIesdVAo=; b=T bLmXRjLPWsby6XCoyCtT+Brvw/MOp4cGmehQtnLRBePb7wjDmrOpIHciBs7nj3X3 3guOQZrwCKxV4WVjlTdrxSyQpK/9B8DfAq8Ig6iIwMVKXrdY8G9TlCyWsgKHpxRr 3syYj/TP5c5xSQbw7z0gqwI73J8plhg/S/TgxAioh7DuVxjtoM0Oy9OhjpV7MHMJ tafZMjSxdrPvyP1Dp5zawqeaXBVCyQRBSf7MIta+FtBvlloQYEZpl1ZBQREWxWQc lCrDZXZwGbFwkLskrxHmoTMs8vjJMdaPbGeJr8e9B4Un73TZvrY99tLGBFpzqadc ro8sM3s5yq79it7wStaWg== 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 :to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t= 1751749425; x=1751835825; bh=cW5LKegZ73GvcDIhRsvIJ7wpevTTpTZRNLg aIesdVAo=; b=ACQ0e0Coo4vqHpKIgzKLQPAKezz2TRN86BlggczsrY+3Tke6McI dSU7mQf1Yv3E15sKk8C4/vDSLevSNxmz7CJIbZ9wCkahg7XA3YA3maYw1fdyQ8wv W80iQP7oDlHkIdA8EWtx/6Lpi6xJicNTDX2sHKMp497MRcMa0V/vV2EI1hvHIGZm +2MFwJqpGuMSw96Gs8C9YDH7ReBvxg8j2gNMA9T+uXvoVE/hZzmM+UWLXTEwhCYn 6CEoTTzJmTlYlXEu6Qq2X3DtUkOU8S6TgTYKNkpjXdHHTBpdo0iymx5gZIjNXhD7 rUFRI6envapr00ZtCbdEojnTED83ULgmugg== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdefgddvjedtjecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpuffrtefokffrpgfnqfghnecuuegr ihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenucfjug hrpefoggffhffvvefkjghfufgtsegrtderreertdejnecuhfhrohhmpedftfhosgcunfgr nhguvghrshdfuceorhhosgessghothhtlhgvugdrtghouggvsheqnecuggftrfgrthhtvg hrnhepteefffegvdduleegkedvuedvhfeifffggfdvudejieektdeltdfgkeevfeeggfef necuffhomhgrihhnpehphhhprdhnvghtpdhgihhthhhusgdrtghomhenucevlhhushhtvg hrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehrohgssegsohhtthhlvggu rdgtohguvghspdhnsggprhgtphhtthhopedvpdhmohguvgepshhmthhpohhuthdprhgtph htthhopegurghnihgvlhdrvgdrshgthhgvrhiivghrsehgmhgrihhlrdgtohhmpdhrtghp thhtohepihhnthgvrhhnrghlsheslhhishhtshdrphhhphdrnhgvth X-ME-Proxy: Feedback-ID: ifab94697:Fastmail Received: by mailuser.phl.internal (Postfix, from userid 501) id AF3B62400094; Sat, 5 Jul 2025 17:03:44 -0400 (EDT) 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 X-ThreadId: Tdffe2a2312921c9a Date: Sat, 05 Jul 2025 23:03:23 +0200 To: "Daniel Scherzer" Cc: internals@lists.php.net Message-ID: <693c4589-0cc4-4097-921e-c98d819fa555@app.fastmail.com> In-Reply-To: References: <96e5addb-1570-4b4f-9915-c3fc55988f24@app.fastmail.com> Subject: Re: [PHP-DEV] Re: [RFC] [Discussion] #[\DelayedTargetValidation] attribute Content-Type: multipart/alternative; boundary=a2fe535ba9774407a5bf0d35cfe50656 From: rob@bottled.codes ("Rob Landers") --a2fe535ba9774407a5bf0d35cfe50656 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On Sat, Jul 5, 2025, at 21:53, Daniel Scherzer wrote: > On Fri, Jul 4, 2025 at 4:50=E2=80=AFPM Rob Landers = wrote: >> __ >>=20 >> On Sun, Jun 22, 2025, at 22:00, Daniel Scherzer wrote: >>> On Tue, Jun 17, 2025 at 4:26=E2=80=AFPM Daniel Scherzer wrote: >>>> Hi internals, >>>>=20 >>>> I'd like to start the discussion for a new RFC about adding a `#[\D= elayedTargetValidation]` attribute. >>>>=20 >>>> * RFC: https://wiki.php.net/rfc/delayedtargetvalidation_attribute >>>> * Implementation: https://github.com/php/php-src/pull/18817 >>>>=20 >>>> --Daniel >>>=20 >>>=20 >>> It seems a common point of discussion is the difference in behavior = between internal and userland attributes, so I wanted to clarify a few t= hings: >>>=20 >>> * Userland attributes are always metadata, in part because of the ba= ckwards and forward compatibility that those attributes provide - the at= tribute does not need to exist in order to be used, it just needs to exi= st (and target the location) when you call ReflectionAttribute::newInsta= nce() to instantiate >>> * Internal attributes are a mix between metadata and a way to plug i= nto the engine. There were already existing ways to plug into the engine= (e.g. using magic methods or implementing the Countable or ArrayAccess = interfaces) but attributes provided a way to plug into the engine when y= ou don't just want to add a function override, but rather something else= (like indicating a parameter should be redacted in backtraces with `#[\= SensitiveParameter]`). It would probably be impossible to do that secure= ly with a userland attribute and userland error handler that manually re= dacted things... >>> * Attributes were designed to have good compatibility - the syntax c= hosen for PHP 8.0 was, in prior versions of PHP, used for comments - so = any code with attributes could also work in PHP 7.4, just without the me= tadata. Similarly, by not validating userland attributes until they are = accessed with reflection, you can add an attribute that does not exist y= et (e.g. if using different versions of a library) with minimal complica= tions. >>> * Internal attributes are validated at compile time - because they c= an be. Internal attributes tell the engine to do something, and it makes= sense (at least to me) that if the engine cannot do that thing, there s= hould be an error without needing to wait for ReflectionAttribute::newIn= stance() to be called. >>>=20 >>> But, the validation of internal attributes at compile time means tha= t they lose the compatibility features of userland attributes, and that = is what this RFC is trying to address. To be clear, I think the differen= ce in behavior between userland and internal attributes is a) fundamenta= lly based on the difference in capabilities (pure metadata vs engine beh= avior) and b) something that should not be changed without significant f= urther investigation. >>=20 >> I don=E2=80=99t think this is quite right. Because non-existent attri= butes can be used, if you use attributes in your library, you already kn= ow to only ever, and I mean only ever, instantiate your own attributes. = Attempting to instantiate someone else=E2=80=99s attributes can result i= n crashes. This means any delayed attributes will most likely never be v= alidated by anything. >>=20 >=20 > Symfony instantiates arbitrary attributes, https://github.com/symfony/= symfony/pull/40307 - and while delayed attributes may not end up being v= alidated, that is a feature, not a bug - the goal is that the validation= not be performed at compile time, and so it was either delay to runtime= , or not do the validation at all. > =20 >>=20 >>>=20 >>> This new `#[\DelayedTargetValidation]` is meant to simplify things, = and to partially unify the behavior of the errors - if you are getting a= ny errors from internal attributes and want to suppress them at compile = time, just add the new attribute everywhere. >>>=20 >>> -Daniel=20 >>=20 >> I don=E2=80=99t think this is =E2=80=9Cforward compatibility=E2=80=9D= , this is completely disabling the attribute. >>=20 >> =E2=80=94 Rob >=20 >=20 > So I guess my use of "forward compatibility" was a bit confusing. This= allows a library to be backwards compatible (using PHP 8.6 attribute ta= rgets but running on PHP 8.5 without errors) by making the language forw= ard compatible (allowing PHP 8.5 to process 8.6 attribute targets withou= t errors, by delaying those errors). The attribute is not disabled at al= l though. >=20 > * If the attribute is used in a place where it would do anything (i.e.= there wouldn't be an error about being a bad target) then that attribut= e is used as normal, and #[\DelayedTargetValidation] does not affect it > * If the attribute is used in a place where it would error, then it al= ready cannot do anything, and #[\DelayedTargetValidation] just delays th= e error - the attribute is already going to be inactive anyway >=20 > -Daniel > =20 Hmmm... I still don't get it. If you scan your codebase for these attrib= utes (maybe using something like https://github.com/olvlvl/composer-attr= ibute-collector) and then instantiate them ... then you are right back w= here you started. You'll end up getting the same validation errors you'd= normally get. But I don't know why you'd do such a thing. You can use \= NoDiscard today, but if you try to instantiate it -- it will crash witho= ut a polyfill. There are attributes used only by IDEs (like `\Pure`) and= attempting to instantiate them will crash. This is why you don't instan= tiate attributes you don't own. By adding this, you force all framework to try instantiating these built= -in attributes; and scanning for them. This increases the bug surface, b= oot times, and maintenance cost ... to basically end up right where we s= tarted. I just don't see the point of this. I think the problem is real, I just = don't think this is the right solution. Instead, it would probably be simpler to backport a change to earlier ve= rsions of PHP that simply allows the future target without validation; o= r to do a two phase rollout of a target change (minor + 1: add the targe= t without validation, then minor + 2, enable validation); or choose a di= fferent name. Another alternative would be to simply write code for the minimum versio= n of PHP you support. For example, if you want to use the pipe operator,= you either need to support 8.5 as a minimum, or not use it. =E2=80=94 Rob --a2fe535ba9774407a5bf0d35cfe50656 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable
On Sat, Jul = 5, 2025, at 21:53, Daniel Scherzer wrote:
On Fri, Jul 4, 2= 025 at 4:50=E2=80=AFPM Rob Landers <rob@bottled.codes> wrote:

=

On Sun, Jun 22, 2025, at 22:00, Daniel Sc= herzer wrote:
On Tue, Jun 17, 2025 at 4:26=E2= =80=AFPM Daniel Scherzer <daniel.e.scherzer@gmail.com> wrote:
= Hi internals,

I'd like to start the discussion = for a new RFC about adding a `#[\DelayedTargetValidation]` attribute.


--Daniel


I= t seems a common point of discussion is the difference in behavior betwe= en internal and userland attributes, so I wanted to clarify a few things= :

* Userland attributes are always metadata, in= part because of the backwards and forward compatibility that those= attributes provide - the attribute does not need to exist in order= to be used, it just needs to exist (and target the location) when you c= all ReflectionAttribute::newInstance() to instantiate
* Intern= al attributes are a mix between metadata and a way to plug into the engi= ne. There were already existing ways to plug into the engine (e.g. using= magic methods or implementing the Countable or ArrayAccess interfaces) = but attributes provided a way to plug into the engine when you don't jus= t want to add a function override, but rather something else (like indic= ating a parameter should be redacted in backtraces with `#[\SensitivePar= ameter]`). It would probably be impossible to do that securely with a us= erland attribute and userland error handler that manually redacted thing= s...
* Attributes were designed to have good compatibility - t= he syntax chosen for PHP 8.0 was, in prior versions of PHP, used for com= ments - so any code with attributes could also work in PHP 7.4, just wit= hout the metadata. Similarly, by not validating userland attributes unti= l they are accessed with reflection, you can add an attribute that does = not exist yet (e.g. if using different versions of a library) with minim= al complications.
* Internal attributes are validated at compi= le time - because they can be. Internal attributes tell the engine to do= something, and it makes sense (at least to me) that if the engine canno= t do that thing, there should be an error without needing to wait for Re= flectionAttribute::newInstance() to be called.

= But, the validation of internal attributes at compile time means that th= ey lose the compatibility features of userland attributes, and that is w= hat this RFC is trying to address. To be clear, I think the difference i= n behavior between userland and internal attributes is a) fundamentally = based on the difference in capabilities (pure metadata vs engine behavio= r) and b) something that should not be changed without significant furth= er investigation.

I do= n=E2=80=99t think this is quite right. Because non-existent attributes c= an be used, if you use attributes in your library, you already know to o= nly ever, and I mean only ever, instantiate your own attributes. Attempt= ing to instantiate someone else=E2=80=99s attributes can result in crash= es. This means any delayed attributes will most likely never be validate= d by anything.


Symfony instantiates arbitrary attributes, https://github.com/symfony/symfony/p= ull/40307 - and while delayed attributes may not end up being valida= ted, that is a feature, not a bug - the goal is that the validation not = be performed at compile time, and so it was either delay to runtime, or = not do the validation at all.
 

=

This new `#[\DelayedTargetValidation]` is= meant to simplify things, and to partially unify the behavior of the er= rors - if you are getting any errors from internal attributes and want t= o suppress them at compile time, just add the new attribute everywhere.<= /div>

-Daniel 

I don=E2=80=99t think this is =E2=80=9Cforward compatib= ility=E2=80=9D, this is completely disabling the attribute.
=E2=80=94 Rob<= /div>


So I guess my= use of "forward compatibility" was a bit confusing. This allows a libra= ry to be backwards compatible (using PHP 8.6 attribute targets but runni= ng on PHP 8.5 without errors) by making the language forward compatible = (allowing PHP 8.5 to process 8.6 attribute targets without errors, by de= laying those errors). The attribute is not disabled at all though.
=

* If the attribute is used in a place where it would= do anything (i.e. there wouldn't be an error about being a bad target) = then that attribute is used as normal, and #[\DelayedTargetValidation] d= oes not affect it
* If the attribute is used in a place where = it would error, then it already cannot do anything, and #[\DelayedTarget= Validation] just delays the error - the attribute is already going to be= inactive anyway

-Daniel
 
=

Hmmm... I still don't get i= t. If you scan your codebase for these attributes (maybe using something= like https://github.com/olvlvl/composer-attribute-collector) and th= en instantiate them ... then you are right back where you started. You'l= l end up getting the same validation errors you'd normally get. But I do= n't know why you'd do such a thing. You can use \NoDiscard today, but if= you try to instantiate it -- it will crash without a polyfill. There ar= e attributes used only by IDEs (like `\Pure`) and attempting to instanti= ate them will crash. This is why you don't instantiate attributes you do= n't own.

By adding this, you force all framewor= k to try instantiating these built-in attributes; and scanning for them.= This increases the bug surface, boot times, and maintenance cost ... to= basically end up right where we started.

I jus= t don't see the point of this. I think the problem is real, I just don't= think this is the right solution.

Instead, it = would probably be simpler to backport a change to earlier versions of PH= P that simply allows the future target without validation; or to do a tw= o phase rollout of a target change (minor + 1: add the target without va= lidation, then minor + 2, enable validation); or choose a different name= .

Another alternative would be to simply write = code for the minimum version of PHP you support. For example, if you wan= t to use the pipe operator, you either need to support 8.5 as a minimum,= or not use it.

=E2=80=94 R= ob
--a2fe535ba9774407a5bf0d35cfe50656--