Static property asymmetric visibility was left out of the original RFC, because it seemed like it would be hard and of little use. Turns out, Ilija found a way to make it easy. (Ilija is smart.) So here's a small RFC to add aviz to static properties, because we can't think of a reason to NOT do so.
https://wiki.php.net/rfc/static-aviz
--
Larry Garfield
larry@garfieldtech.com
Hi
Am 2024-11-25 17:52, schrieb Larry Garfield:
Static property asymmetric visibility was left out of the original RFC, because it seemed like it would be hard and of little use. Turns out, Ilija found a way to make it easy. (Ilija is smart.) So here's a small RFC to add aviz to static properties, because we can't think of a reason to NOT do so.
The RFC has the status “Draft” and does not appear to be linked in the overview at PHP: rfc. Is it intended to be open for discussion yet?
Regarding the RFC contents: I am not sure the behavior of `private(set)` implying `final` is obvious for static properties. Consider this example (Online PHP editor | output for bvQjM):
<?php
class Foo {
public static $baz;
}
class Bar extends Foo {
public static $baz;
}
Foo::$baz = '2';
Bar::$baz = '1';
var_dump(Foo::$baz, Bar::$baz);
Non-static properties only have `$this->` for property access, but with static properties you can distinguish between `self::` and `static::`, which makes overriding a `private(set)` static property meaningful, because they are actually independent properties.
Best regards
Tim Düsterhus
On Tue, Nov 26, 2024, at 3:57 AM, Tim Düsterhus wrote:
Hi
Am 2024-11-25 17:52, schrieb Larry Garfield:
Static property asymmetric visibility was left out of the original RFC,
because it seemed like it would be hard and of little use. Turns out,
Ilija found a way to make it easy. (Ilija is smart.) So here's a
small RFC to add aviz to static properties, because we can't think of a
reason to NOT do so.The RFC has the status “Draft” and does not appear to be linked in the
overview at PHP: rfc. Is it intended to be open for
discussion yet?
Paperwork oversight on my part. Fixed now, thanks.
Regarding the RFC contents: I am not sure the behavior of `private(set)`
implying `final` is obvious for static properties. Consider this example
(Online PHP editor | output for bvQjM):<?php
class Foo {
public static $baz;
}class Bar extends Foo {
public static $baz;
}Foo::$baz = '2';
Bar::$baz = '1';
var_dump(Foo::$baz, Bar::$baz);Non-static properties only have `$this->` for property access, but with
static properties you can distinguish between `self::` and `static::`,
which makes overriding a `private(set)` static property meaningful,
because they are actually independent properties.Best regards
Tim Düsterhus
Hm, interesting point. I am not sure of the best way to model that.
Thinking aloud, my expectation would be that it behaves similarly to how final static methods would behave. Which appears to be a syntax error: Online PHP editor | output for j8mp0
So, shouldn't properties do the same?
--Larry Garfield
On Tue, Nov 26, 2024, at 3:42 PM, Jonathan Vollebregt wrote:
On 11/26/24 9:35 PM, Larry Garfield wrote:
Thinking aloud, my expectation would be that it behaves similarly to how final static methods would behave. Which appears to be a syntax error: Online PHP editor | output for j8mp0
So, shouldn't properties do the same?
Without final you can still override both private static properties and
private static methods: Online PHP editor | output for MS73YWhen explicitly declared final, static properties do result in a syntax
error: Online PHP editor | output for fqI8vRe-reading the logic in the original aviz RFC makes me think implicit
final here is unnecessary. All static properties are "Shadowed" like
private properties (even when they're public) so there's no conflicting
behavior.The two behaviors described as conflicting in the aviz RFC are decided
explicitly in the context of static properties, by the caller accessing
it with `self::` or `static::`. Not by a combination of the visibility
and child classes.Consider this example:
class A { private(set) static int $a = 1; public static function test(int $val) { static::$a = $val; } } class B extends A { private(set) static int $a = 2; } B::test(3)Yes this would produce a fatal error, but doing this with just `private
static` does the same in current PHP: Online PHP editor | output for Y6lZ7You might want to discuss banning use of `static::` on private statics,
but that's a big BC break.Since static properties do still have to have equal or wider visibility
when extending I'd say using `static::$prop` on a property you know is
private is a known risk and remove the implicit final.- Jonathan
Hi folks. Finally getting back to this thread.
Ilija and I have discussed it, and it seems to us that the consistency of implicit-final-on-private-set (on both object and static properties) is more valuable than maximal flexibility, especially as we're dealing with a rare use case to begin with.
Specifically, `static::$prop` is still expected to behave the same as the parent class's $prop. Eg, its type can't change, its visibility cannot be narrowed, etc. So a private(set) parent $prop would violate that expectation, the same way it would for a dynamic property. Introducing another subtle difference between self:: and static:: doesn't seem like a good thing for DX.
Additionally, if we find in the future that it really would be better to not have the implicit final and allow people to "opt in" via self::vs static::, it's less of a BC break to remove the implicit final in the future than to try and introduce it.
For that reason, we are going to keep static properties consistent with object properties in this regard.
--Larry Garfield
On Mon, Nov 25, 2024, at 10:52 AM, Larry Garfield wrote:
Static property asymmetric visibility was left out of the original RFC,
because it seemed like it would be hard and of little use. Turns out,
Ilija found a way to make it easy. (Ilija is smart.) So here's a
small RFC to add aviz to static properties, because we can't think of a
reason to NOT do so.
Heads up: Absent any other feedback, I'm going to call the vote on this RFC early next week.
--Larry Garfield
On 04/01/2025 00:14, Larry Garfield wrote:
On Mon, Nov 25, 2024, at 10:52 AM, Larry Garfield wrote:
Static property asymmetric visibility was left out of the original RFC,
because it seemed like it would be hard and of little use. Turns out,
Ilija found a way to make it easy. (Ilija is smart.) So here's a
small RFC to add aviz to static properties, because we can't think of a
reason to NOT do so.Heads up: Absent any other feedback, I'm going to call the vote on this RFC early next week.
--Larry Garfield
Hi
I'm not sure how I feel about this.
The current implementation actually uses a workaround because otherwise it interferes with cache slot merging.
This creates duplicated code in the VM handlers. Although the complication is small, it still is one. And the complication may be duplicated in the future in the JIT.
Workarounds/complications can be fine if having the feature is justifiable, but you said yourself that it seems of little use and the RFC text says it's just for completeness.
So honestly, I'm conflicted and leaning towards voting "no".
Kind regards
Niels
Hi Niels
On Tue, Jan 7, 2025 at 7:37 PM Niels Dossche <dossche.niels@gmail.com> wrote:
On 04/01/2025 00:14, Larry Garfield wrote:
>>
>> PHP: rfc:static-aviz
I'm not sure how I feel about this.
The current implementation actually uses a workaround because otherwise it interferes with cache slot merging.
This creates duplicated code in the VM handlers. Although the complication is small, it still is one. And the complication may be duplicated in the future in the JIT.
Workarounds/complications can be fine if having the feature is justifiable, but you said yourself that it seems of little use and the RFC text says it's just for completeness.
So honestly, I'm conflicted and leaning towards voting "no".
I wouldn't necessarily call this a workaround, but more of a missed
optimization (one branch for each static property write that is
unlikely to mispredict). If you wish, I can have a look at separating
cache slots. This may lead to a slowdown due to cache priming (as only
R/IS and RW/W/UNSET will be shared). To be honest, I doubt either of
those will lead to a measurable difference in real code. I can confirm
this by running some benchmarks. The benefit of separate cache slots
is that it can be entirely handled in
zend_fetch_static_property_address(), which would reduce VM/ future
JIT changes, although the separation of the cache slots themselves
might be more complex (given how it's currently implemented).
Ilija
On 07/01/2025 19:49, Ilija Tovilo wrote:
I wouldn't necessarily call this a workaround, but more of a missed
optimization (one branch for each static property write that is
unlikely to mispredict). If you wish, I can have a look at separating
cache slots. This may lead to a slowdown due to cache priming (as only
R/IS and RW/W/UNSET will be shared). To be honest, I doubt either of
those will lead to a measurable difference in real code. I can confirm
this by running some benchmarks. The benefit of separate cache slots
is that it can be entirely handled in
zend_fetch_static_property_address(), which would reduce VM/ future
JIT changes, although the separation of the cache slots themselves
might be more complex (given how it's currently implemented).
I think splitting cache slots will indeed be worse than what it is now.
However, that argument in itself just doesn't really convince me honestly.
You are likely right about the branch prediction, but the CPU still has to do some extra work; and it's also just more things to think about as a maintainer.
I'll think about it more, maybe I'll just abstain from voting.
Kind regards
Niels
Le 25 nov. 2024 à 17:52, Larry Garfield <larry@garfieldtech.com> a écrit :
Static property asymmetric visibility was left out of the original RFC, because it seemed like it would be hard and of little use. Turns out, Ilija found a way to make it easy. (Ilija is smart.) So here's a small RFC to add aviz to static properties, because we can't think of a reason to NOT do so.
Hi,
About use case: Looking at my code, I’ve found places where I store data (e.g. global config values) in public static properties, but those data is supposed to be initialised and/or modified only inside the class.
In those cases, aviz would allow me to document and enforce what is currently a gentleman's agreement.
—Claude
On Tue, Jan 7, 2025, at 1:21 PM, Niels Dossche wrote:
On 07/01/2025 19:49, Ilija Tovilo wrote:
I wouldn't necessarily call this a workaround, but more of a missed
optimization (one branch for each static property write that is
unlikely to mispredict). If you wish, I can have a look at separating
cache slots. This may lead to a slowdown due to cache priming (as only
R/IS and RW/W/UNSET will be shared). To be honest, I doubt either of
those will lead to a measurable difference in real code. I can confirm
this by running some benchmarks. The benefit of separate cache slots
is that it can be entirely handled in
zend_fetch_static_property_address(), which would reduce VM/ future
JIT changes, although the separation of the cache slots themselves
might be more complex (given how it's currently implemented).I think splitting cache slots will indeed be worse than what it is now.
However, that argument in itself just doesn't really convince me
honestly.
You are likely right about the branch prediction, but the CPU still has
to do some extra work; and it's also just more things to think about as
a maintainer.
I'll think about it more, maybe I'll just abstain from voting.Kind regards
Niels
Late follow-up here. Ilija finally managed to squeeze in time to benchmark this. Synthetic benchmarks were inconclusive. A practical benchmark using Symfony Demo showed the code from this patch as 0.03% faster than HEAD, which is within the margin of error of the benchmark, so it's basically a wash/no-impact.
With that settled, I will be opening the vote on this RFC tomorrow sometime, baring any last second questions or complications.
--Larry Garfield