On Mon, Mar 4, 2024 at 4:40 PM Stephen Reay <php-li...@koalephant.com> wrote: > > > > > On 28 Feb 2024, at 06:17, Larry Garfield <la...@garfieldtech.com> wrote: > > > > On Sun, Feb 25, 2024, at 10:16 PM, Rowan Tommins [IMSoP] wrote: > >> [Including my full previous reply, since the list and gmail currently > >> aren't being friends. Apologies that this leads to rather a lot of > >> reading in one go...] > > > > Eh, I'd prefer a few big emails that come in slowly to lots of little > > emails that come in fast. :-) > > > >>> On 21/02/2024 18:55, 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. > >>> > >>> > >>> Hello, and a huge thanks to both you and Ilija for the continued work > >>> on this. I'd really like to see this feature make it into PHP, and > >>> agree with a lot of the RFC. > >>> > >>> > >>> My main concern is the proliferation of things that look the same but > >>> act differently, and things that look different but act the same: > > > > *snip* > > > >>> - a and b are both what we might call "traditional" properties, and > >>> equivalent to each other; a uses legacy syntax which we haven't > >>> removed for some reason > > > > I don't know why we haven't removed `var` either. I can't recall the last > > time I saw it in real code. But that's out of scope here. > > > > *snip* > > > >> I think there's some really great functionality in the RFC, and would > >> love for it to succeed in some form, but I think it would benefit from > >> removing some of the "magic". > >> > >> > >> Regards, > >> > >> -- > >> Rowan Tommins > >> [IMSoP] > > > > > > I'm going to try and respond to a couple of different points together here, > > including from later in the thread, as it's just easier. > > > > == Re, design philosophy: > > > >> In C#, all "properties" are virtual - as soon as you have any > >> non-default "get", "set" or "init" definition, it's up to you to declare > >> a separate "field" to store the value in. Swift's "computed properties" > >> are similar: if you have a custom getter or setter, there is no backing > >> store; to add behaviour to a "stored property", you use the separate > >> "property observer" hooks. > >> > >> Kotlin's approach is philosophically the opposite: there are no fields, > >> only properties, but properties can access a hidden "backing field" via > >> the special keyword "field". Importantly, omitting the setter doesn't > >> make the property read-only, it implies set(value) { field = value } > > > > A little history here to help clarify how we ended up where we are: The > > original RFC as we designed it modeled very closely on Swift, with 4 hooks. > > Using get/set at all would create a virtual property and you were on your > > own, while the beforeSet/afterSet hooks would not. We ran that design by > > some PHP Foundation sponsors a year ago (I don't actually know who, Roman > > did it for us), and the general feedback was "we like the idea, but woof > > this is complicated with all these hooks and having to make my own backing > > property for all these little things. Couldn't this be simplified?" We > > thought a bit more, and I off-handedly suggested to Ilija "I mean, would it > > be possible to just detect if a get/set hook is using a backing store and > > make it automatically? Then we could get rid of the before/after hooks." > > He gave it a quick try and found that was straightforward, so we pivoted to > > that simplified version. We then realized that we had... mostly just > > recreated Kotlin's design, so shrugged happily and went on with life. > > > > As noted in an earlier email, C#, Kotlin, and Swift all have different > > stances on the variable name for the incoming value. We originally modeled > > on Swift so had that model (optional newVal name), and also because we > > liked how compact it was. When we switched to the simplified, incidentally > > Kotlin-esque approach, we just kept the optional variable as it works. > > > > I think where that ended up is pretty nice, personally, even if it is not a > > direct map of any particular other language. > > > > == Re asymmetric typing: > > > > This is capability already present today if using a setter method. > > > > class Person { > > private $name; > > > > public function setName(UnicodeString|string $name) > > { > > $this->name = $value instanceof UnicodeString ? $value : new > > UnicodeString($value); > > } > > } > > > > And widening the parameter type in a child class is also entirely legal. > > As the goal of the RFC is, essentially, "make most common getter/setter > > patterns easy to add to a property without making an API-breaking method, > > so people don't have to add redundant just-in-case getters and setters all > > the time," covering an easy-to-cover use case seems like a good thing to do. > > > > It also ties into the question of the explict/implicit name, for the reason > > you mentioned earlier (unspecified means mixed), not by intent. More on > > that in another section. > > > > == Re virtual properties: > > > > Ilija and I talked this through, and there's pros and cons to a `virtual` > > keyword. Ilija also suggested a `backed` keyword, which forces a backed > > property to exist even if it's not used in the hook itself. > > > > * Adding `virtual` adds more work for the developer, but more clarity. It > > would also mean $this->$propName or $this->{__PROPERTY__} would work "as > > expected", since there's no auto-detection for virtual-ness. On the > > downside, if you have a could-be-virtual property but never actually use > > the backing value, you have an extra backing value hanging around in memory > > that is inaccessible normally, but will still show up in some serialization > > formats, which could be unexpected. If you omit one of the hooks and > > forget to mark it virtual, you'll still get the default of the other > > operation, which could be unexpected. (Mostly this would be for a > > virtual-get that accidentally has a default setter because you forgot to > > mark it `virtual`.) > > * Doing autodetection as now, but with an added "make a backing value > > anyway" flag would resolve the use case of "My set hook just calls a > > method, and that method sets the property, but since the hook doesn't > > mention the property it doesn't get created" problem. It would also allow > > for $this->$propName to work if a property is explicitly backed. On the > > flipside, it's one more thing to think about, and the above example it > > solves would be trivially solved by having the method just return the value > > to set and letting the set hook do the actual write, which is arguably > > better and more reliable code anyway. > > * The status quo (auto-detection based on the presence of $this->propName). > > This has the advantage it "just works" in the 95% case, without having to > > think about anything extra. The downside is it does have some odd edge > > cases, like needing $this->propName to be explicitly used. > > > > I don't think any is an obvious winner. My personal preference would be > > for status quo (auto-detect) or explicit-virtual always. I could probably > > live with either, though I think I'd personally favor status quo. Thoughts > > from others? > > > > I agree that a flag to make the field *virtual* (thus disabling the backing > store) makes more sense than a flag to make it backed; It's also easier to > understand when comparing hooked properties with regular properties > (essentially, backed is the default, you have to opt-in to it being virtual). > I don't think the edge cases of "auto" make it worthwhile just to not need > "virtual". > > > == Re reference-get > > > > Allowing backed properties to have a reference return creates a situation > > where any writes would then bypass the set hook, and thus any validation > > implemented there. That is, it makes the validation unreliable. A major > > footgun. The question is, do we favor caveat-emptor flexibility or > > correct-by-construction safety? Personally I always lead toward the > > latter, though PHP in general is... schizophrenic about it, I'd say. :-) > > > > At this point, we'd much rather leave it blocked to avoid the issue; it's > > easier to enable that loophole in the future if we really want it than to > > get rid of it if it turns out to have been a bad idea. > > > > There is one edge case that *might* make sense: If there is no set hook > > defined, then there's no set hook to worry about bypassing. So it may be > > safe to allow &get on backed properties IFF there is no set hook. I worry > > that is "one more quirky edge case", though, so as above it may be better > > to skip for now as it's easier to add later than remove. But if the > > consensus is to do that, we're open to it. (Question for everyone.) > > > > I don't have strong feeling about this, but in general I usually tend to > prefer options that are consistent, and give power/options to the developer. > If references are opt-in anyway, I see that as accepting the trade-offs. If a > developer doesn't want to allow by-ref modifications of the property, why > would they make it referenceable in the first place? This sounds a bit like > disallowing regular public properties because they might be modified outside > the class - that's kind of the point, surely. > > > == Re > > > > == Re arrays > > > >> Regarding arrays, have you considered allowing array-index writes if > > an &get hook is defined? i.e. "$x->foo['bar'] = 42;" could be treated > > as semantically equivalent to "$_temp =& $x->foo; $_temp['bar'] = 42; > > unset($_temp);" > > > > That's already discussed in the RFC: > > > >> The simplest approach would be to copy the array, modify it accordingly, > >> and pass it to set hook. This would have a large and obscure performance > >> penalty, and a set implementation would have no way of knowing which > >> values had changed, leading to, for instance, code needing to revalidate > >> all elements even if only one has changed. > > > > Unless we were OK with that bypassing the set hook entirely if defined, > > which, as noted above, means any safety guarantees provided by a set hook > > are bypassed, leading to untrustworthy code. > > > > == Re hook shorthands and return values > > > > Ilija and I have been discussing this for a bit, and we've both budged a > > little. :-) Here's our counter-proposal: > > > > - Drop the "top level" shorthand, for get-only hooks. > > - Keep the => shorthand for the get hook itself. > > - For a set hook, the {} form has no return value; set the value yourself > > however you want. > > - For a set hook, the => form implies a backed value and will set the > > property to whatever value that evaluates to. > > > > So these are equivalent: > > > > public $foo { set { $this->foo = $value; } } > > public $foo { set => $value; } > > > > These are equivalent: > > > > public string $foo { > > get { > > return strtoupper($this->foo); > > } > > } > > public string $foo { get => strtoupper($this->foo); } > > > > And this goes away: > > > > public string $foo => strtoupper($this->foo); > > > > That covers the common cases with an arrow-function-like syntax that > > behaves as you'd expect (it returns things), and allows a longer version > > with arbitrarily complex logic if desired. It also means that each syntax > > variant does mean something importantly different. > > > > Would that be an acceptable compromise? (Question for everyone.) > > > > I think the examples given are clear, and the lack of the top-level short > closure-esque version makes it more obvious. Forgive me, I must have missed > some of the previous comments - is there a reason the 'full' setter can't > return a value, for the sake of consistency? I understand that you don't want > "return to set" to be the *only* option, for the sake of e.g. change/audit > logging type functionality (i.e. set and then some action to record that the > change was made), but it seems a little odd and inconsistent to me that the > return value of a short closure would be used when the return value of the > long version isn't. This isn't really a major issue, I'm just curious if > there was some explanation about it? > > > == Re the $value variable in set > > > > Honestly, Rowan's earlier point here is the strongest argument in favor for > > me of the current RFC approach. Anywhere else in PHP, something that looks > > like a parameter and has no type, like `($param)`, means its type is > > `mixed`. It would be weird and confusing to be different here. That's > > above and beyond the issue of forcing people to retype something obvious > > every time. (I cite again, recent PHP's trend toward removing needless > > boilerplate, which is very good.) Requiring that the type be specified, > > for consistency, makes little sense if the type is not allowed to vary. > > You're just repeating a string from earlier on the same line, for no > > particular benefit. > > > > I genuinely don't understand the pushback on $value. It's something you > > learn once and never have to think about again. It's consistent. > > > > Ilija jokingly suggested making it always $value, unconditionally, and > > allowing only the type to be specified if widening: > > > > public int $foo { set(int|float) => floor($value); } > > > > Though I suspect that won't go over well, either. :-) > > > > So what makes the most sense to me is to keep $value optional, but IF you > > specify an alternate name, you must also specify a type (which may be > > wider). So these are equivalent: > > > > public int $foo { set (int $value) => $value + 1 } > > public int $foo { set => $value + 1 } > > > > And only those forms are legal. But you could also do this, if the > > situation called for it: > > > > public int $foo { set(int|float $num) => floor($num) + 1; } > > > > This "all or nothing" approach seems like it strikes the best balance, > > gives the most flexibility where needed while still having the least > > redundancy when not needed, and when a name/type is provided, its behavior > > is the same as for a method being inherited. > > > > Does that sound acceptable? (Again, question for everyone.) > > > > My only question with this is the same as I had in an earlier reply (and I'm > not sure it was ever answered directly?), and you allude to this yourself: > everywhere *else*, `($var)` means a parameter with type `mixed`. Why is the > type *required* here, when you've specifically said you want to avoid > boilerplate? If we're going to assume people can understand that `(implicit > property-type $value) is implicit, surely we can also assume that they will > understand "specifying a parameter without a type" means the parameter has no > type (i.e. is `mixed`). > > Again, for myself I'd be likely to type it (or regular parameters, > properties, etc) as `mixed` if that's what I want *anyway*, but the > inconsistency here seems odd, unless there's some until-now unknown drive to > deprecate type-less parameters/properties/etc. > > > > The alternative that gives the most future-flexibility is to do neither: > > The variable is called $value, period, you can't change it, and you can't > > change the type, either. There is no () after set, ever. Punt both of > > those to a later follow-up. I'd prefer to include both now, but including > > neither now is the next-safer option. > > > > > > ## Regarding $field > > > > Sigh, now y'all like it. :-P Most of the feedback on this has been > > negative, so I'm inclined to leave it out at this point, unless there's a > > major swing in feedback to bring it back. But the RFC seems more likely to > > pass without it than with right now. > > > > --Larry Garfield > > > > > Cheers > > > Stephen
I would think that simply using return-to-set would be the simplest solution, if you need to run something after it's set, you can use the regular way of running code after a return: try { return $value + 100; } finally { // this runs after returning } Robert Landers Software Engineer Utrecht NL