Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:120294
Feedback-ID: i8414410d:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-415-gf2b17fe6c3-fm-20230503.001-gf2b17fe6
Mime-Version: 1.0
Message-ID: <b501b7ac-0193-4604-a9b2-e03e69ba335c@app.fastmail.com>
In-Reply-To: 
 <CAG4FTXzt6qnpawSgPftnuXhNz9b1RAPtrOvVwvgY723W6WeLDA@mail.gmail.com>
References: <641b1ca0-d33f-4f38-ae64-81b4abce24da@app.fastmail.com>
 <CAG4FTXzt6qnpawSgPftnuXhNz9b1RAPtrOvVwvgY723W6WeLDA@mail.gmail.com>
Date: Mon, 15 May 2023 18:34:19 +0000
To: "php internals" <internals@lists.php.net>
Content-Type: text/plain
Subject: Re: [PHP-DEV] [RFC] Property hooks, nee accessors
From: larry@garfieldtech.com ("Larry Garfield")

Consolidating all replies into a single message.  Please try to keep them together so it's easier to respond to.

(Also, when replying, please please remove duplicate names from the to/cc line.  I just got double copies of everything in this thread this morning.  I'm on the list, I don't need to be CCed.)

On Sun, May 14, 2023, at 11:41 PM, Hendra Gunawan wrote:

> Hi Larry.
>
> ```
> public string $fullName => $this->first . " " . $this->last;
> ```
> 1. Suppose that I forgot to declare ```$this->first```. Based on the
> "deprecate dynamic properties" proposal, will I get an error/warning?

Correct, it will behave the same as if you had a fullName() method that accessed an undefined $first property.  

> 2. The shorthand notations supported (the shortest one) creates
> impaired syntax, and not pretty to look at for constructor property
> promotion.
>
> ```
> public function __construct(
>   public string $prop1 => strtoupper($this->_prop1),
>   public string $prop2 {set => $field = strtolower($value);},
> ){}
> ```

Well, the first is not really valid anyway.  That creates a virtual property with no set hook, which when you pass a value to it in the constructor call will give an error that you are trying to write to a virtual property.  So that's already a non-issue.

For the second, the problem with omitting the {} is that it creates yet another syntax variant.  That makes it harder for the parser, for static analyzers, for user-space parsing tools like php-parser, etc.  That's more work for everyone for fairly little gain.

Removing the {} entirely from all forms makes it harder to denote where the list of hooks begins and ends.  They would probably have to be comma-separated, like this:

public string $foo 
  get { return $this->bar; }, 
  set { $this->bar = $value; };

Which I'd argue is harder to read anyway, because you have to be careful of the very small , and ; marks.  The {} is just easier for humans to parse as well as the machine.

> Suppose that I just want to cache once and freeze, and I don't want to
> create additional hidden property. Please make a judgement on the
> validity of this syntax:
> ```
> public string $prop1 {
>   get => $field ??= $this->heavyTask($this->propx, $prop->y);
>   set => null;
> }
> ```
>
> Explanation:
> * Since the ```get``` hook uses ```$field```, the engine will create a
> backing store value with name ```$prop1```, and will be filled at the
> first call only. The later call will use the already stored value.
> * Since no new value is assigned to ```$field``` in the ```set```
> hook, the value stored in ```$prop1``` is still the same as before.
> * Since any return statement will be discharged in ```set``` hook, the
> assignment chaining is safe to use and the value which is transferred
> is alway the rightmost value (not the value returned by the ```set```
> hook).
> ```
> $varx = $obj->prop1 = "hello";  // $varx value is "hello", and
> $obj->prop1 value is unchanged
> ```

An interesting approach.  It should work as you describe, but there's two reasons I would discourage it:

1. A "set that doesn't set" creates unexpected behavior for callers.  If I call set, I expect something to happen.  It would be rather confusing to call set and not set things.
2. It doesn't actually allow a cache clearing option.

What we really would need here is asymmetric visibility.  No set hook, but make the property private(set) so that trying to write to it gives an error, as it should.  For cache clearing, the best way to do that is probably with an unset hook.  That's been omitted from the RFC for now for simplicity, but that sounds like it would be an argument to introduce it later.  Asymmetric visibility would be the higher priority, though.

> But, this statement below is NOT ALWAYS correct for "cache once and freeze":
> ```
> public string $prop1 {
>   set => $field ??= $this->heavyTask($this->propx, $prop->y);
> }
> ```
> Explanation:
> * Since default ```get``` hook can be called before ```set``` hook,
> backing store value with name ```$prop1``` will still be empty.
> * Otherwise, I have to duplicate the body of the ```set``` hook into
> ```get``` hook to prevent an emptiness backing store.
>
> Please give a comment.
> Thanks.

Correct; while this syntax would compile, it would result in the value assigned to the property being ignored, and then the other heavyTask() value assigned to it, but that still wouldn't ensure it was available the first time you called `get`.  That's probably not what anyone wants.

It looks like you answered your own question on the interface point, so I'll leave that be.

--Larry Garfield