Allow "self" and "parent" as parent of anonymous classes

[thekid]

Fixes bug 71678 and allows the following (which currently gives a fatal error):

class T {
  static function new() {
    return new class() extends self { };
  }
}

var_dump(T::new());

The original RFC neither states this should be allowed nor that it is an error - maybe @philsturgeon and @krakjoe want to have a look at this?

[thekid]

Oh, on HHVM this also yields a fatal error, but for a different reason:

$ php -r 'class T { static function new() { return new class() extends self { }; } }'

Fatal error: unknown class self in /tmp/php-wrap-rylTB2 on line 2

[bwoebi]

Usually we don't put EXPECTED() in compiler, but if you include it, it should really be UNEXPECTED(). Branches leading to errors should always be UNEXPECTED(). [in fact zend_error_noreturn is marked as ZEND_COLD which is why that's totally redundant.]

[laruence]

@thekid I think we should change(improve) the error message in this case instead of allowing this. actually I can not think out how this is useful. thanks

[marcioAlmada]

@laurence this is useful solely to create an augmented version of the current class without repeating the class name itself.

[laruence]

@marcioAlmada hmm, okey, although I think it's a little narrow...

[thekid]

actually I can not think out how this is useful

I've been using this in an "typesafe enum" pattern which works much like the second example listed at https://wiki.php.net/rfc/enum#future_scope (but without the syntactic support, of course) and uses a static constructor (like https://wiki.php.net/rfc/static_class_constructor suggests) to initialize the members.

abstract class Operation {
  public static $plus, $minus;

  static function __static() {
    self::$plus= new class() extends self {  // I currently need "extends Operation" here
      static function __static() { }
      function apply($a, $b) { return $a + $b; }
    }
    // ...
  }
  abstract function apply($a, $b);
}
Operation::__static(); // Actually called inside framework's autoloading

// Later on:
$result= Operation::$plus->apply($a, $b);

The motivation to create a bugreport was to make self (and parent) work consistently in all places. It already works in type hints and return types, for new, for static calls, etc., why not here?

[thekid]

Usually we don't put EXPECTED() in compiler

Thanks for your feedback, I've removed it.

[jesseschalken]

Since self and parent (but not static) can be resolved entirely within the parser (right?) [edit: wrong, see Closure::bind()], I don't see why you wouldn't permit them in any place a class name is allowed. It's a fairly straightforward syntactic shortcut, and it's surprising they work in some places but not others.

[nikic]

If we want to do this change, lets make sure it's general and not only for some subset again:

• The check should be dropped entirely, not just for anon classes and replaced with the zend_ensure_valid_class_fetch_type sanity check only.
• Needs test that static works as well.
• Needs test that self is bound correctly in a trait.
• Needs test for use of self in inheritance inside a scope-rebound closure (preferably inside and outside a class).

I think the change itself is fine -- we allow self etc pretty much everywhere, but not here. Previously it just wasn't terribly useful in this context, but now with anon classes it makes more sense. One concern is that this use is somewhat ambiguous, as self might refer to the defined class, rather than the wrapping one.

One other place where we currently don't allow self etc are catch clauses -- though supporting it there might be more problematic (technically).

[thekid]

• Needs test that self is bound correctly in a trait.
• Needs test for use of self in inheritance inside a scope-rebound closure (preferably inside and outside a class).

[thekid]

The check should be dropped entirely, not just for anon classes

This check prevents extending self and parent from regular classes. Changing it to use zend_ensure_valid_class_fetch_type() also changes the error message for the better:

$ php -r 'class Test extends self { }'
Fatal error: Cannot use 'self' as class name as it is reserved in Command line code on line 1

$ ./sapi/cli/php -r 'class Test extends self { }'
Compile error: Cannot use 'self' when no class scope is active

Currently, however, zend_ensure_valid_class_fetch_type() will not work in this case, since it tests the following:

1. fetch_type != ZEND_FETCH_CLASS_DEFAULT => yep, ZEND_FETCH_CLASS_SELF
2. !CG(active_class_entry) => yep, that's NULL
3. zend_is_scope_known() => fail, the scope is unknown, cause were in the main scope

Not sure whether the logic is incorrect or whether I'm misunderstanding something here:

static void zend_ensure_valid_class_fetch_type(uint32_t fetch_type) /* {{{ */
{
    if (fetch_type != ZEND_FETCH_CLASS_DEFAULT && !CG(active_class_entry) && zend_is_scope_known()) {
        zend_error_noreturn(E_COMPILE_ERROR, "Cannot use \"%s\" when no class scope is active",
            fetch_type == ZEND_FETCH_CLASS_SELF ? "self" :
            fetch_type == ZEND_FETCH_CLASS_PARENT ? "parent" : "static");
    }
}

In this case, it should be the other way around: !zend_is_scope_known(). However, changing that breaks quite a number of tests...

[thekid]

Needs test that static works as well.

This would require a change to the parser, as well. I'll look into it.

[jesseschalken]

Since static uses the class used at runtime, I don't see how extends static would work for anonymous classes. As far as I know PHP creates a real class with a special name to use where the anonymous class is instantiated, but if it extends static then it will need a seperate class for each thing static turns out to be at runtime.

It's also quite damaging to static analysis. Since it is impossible to know all the things static can be (e.g. a class in a library extended by users of the library), it's difficult or impossible for static analysis to do any class hierarchy checks, or even generate a class hierarchy including anonymous classes.

// in library
abstract class Foo {
    public static function baz() {
        return new class() extends static {
            public function name() {
                return 'foo';
            }
        };
    }
}

// in code using the library
final class Blah extends Foo {
    public final function name($pre) {
        return $pre . ' blah';
    }
}
// This will fail because the anonymous class will attempt to
// - extend a final class
// - override a final method
// - override a method with an incompatible signature
Blah::baz();

Unfortunately, all the same problems apply to using self and parent in traits. You cannot analyse new class(...) extends self/parent inside a trait without knowing all the classes using the trait.

[nikic]

Since static uses the class used at runtime, I don't see how extends static would work for anonymous classes. As far as I know PHP creates a real class with a special name to use where the anonymous class is instantiated, but if it extends static then it will need a seperate class for each thing static turns out to be at runtime.

Good point. Even without static, that problem already exists for self in closures.

[jesseschalken]

Good point. Even without static, that problem already exists for self in closures.

Are you referring to Closure::bind()/Closure::bindTo()? I didn't know those rebound self. I thought self and parent were resolved at the parser level. 😕 Must be why I never use those methods.

[thekid]

Syntactic support is added, but the problem @jesseschalken describes exists of course.

[thekid]

• Needs test that static works as well.

Now works with static. Implemented using special case handling for ZEND_FETCH_CLASS_STATIC during compile time and performing class resolution at runtime.

[nikic]

Re comment #1779 (comment), the reason for that code is that, as weird as that may seem, it is legal to use $this or self in pseudo-main scope, because the file may be included inside the body of a class method. If it isn't, a runtime error will be generated. This will probably make a bit more sense if you think about pure-PHP templating using $this. (This is why class Test extends self {} would be not wholly absurd -- it can be included in a class, where self will have meaning.)

With the static issue resolved, what do you think about the same problem with self and parent if scope is rebound in closures? Right now you'll be creating an anon class extending the class that was bound as the scope on the first call of the closure (which does not necessarily coincide with the original scope and also does not necessarily coincide with future scopes), if I'm reading the code correctly.

Honestly the whole thing makes me uncomfortable. One anon class clause will be able to generate objects of different classes, with those classes not even necessarily having a common ancestor :/ I think the whole thing becomes weird enough that it would be good to drop a mail on internals to get more input.

[marcioAlmada]

Perhaps we are looking for new class extends __CLASS__ {} ?

[php-pulls]

Comment on behalf of kalle at php.net:

Closing due to no activity, please re-open if you still intend on working on it