Re: [PHP-DEV] [RFC] [Vote] #[\Deprecated] attribute
On Wed, May 22, 2024 at 9:33 AM Nicolas Grekas wrote: > Hi Benjamin, > > The vote for the RFC #[\Deprecated] attribute is now open: >> >> https://wiki.php.net/rfc/deprecated_attribute >> >> Voting will close on Wednesday 5th June, 08:00 GMT. >> > > I voted "no" because I think this is better addressed in userland, as this > gives more flexibility. > I would better have an attribute that is made only for static analysis > with no runtime side effects at all. > Being forced to make the attribute final because the implementation in the > engine requires is an example of why the engine is not the correct place to > send this notice. Another example is not being able to add the attribute on > classes because [engine reasons]. > This is a misrepresentation of what this RFC is. There are no engine reasons for not being able to set it on classes. It just has not been implemented yet. Allowing deprecations on classes is something that needs to be figured out in a separate RFC in how these semantics work. Then adding Deprecated attribute would just be a trivial matter. You could argue about the order of things, we could have first made an RFC for deprecated classes, but we should avoid a waterfall process of dependencies in things to do in the language. > > trigger_error() is better fitted for the runtime side-effect when it's > desired. > In my opinion, #[Deprecated] should be only for static analysers / > reflection (although this would need another discussion - I'm not sure this > would benefit being in the engine vs in a userland package. > This is the same as above, changing how deprecations work in the engine is orthogonal to this attribute. These attributes are concerned with tagging code elements with flags that the engine already provides, already makes available for internal code elements. > > Thanks for the RFC anyway. > > Nicolas >
[PHP-DEV] [RFC] [Vote] #[\Deprecated] attribute
The vote for the RFC #[\Deprecated] attribute is now open: https://wiki.php.net/rfc/deprecated_attribute Voting will close on Wednesday 5th June, 08:00 GMT.
Re: [PHP-DEV] Switching max_execution_time from CPU time to wall-clock time and from SIGPROF to SIGALRM
On Fri, May 17, 2024 at 3:49 PM Arnaud Le Blanc wrote: > Hi internals, > > There is an issue with max_execution_time on MacOS, probably only > MacOS 14 on Apple Silicon, that causes timeouts to fire too early [1]. > max_execution_time is implemented with setitimer(ITIMER_PROF) on this > platform, and the SIGPROF signal is delivered too early for some > reason. Switching to ITIMER_REAL appears to fix the issue, and Manuel > Kress opened a PR [3]. > > Are there objections against switching to ITIMER_REAL on MacOS (Apple > Silicon only) in all currently supported PHP versions? (next 8.2.x, > 8.3.x, 8.x) > > Apart from fixing the issue, this would introduce the following minor > breaking changes: > > - max_execution_time on MacOS would be based on wall-clock time rather > than CPU time, so the time spent in I/O, sleep(), and syscalls in > general would count towards the max_execution_time > - The SIGALRM signal would be used instead of the SIGPROF signal > (using SIGALRM conflicts with pcntl_alarm(), but SIGPROF conflicts > with profilers). As noted by Dmitry, it also conflicts with sleep() on > some platforms, however this should be safe on MacOS. > > Currently max_execution_time is based on wall-clock time on ZTS and > Windows, and CPU time otherwise. On Linux and FreeBSD, wall-clock time > can also be used when building with > --enable-zend-max-execution-timers. Máté proposed to add a wall-clock > based timeout in the past [2] but the discussion has stalled. Any > thoughts about eventually switching other platforms to wall-clock > timeouts in the next 8.x ? > > TL;DR: > - Any objection about using wall-clock max_execution_time and SIGALRM > on MacOS Apple Silicon in all supported versions? > - Thoughts about using wall-clock timeouts on all platforms in the next > 8.x ? > > [1] https://github.com/php/php-src/issues/12814 > [2] https://github.com/php/php-src/pull/6504 > [3] https://github.com/php/php-src/pull/13567 > > Best Regards, > Arnaud > For reference: There is also this RFC for max_execution_wall_time that was discussed four years ago. https://externals.io/message/112492#112492
[PHP-DEV] Re: [RFC] [Discussion] #[\Deprecated] attribute again v1.3
On Tue, Apr 23, 2024 at 3:27 PM Benjamin Außenhofer wrote: > Hi internals, > > My PR for #[\Deprecated] attribute was in hibernation for a long while now > and after some off-list discussion a few weeks ago I have decided to > revisit it and asked Tim to help me out with the work. > > Tim has cleaned up the PR quite a bit and also worked in additional > features such as #[Deprecated] support in stub generation. > > While there are still some small todos, at this point we want to restart > the discussion about the RFC for inclusion in 8.4: > > RFC: https://wiki.php.net/rfc/deprecated_attribute > PR: https://github.com/php/php-src/pull/11293 > Old discussion: https://externals.io/message/112554#112554 > > Let me know about your questions and feedback. > Feedback period is now nearing 4 weeks and the last 2 have not brought any new significant discussions, as such we plan to open the vote next Wednesday if no new roadblocks come up. Thank you to all participants for the feedback. > > greetings > Benjamin >
[PHP-DEV] Re: [RFC] [Discussion] #[\Deprecated] attribute again v1.3
On Thu, May 2, 2024 at 1:00 PM Benjamin Außenhofer wrote: > > > On Tue, Apr 23, 2024 at 3:27 PM Benjamin Außenhofer > wrote: > >> Hi internals, >> >> My PR for #[\Deprecated] attribute was in hibernation for a long while >> now and after some off-list discussion a few weeks ago I have decided to >> revisit it and asked Tim to help me out with the work. >> >> Tim has cleaned up the PR quite a bit and also worked in additional >> features such as #[Deprecated] support in stub generation. >> >> While there are still some small todos, at this point we want to restart >> the discussion about the RFC for inclusion in 8.4: >> >> RFC: https://wiki.php.net/rfc/deprecated_attribute >> PR: https://github.com/php/php-src/pull/11293 >> Old discussion: https://externals.io/message/112554#112554 >> >> Let me know about your questions and feedback. >> >> greetings >> Benjamin >> > > We have updated the RFC and PR to include #[Deprecated] on class constants > and enum cases as this is something the engine already supports for > internal class constants. > > This does not include support for non-class based constants, because they > don't have support for attributes at the moment and also only recently got > reflection support (for 8.4). > > We are planning to work on adding $since next and get back to the list > once that is finished. > The $since implementation is now added to the RFC and to the PR. We decided to make this a secondary voting choice, while there are use-cases for it in php core for doc rendering, other ways of doing the same thing are also available and given userland frameworks stated different requirements maybe its better left for userland to implement.
Re: [PHP-DEV] Inconsistencies between parameter number and index when reflecting a method/function
On Thu, May 2, 2024 at 2:51 PM Ollie Read wrote: > Hi All, > > I've been working on a PR that introduces > ReflectionFunctionAbstract::getParameter() and > ReflectionFunctionAbstract::hasParameter(), to fall more inline with the > other method sets we have, as well as just generally making peoples lives > easier. > > The PR is here: https://github.com/php/php-src/pull/10431 > > These methods accept an integer to retrieve a parameter by its position, > or a string to retrieve by its name. So far, I have built this so that if > you required the first parameter, it's parameter 0. I treat it this way > because the only other place where we deal with parameter indexes, is > ReflectionFunctionAbstract::getParameters() which returns the parameters > zero-indexed. > > The question that is holding this PR back is should these methods be 1 > indexed, so that the provided position is consistent with the error > messages, or how a person would typically count, or should they be 0 > indexed to remain consistent with the existing API. > PHP being a mostly zero indexed language I would say it should be getParamter(0) to get the parameter #1 (referred to as 1 in error messages). > > Girgias has asked that I pause the PR until we can have a discussion on > this mailing list about how to approach it, so I'm looking for feedback on > this. > > > --- > Best Regards, > *Ollie Read* > >
[PHP-DEV] Re: [RFC] [Discussion] #[\Deprecated] attribute again v1.3
On Tue, Apr 23, 2024 at 3:27 PM Benjamin Außenhofer wrote: > Hi internals, > > My PR for #[\Deprecated] attribute was in hibernation for a long while now > and after some off-list discussion a few weeks ago I have decided to > revisit it and asked Tim to help me out with the work. > > Tim has cleaned up the PR quite a bit and also worked in additional > features such as #[Deprecated] support in stub generation. > > While there are still some small todos, at this point we want to restart > the discussion about the RFC for inclusion in 8.4: > > RFC: https://wiki.php.net/rfc/deprecated_attribute > PR: https://github.com/php/php-src/pull/11293 > Old discussion: https://externals.io/message/112554#112554 > > Let me know about your questions and feedback. > > greetings > Benjamin > We have updated the RFC and PR to include #[Deprecated] on class constants and enum cases as this is something the engine already supports for internal class constants. This does not include support for non-class based constants, because they don't have support for attributes at the moment and also only recently got reflection support (for 8.4). We are planning to work on adding $since next and get back to the list once that is finished.
Re: [PHP-DEV] [RFC] [Discussion] #[\Deprecated] attribute again v1.3
On Tue, Apr 23, 2024 at 7:27 PM Levi Morrison
wrote:
> On Tue, Apr 23, 2024 at 7:30 AM Benjamin Außenhofer
> wrote:
> >
> > Hi internals,
> >
> > My PR for #[\Deprecated] attribute was in hibernation for a long while
> now and after some off-list discussion a few weeks ago I have decided to
> revisit it and asked Tim to help me out with the work.
> >
> > Tim has cleaned up the PR quite a bit and also worked in additional
> features such as #[Deprecated] support in stub generation.
> >
> > While there are still some small todos, at this point we want to restart
> the discussion about the RFC for inclusion in 8.4:
> >
> > RFC: https://wiki.php.net/rfc/deprecated_attribute
> > PR: https://github.com/php/php-src/pull/11293
> > Old discussion: https://externals.io/message/112554#112554
> >
> > Let me know about your questions and feedback.
> >
> > greetings
> > Benjamin
>
> I skimmed through the previous discussion and didn't see anything
> about adding a `since` property. This is occasionally useful, at least
> in my limited usage of it in Rust. The names below are modelled after
> the names in [Rust's deprecated attribute][1], but "note" is the same
> as the proposed "message":
>
> ```php
> #[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_FUNCTION)]
> class Deprecated
> {
> public function __construct(
> public readonly ?string $note = null,
> public readonly ?string $since = null
> ) {
> }
> }
>
> #[Deprecated(since: "1.3", note: "this is not good, use good_pls_use")]
> function bad_dont_use() {}
>
> #[Deprecated("this wasn't meant to be public, use good_pls_use instead")
> function oops_dont_use() {}
>
> function good_pls_use() {}
> ```
>
> In Rust, you get a message for each of ["since" and "note"][2]. In
> PHP, this might look something like:
>
> > Deprecated: Function bad_dont_use() is deprecated since 1.3,
> > this wasn't meant to be public, use good_pls_use instead in %s
> > on line %d
>
After discussing with Mate shortly one reason for adding $since from a PHP
project POV is that we do show the $since information in the generated
documentation output.
Integrating with the work in progress to auto generate parts of the
function docs based on the stub files, having the $since attribute on the
stubs would allow to use this as the central information in code. Therefore
we would reconsider and add the $since argument to the Deprecated class.
I am still partly on the side on Rowan (
https://externals.io/message/123184#123206) that i don't find the arguments
for this parameter very convincing but at least there is a use-case
internally now that warrants adding it.
>
> [1]:
> https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-deprecated-attribute
> [2]:
> https://github.com/rust-lang/rfcs/blob/master/text/1270-deprecation.md#intended-use
>
Re: [PHP-DEV] [RFC] [Discussion] #[\Deprecated] attribute again v1.3
On Wed, Apr 24, 2024 at 7:18 PM Jorg Sowa wrote:
> I like the proposition and I like the idea of $since parameter, however,
> this option is too ambiguous about what should store. Should it store the
> PHP version, package version, or the date?
>
> What about setting this parameter vaguely as the boolean we can pass?
>
> #[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_FUNCTION)]
> class Deprecated
> {
> public function __construct(
> public readonly ?string $note = null,
> public readonly ?bool $since = null
> ) {
> }
> }
>
> #[Deprecated(since: $packageVersion > 5.5)]
> #[Deprecated(since: PHP_VERSION_ID > 80100)]
> #[Deprecated(since: date("Y-m-d") > "2024-01-21")]
>
These are not valid uses of attributes, the expressions for since parameter
are illegal in the language.
PHP Fatal error: Constant expression contains invalid operations in
/private/tmp/dep.php on line 6
And this proposal would only make the condition human readable, the machine
would see $since as either true or false, and that is not really something
that tools will find valuable enough.
But I agree on $since being less useful, users could put a date, a string,
or anyhing in there.
The way I see it vendors should add their own attributes, example:
#[\Deprecated, Since(package: "symfony/http-request", version: "7.0.2")]
or for PHPStorms replacement functinoality (potentially usable by Rector):
#[]Deprecated, Replacement('%class%->setPublic(!%parameter0%)')]
This is
a.) composable, which increases the potential use-cases for different
vendor and communities and doesn't restrict them to what core "designed in
the ivory tower once".
b.) allows vendors to have very strict validation on their attributes
arguments. I.e. Symfony could enforce package to be a valid composer
package string and version to follow composers version parser. This is
something we cannot enforce in the engine.
>
> Kind regards,
> Jorg
>
Re: [PHP-DEV] [RFC] [Discussion] #[\Deprecated] attribute again v1.3
On Wed, Apr 24, 2024 at 2:55 PM Lynn wrote: > > > On Tue, Apr 23, 2024 at 3:30 PM Benjamin Außenhofer > wrote: > >> Hi internals, >> >> My PR for #[\Deprecated] attribute was in hibernation for a long while >> now and after some off-list discussion a few weeks ago I have decided to >> revisit it and asked Tim to help me out with the work. >> >> Tim has cleaned up the PR quite a bit and also worked in additional >> features such as #[Deprecated] support in stub generation. >> >> While there are still some small todos, at this point we want to restart >> the discussion about the RFC for inclusion in 8.4: >> >> RFC: https://wiki.php.net/rfc/deprecated_attribute >> PR: https://github.com/php/php-src/pull/11293 >> Old discussion: https://externals.io/message/112554#112554 >> >> Let me know about your questions and feedback. >> >> greetings >> Benjamin >> > > JetBrains was mentioned in the previous discussion but not sure if this > was considered in the RFC: > https://github.com/JetBrains/phpstorm-attributes/blob/master/src/Deprecated.php > > Yes we considered it, but this is one implementation of a vendor, it might be conflicting with requirements of other projects and vendors. For the implementation and the engine itself this information is just "weight" that is carried around, so we decided not to add them, because its harder to decide on what to include than recommending tools to add their own supplemental attributes.
Re: [PHP-DEV] [RFC] [Discussion] #[\Deprecated] attribute again v1.3
On Wed, Apr 24, 2024 at 3:57 PM Nicolas Grekas wrote: > Hi Benjamin, > > My PR for #[\Deprecated] attribute was in hibernation for a long while now >> and after some off-list discussion a few weeks ago I have decided to >> revisit it and asked Tim to help me out with the work. >> >> Tim has cleaned up the PR quite a bit and also worked in additional >> features such as #[Deprecated] support in stub generation. >> >> While there are still some small todos, at this point we want to restart >> the discussion about the RFC for inclusion in 8.4: >> >> RFC: https://wiki.php.net/rfc/deprecated_attribute >> PR: https://github.com/php/php-src/pull/11293 >> Old discussion: https://externals.io/message/112554#112554 >> >> Let me know about your questions and feedback. >> > > Thanks for the RFC. > > While I'd like to be able to replace as many annotations as possible with > attributes, the devil is in the details and here, I'm not sold about the > details: > >- I concur with others about the "since" property being missing > > >- We'd also need a "package" property so that it's trivial to know >which composer package is deprecating. >- The attribute class should not be final, so that packages could >subclass, e.g. to define the "since" or "package" property once for all - >or to define a custom constructor, etc. > > As the RFC mentions, this is technically not possible, because the attribute is processed during compilation and an instanceof check at that point is not legal (cant autoload more during compiling). Even considering we can solve this technically, it would also otherwise secretly mean that all method/function attributes are getting autoloaded and this would break the core assumption of attributes being backed by classes on Reflection access only. > >- We should be able to add the attribute to a class. > > This is in the future scope as it requires a very different (orthogonal) implementation. > >- > > Yes, the "package" + "since" info can be put in the message itself, but > having a structured way to declare them is critical to 1. be sure that > authors don't forget to give that info and 2. build tools around that. > Are there tools around them? I can't find a use case in my head what the "since" property can be used for. Both since and package would be optional attributes, so tooling that checks they are not forgotten is needed anyways. It could just as well check for missing additional attributes next to the internal Deprecated attribute. > > You're saying that these are not useful because the engine wouldn't make > anything useful out of e.g. "since". > That's true, although I'd suggest using them to prefix the final message. > If that's the policy around attributes for the engine, then I'm wondering > if the attribute shouldn't live elsewhere. Or if we shouldn't discuss this > policy. > The attribute is there to close the gap with internal functions ZEND_ACC_DEPRECATED flag and ReflectionFunction/Method::isDeprecated already existing. This cannot live elsewhere. The since, package (and replacement) requirements however could live elsewhere. Given the many different requirements, we could think about adding an extra variadic argument, so that you could add whatever information you pass to the attributa via a key value array returned from Deprecated::getExtraInformation() ; > I also anticipate a problem with the adoption period of the attribute: in > order to be sure that a call to a deprecated method will trigger a > deprecation, authors will have to 1. add the attribute and 2. still call > trigger_error when running on PHP < 8.next. That's a lot of boilerplate. > This is a problem of any new feature that covers a use-case previously solved differently, for example attributes themselves, where libraries/Frameworks shipped both at the same time for a while. Taking this argument at face value would mean we stop evolving PHP. > > Personally, I'm not convinced that there should be any runtime > side-effects to the attribute. I'd prefer having it be just reported by > reflection. > This RFC does not introduce the runtime side effects of ZEND_ACC_DEPRECATED flag on methods or functions. They already exist with internal functions. Having this attribute not trigger the deprecation where they are already triggered for internal functions introduces an inconsistency. We should talk about how PHP implements deprecation tracking and logging as a separate issue, I have a lot of ideas here how to change that. > Nicolas > > > > >
Re: [PHP-DEV] [RFC] [Discussion] #[\Deprecated] attribute again v1.3
On Tue, Apr 23, 2024 at 7:27 PM Levi Morrison
wrote:
> On Tue, Apr 23, 2024 at 7:30 AM Benjamin Außenhofer
> wrote:
> >
> > Hi internals,
> >
> > My PR for #[\Deprecated] attribute was in hibernation for a long while
> now and after some off-list discussion a few weeks ago I have decided to
> revisit it and asked Tim to help me out with the work.
> >
> > Tim has cleaned up the PR quite a bit and also worked in additional
> features such as #[Deprecated] support in stub generation.
> >
> > While there are still some small todos, at this point we want to restart
> the discussion about the RFC for inclusion in 8.4:
> >
> > RFC: https://wiki.php.net/rfc/deprecated_attribute
> > PR: https://github.com/php/php-src/pull/11293
> > Old discussion: https://externals.io/message/112554#112554
> >
> > Let me know about your questions and feedback.
> >
> > greetings
> > Benjamin
>
> I skimmed through the previous discussion and didn't see anything
> about adding a `since` property. This is occasionally useful, at least
> in my limited usage of it in Rust. The names below are modelled after
> the names in [Rust's deprecated attribute][1], but "note" is the same
> as the proposed "message":
>
> ```php
> #[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_FUNCTION)]
> class Deprecated
> {
> public function __construct(
> public readonly ?string $note = null,
> public readonly ?string $since = null
> ) {
> }
> }
>
> #[Deprecated(since: "1.3", note: "this is not good, use good_pls_use")]
> function bad_dont_use() {}
>
> #[Deprecated("this wasn't meant to be public, use good_pls_use instead")
> function oops_dont_use() {}
>
> function good_pls_use() {}
> ```
>
> In Rust, you get a message for each of ["since" and "note"][2]. In
> PHP, this might look something like:
>
> > Deprecated: Function bad_dont_use() is deprecated since 1.3,
> > this wasn't meant to be public, use good_pls_use instead in %s
> > on line %d
>
This request is similar to Roman's question about a replacement parameter
elsewhere. We are unsure about these, because from an engine POV they do
not add value, and from a user messaging perspective they could be put into
the message with #[\Deprecated("since 1.3, this is not good, use
good_pls_use")].
The only reason this might make sense is to allow third party tooling to
work on this, but there are no conventions ala php-doc here in place
already. If tools need more infos, they could just introduce their own
attributes. example:
#[\Deprecated, Since("1.3"), Replacement("good_pls_use()")].
It feels arbitrary if we add parameters that the engine does not use and
where no tooling conventions exist on how they are being used, so we left
them out for now.
> [1]:
> https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-deprecated-attribute
> [2]:
> https://github.com/rust-lang/rfcs/blob/master/text/1270-deprecation.md#intended-use
>
[PHP-DEV] [RFC] [Discussion] #[\Deprecated] attribute again v1.3
Hi internals, My PR for #[\Deprecated] attribute was in hibernation for a long while now and after some off-list discussion a few weeks ago I have decided to revisit it and asked Tim to help me out with the work. Tim has cleaned up the PR quite a bit and also worked in additional features such as #[Deprecated] support in stub generation. While there are still some small todos, at this point we want to restart the discussion about the RFC for inclusion in 8.4: RFC: https://wiki.php.net/rfc/deprecated_attribute PR: https://github.com/php/php-src/pull/11293 Old discussion: https://externals.io/message/112554#112554 Let me know about your questions and feedback. greetings Benjamin
Re: [PHP-DEV] [RFC] [Discussion] Add openStream() to XML{Reader,Writer}
On Mon, Apr 22, 2024 at 8:43 PM Niels Dossche
wrote:
> Hi internals
>
> I'm opening the discussion for my RFC "Add openStream() to
> XML{Reader,Writer}".
> RFC link: https://wiki.php.net/rfc/xmlreader_writer_streams
Not sure this needs an RFC. Its a sensible addition and the explanations
make sense. :+1: from me.
>
>
> Kind regards
> Niels
>
Re: [PHP-DEV] [RFC[ Property accessor hooks, take 2
Hey, On Wed, Feb 21, 2024 at 7:58 PM Larry Garfield wrote: > Hello again, fine Internalians. > > After much on-again/off-again work, Ilija and I are back with a more > polished property access hooks/interface properties RFC. It’s 99% > unchanged from last summer; the PR is now essentially complete and more > robust, and we were able to squish the last remaining edge cases. > > Baring any major changes, we plan to bring this to a vote in mid-March. > > https://wiki.php.net/rfc/property-hooks > > It’s long, but that’s because we’re handling every edge case we could > think of. Properties involve dealing with both references and inheritance, > both of which have complex implications. We believe we’ve identified the > most logical handling for all cases, though. > > Note the FAQ question at the end, which explains some design choices. > > There’s one outstanding question, which is slightly painful to ask: > Originally, this RFC was called “property accessors,” which is the > terminology used by most languages. During early development, when we had > 4 accessors like Swift, we changed the name to “hooks” to better indicate > that one was “hooking into” the property lifecycle. However, later > refinement brought it back down to 2 operations, get and set. That makes > the “hooks” name less applicable, and inconsistent with what other > languages call it. > > However, changing it back at this point would be a non-small amount of > grunt work. There would be no functional changes from doing so, but it’s > lots of renaming things both in the PR and the RFC. We are willing to do so > if the consensus is that it would be beneficial, but want to ask before > putting in the effort. thank you for this proposal. there are some points i'd like to make into this discussion: * Thank you for the removal of $field, it was non-idomatic from a PHP POV. * I would prefer that the short syntax $foo => null; be voted upon separately. Personally I think it could be confusing and is too close to a regular assignment for default value and I prefer not to have it and keep the rest of the RFC. * The magic of detecting if a property is virtual or backed is - like Rowan said - subtle. I would also prefer this to be managed via an explicit mechanism, by for example keywording the property as "virtual", instead of the implicit way with the parsing based detection. * As Doctrine project maintainer, we have had some troubles supporting read only properties for proxies. I would have hoped that with hooks I can overwrite a readonly property and "hook" into it instead by defining a getter, but "no setter". From my read of the RFC this would not be allowed and throws a compile time error. Could you maybe clarify why at least this special case is not possible? I didn't immediately get it from the RFC section on readonly as it only speaks about the problems in abstract. greetings Benjamin > > -- > Larry Garfield > la...@garfieldtech.com >
Re: [PHP-DEV] Declaration-aware attributes
On Tue, May 30, 2023 at 2:49 AM Andreas Hennings
wrote:
> Hello internals,
> I am picking up an idea that was mentioned by Benjamin Eberlei in the past.
> https://externals.io/message/110217#110395
> (we probably had the idea independently, but Benjamin's is the first
> post where I see it mentioned in the list)
>
> Quite often I found myself writing attribute classes that need to fill
> some default values or do some validation based on the symbol the
> attribute is attached to.
> E.g. a parameter attribute might require a specific type on that
> parameter, or it might fill a default value based on the parameter
> name.
>
> Currently I see two ways to do this:
> 1. Do the logic in the code that reads the attribute, instead of the
> attribute class. This works ok for one-off attribute classes, but it
> becomes quite unflexible with attribute interfaces, where 3rd parties
> can provide their own attribute class implementations.
> 2. Add additional methods to the attribute class that take the symbol
> reflector as a parameter, like "setReflectionMethod()", or
> "setReflectionClass()". Or the method in the attribute class that
> returns the values can have a reflector as a parameter.
>
> Both of these are somewhat limited and unpleasant.
>
> I want to propose a new way to do this.
> Get some feedback first, then maybe an RFC.
>
> The idea is to mark constructor parameters of the attribute class with
> a special parameter attribute, to receive the reflector.
> The other arguments are then shifted to skip the "special" parameter.
>
> #[Attribute]
> class A {
> public function __construct(
> public readonly string $x,
> #[AttributeContextClass]
> public readonly \ReflectionClass $class,
> public readonly string $y,
> ) {}
> }
>
> $a = (new ReflectionClass(C::class))->getAttributes()[0]->newInstance();
> assert($a instanceof A);
> assert($a->x === 'x');
> assert($a->class->getName() === 'C');
> assert($a->y === 'y');
>
> Note that for methods, we typically need to know the method reflector
> _and_ the class reflector, because the method could be defined in a
> base class.
>
> #[Attribute]
> class AA {
> public function __construct(
> #[AttributeContextClass]
> public readonly \ReflectionClass $class,
> #[AttributeContextMethod]
> public readonly ReflectionMethod $method,
> ) {}
> }
>
> class B {
> #[AA]
> public function f(): void {}
> }
>
> class CC extends B {}
>
> $aa = (new ReflectionMethod(CC::class,
> 'f))->getAttributes()[0]->newInstance();
> assert($a->class->getName() === 'CC');
> assert($a->method->getName() === 'f');
>
> ---
>
> Notice that the original proposal by Benjamin would use an interface
> and a setter method, ReflectorAwareAttribute::setReflector().
>
> I prefer to use constructor parameters, because I generally prefer if
> a constructor creates a complete and immutable object.
>
Thank you bringing this up, the more I work with attributes the more often
this comes up. I think when we designed the attributes there was just so
little concrete exprimentation that we didn't pick this up as a serious
missing gap.
As for implementation, reviewing the whole e-mail thread, i like both:
1, ReflectionAttribute::getReflectionTarget() - this we should add no
matter what and is a no brainer
2. An argument attribute that instructs newInstance() to inject the
reflector or the ReflectionAttribute, for example #[AttributeContext] or
#[AttributeTargetReflector]
>
>
>
> Thoughts?
>
> -- Andreas
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>
Re: [PHP-DEV] [RFC] Property hooks, nee accessors
On Mon, May 8, 2023 at 11:38 PM Larry Garfield wrote: > Ilija Tovilo and I would like to offer another RFC for your > consideration. It's been a while in coming, and we've evolved the design > quite a bit just in the last week so if you saw an earlier draft of it in > the past few months, I would encourage you to read it over again to make > sure we're all on the same page. I'm actually pretty happy with where it > ended up, even if it's not the original design. This approach eliminates > several hard-to-implement edge cases while still providing a lot of > functionality in one package. > > https://wiki.php.net/rfc/property-hooks Thank you! Looks interesting, I will need to think about things before more qualitative feedback. An error maybe, the "class C" example in "Detailed Proposal > set" uses a "$this->_prop", but probably meant to use $this->_names, since private array $_names; is declared in the example. I think $field should be its own "chapter". Its a central part of the proposal that should be clarified early and so that readers don't accidentally skip it. I am also confused why $field exists when $this->propertyName works and why its not recommended to be used. Is $field a reference? or does the "compile time macro" part mean its replaced at compile time? Ifso, this feels different to anything else PHP, i am leaning towards $this->propertyName if there are no other compelling reasons why $field should be used. > > > -- > Larry Garfield > la...@garfieldtech.com > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > >
Re: [PHP-DEV] [VOTE] PHP Technical Committee
On Fri, Apr 28, 2023 at 12:01 PM Jakub Zelenka wrote: > Hi, > > The vote is now open for the RFC about introduction of the PHP Technical > Committee: > > https://wiki.php.net/rfc/php_technical_committee I found this idea of a TC interesting on the outset, but after carefully consideirng I voted no on this RFC because a.) i believe it to be too much bearucracy for the little benefit b.) potentially harmful depending on who is on the TC. c.) There is also no process of overuling the TC, or voting a TC out due to no confidence or something. Without the votes known of TC members, voters of the TC have no insights into who they might want to boot or keep in the next election. However introducing these data points would make everything even more complicated. Ultimately, already at the moment each controversial change can be asked to be RFCed and then the voters can decide with arguments made by people knowledgable in the area. Yes, there is always some politics involved, but the same would be true of the TC decisions. > > Regards > > Jakub >
