Skip to content
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

RFC: Anonymous Class Lexical Scope #1871

Closed
wants to merge 1 commit into from
Closed

RFC: Anonymous Class Lexical Scope #1871

wants to merge 1 commit into from

Conversation

krakjoe
Copy link
Member

@krakjoe krakjoe commented Apr 19, 2016

RFC


if (zend_is_auto_global(var_name)) {
zend_error_noreturn(E_COMPILE_ERROR, "Cannot use auto-global as lexical variable");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens with use($a, $a) and similar?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solved for use($a, $a), but properties are a bit difficult to detect duplicates of, I think ...

@Majkl578
Copy link
Contributor

💯 This is definitely one of the main reasons why writing anonymous classes is irritating now.
What would be really cool is to specify passed variables, rename them and possibly also specify visibility.
For example this:

new class use ($foo->bar as private $bar)

would create private property `$bar.

This would be also reusable for passing $this:

new class use ($this as private $outsideScope)

@jesseschalken
Copy link

Very cool! I don't know where to comment on the RFC so I'll comment here.

If this is the syntax for variable capture in anonymous functions:

function () use ($glow) {
    print $glow;
}

I would naively expect this to be the syntax for variable capture in anonymous classes:

new class () use ($glow) {
    public function printGlow() {
        print $glow;
    }
}

But the correct code is actually:

new class () use ($glow) {
    public function printGlow() {
        print $this->glow;
    }
}

This seems jarring to me. $this->glow looks like a reference to a property that isn't defined, and use ($glow) looks like a variable being captured that isn't used.

Might I suggest actually requiring the public/protected/private keyword in the use clause for an anonymous class. This makes it obvious that you're declaring a property and not a variable, and keeps the door open for adding direct variable capture (not via a property) as a feature later.

new class () use (private $glow) {
    public function printGlow() {
        print $this->glow;
    }
}

This also has a handy correspondence with the property+constructor initialization shorthand in Hack and TypeScript:

class Foo {
    public function __construct(private $glow) {
    }
}

zend_string_copy(
zend_ast_get_str(use_var->child[1]));

zend_compile_prop(&prop, use_var, BP_VAR_R);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For use(&$foo->bar) shouldn't this compile BP_VAR_W?

@jesseschalken
Copy link

Another possible alternative:

function foo($glow) {
    new class () {
        private $glow = $glow;
        public function printGlow() {
            print $this->glow;
        }
    }
}

This makes it obvious how to use a different name, and possibly permits an arbitrary expression rather than just $var and $this->var.

@krakjoe
Copy link
Member Author

krakjoe commented Apr 20, 2016

This patch is actually completely wrong, it's so obvious this morning ...

The syntax can be changed after the fact, there is a problem with the implementation that must be fixed first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants