Skip to content

static $var = xxx should support any expression #8959

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mvorisek opened this issue Jul 9, 2022 · 8 comments
Closed

static $var = xxx should support any expression #8959

mvorisek opened this issue Jul 9, 2022 · 8 comments

Comments

@mvorisek
Copy link
Contributor

mvorisek commented Jul 9, 2022

Description

https://3v4l.org/gTufh

https://3v4l.org/G1EiN - this works, static $var can hold an object, only the initial value declaration needs longer code

<?php

class Cl {
    public function scalar(): void
    {
        static $counter = 0;
        
        $counter++;
        
        var_dump($counter);
    }

    public function obj(): void
    {
        static $counter = new class() {
            public int $n = 0;
        };
        
        $counter->n++;
        
        var_dump($counter->n);
    }
}

$o = new Cl();
$o->scalar();
$o->scalar();
$o->obj();
$o->obj();

$o = new class() extends Cl {};
$o->scalar();
$o->obj();

Currently php emits fatal error for static $var = new class() {};.

But static $var = ...; should support any expression like if the assign op would not be prefixed with static as such prefix only defines the statement should be evaluated only once per php request lifecycle & the variable kept alive.

It seems like some historical php limitation...

Instead of a fatal error, I expect this result for the test case above:

int(1)
int(2)
int(1)
int(2)
int(3)
int(3)
@cmb69
Copy link
Member

cmb69 commented Jul 9, 2022

It seems like some historical php limitation...

No. That has been explicitly excluded from the "new in initializers" RFC:

The use of a dynamic or non-string class name or an anonymous class is not allowed.

See the "Order of evaluation" section to get some insight why supporting arbitrary expressions might not be a good idea, or at the very least needs careful consideration.

@mvorisek
Copy link
Contributor Author

mvorisek commented Jul 9, 2022

Thank you for pointing to the RFC. It seems the RFC actually claims it will be allowed :), citing:

New expressions are allowed in parameter default values, attribute arguments, static variable initializers and global class constant initializers.

It seems you meant "new expressions in (static and non-static) property initializers and class constant initializers". But this issue is about "static variable initializers".

The "Order of evaluation" section clearly discuss when "static variable initializers" will be evaluated:

Static variable initializers are evaluated when control flow reaches the static variable declaration.

So I belive this does not require a new RFC and could be maybe even considered as a bug.

@cmb69
Copy link
Member

cmb69 commented Jul 9, 2022

The use of a dynamic or non-string class name or an anonymous class is not allowed.

@mvorisek
Copy link
Contributor Author

mvorisek commented Jul 9, 2022

I see. The issue is not that a new is unsupported in static variable initializers, but only "new anonymous class" is unsupported.

In the RFC there is one example:

function test(
    $b = new class {}, // anonymous class
) {}

What is the reason behind this decision? Unclear order of evaluation when using a reflection?

For static $var = ... (in code, not as property) the order of evaluation is defined by:

Static variable initializers are evaluated when control flow reaches the static variable declaration.

so the usecase from this issue description should be supported and probably no RFC should be required, IMHO there is no ambiguity, as it is more or less syntactic sugar to https://3v4l.org/G1EiN.

@nikic
Copy link
Member

nikic commented Jul 24, 2022

Making static support arbitrary expressions should be fine -- this should largely be an implementation problem, as the current initialization approach will not work.

I expect the largest complication would be the existence of ReflectionFunction::getStaticVariables(), because static variables with arbitrary (context-dependent) initializers cannot be evaluated in isolation. The semantics of the reflection method would likely have to change towards only returning those static variables that have already been dynamically reached.

@github-actions
Copy link

There has not been any recent activity in this feature request. It will automatically be closed in 14 days if no further action is taken. Please see https://github.com/probot/stale#is-closing-stale-issues-really-a-good-idea to understand why we auto-close stale feature requests.

@github-actions github-actions bot added Stale and removed Stale labels Oct 23, 2022
@mvorisek
Copy link
Contributor Author

dear stale bot, it will be fixed by #9301 :)

@nielsdos
Copy link
Member

nielsdos commented Jul 6, 2023

This wasn't closed automatically because of the manual merging. I'll close it now.

@nielsdos nielsdos closed this as completed Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants