Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

"::class resolution as scalar" Feature #187

Closed
wants to merge 3 commits into from

6 participants

@ralphschindler

FOR RFC: https://wiki.php.net/rfc/class_name_scalars

Patch addresses:

  • Allows for Name::class, self::class, static::class, parent::class resolution to a scalar based on current use rules and current namespace.
  • Reuses existing keyword "class"
  • Alters zend_compile.c\zend_resolve_class_name() to facilitate compile time class name resolution (and runtime by way of FCALL)
ralphschindler added some commits
@ralphschindler ralphschindler "::class resolution as scalar" Feature
* Allows for Name::class, self::class, static::class, parent::class resolution to a scalar based on current use rules and current namespace.
* Reuses existing keyword "class"
* Alters zend_compile.c\zend_resolve_class_name() to facilitate compile time class name resolution (and runtime by way of FCALL)
76703a0
@ralphschindler ralphschindler "::class" Feature:
* Added class scope checking for self,parent+static
* Fixed tab/spaces in phpt file
26d592a
@laruence
Owner

just a quick test with your patch, got a memleak:

<?php

class b {
  public function dummy() {
      echo self::class;
    }
}

lead to a memleak:

[Sun Sep  9 22:19:52 2012]  Script:  '/tmp/1.php'
Zend/zend_language_scanner.l(1907) :  Freeing 0x2B1498A80288 (5 bytes), script=/tmp/1.php
=== Total 1 memory leaks detected ===
@ralphschindler

I've located the leak and will work the fix out, thanks- (I forgot to compile with --enable-debug).

@ralphschindler ralphschindler "::class" Feature
* Fixed memory leak by adding zval_dtor() on class_name in zend_compile.c's zend_resolve_class_name()
41a10e7
@sebastianbergmann sebastianbergmann commented on the diff
Zend/tests/class_name_scalar.phpt
@@ -0,0 +1,56 @@
+--TEST--
+class name as scaler from ::class keyword

Nitpicking: scaler -> scalar

Indeed. Thanks for pointing this out.

I am aware that this implementation needs more work (there are additional edge cases). But I wanted to bring it to a vote to ensure that additional work on this feature would not ultimately be all-for-naught, as outside of core/master inclusion, this feature is effectively useless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@lstrojny

I am working on a few more tests and some remaining issues (parent::class in type hints and constants e.g.)

@lstrojny

@ralphschindler would love to get this merged, as the RFC ended positively. Could you drop me a message to discuss the remaining work?

@ralphschindler

I need to rework the patch as per the conversations that were had on the internals list. My plan is to do it during the holidays this week (American Thanksgiving), so I should be able to get this updated by end of week.

In its current state, it's not good for merge.

@lstrojny

@ralphschindler, cool, thanks! If you need help with anything, drop me a note.

@lstrojny

@ralphschindler ping again.

@nikic
Owner

@ralphschindler By the way, if you want this in 5.5 you need to hurry. We're close to feature freeze ;)

@ralphschindler

@nikic I have it mostly working, in two different forms, any chance you could help me bring it to completion?

@nikic
Owner

@ralphschindler Depends on how much help you need, but in principle yes :) What are you still missing / what issues are you having with the implementation?

@ralphschindler

@nikic I will push what I have to my github account, and give you a summary of the decisions I need to make.

Mostly, I need to decide how to handle situations like this:


class Foo {
    public function bar(static::class $foo, parent::class $bar, self::class $baz) {}
}

I believe I had self::class working in that scenario, but the other two were not with the current implementation of the FETCH_CLASS_CONST.

@nikic
Owner

@ralphschindler Assuming you mean $foo = static::class etc.

Parameter initializers (and all other initializers) in PHP only accept compile-time scalars. As static::class and parent::class are not resolvable at compile time they should just throw an error there. parent::class is theoretically resolvable at compile-time (because it only needs the name and not the actual CE) and you can obtain it by checking the op array for the FETCH_CLASS op of the class declaration. But I don't think that it's worth adding such a hack for something nobody will do anyway (what's supposed to be the use for initializing a parameter to the name of the parent class?)

@lstrojny

I agree with @nikic here. If parent and static throw an error, that’s more than fine.

@ralphschindler

Since I decided to go in a different direction with the implementation and wanted to keep a clean commit history, I re-branched from master and applied my fixes there and will open up a new PR. see #256

@Majkl578

@ralphschindler: You don't need a new pull request, just force-push to this branch, the history will update here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 8, 2012
  1. @ralphschindler

    "::class resolution as scalar" Feature

    ralphschindler authored
    * Allows for Name::class, self::class, static::class, parent::class resolution to a scalar based on current use rules and current namespace.
    * Reuses existing keyword "class"
    * Alters zend_compile.c\zend_resolve_class_name() to facilitate compile time class name resolution (and runtime by way of FCALL)
  2. @ralphschindler

    "::class" Feature:

    ralphschindler authored
    * Added class scope checking for self,parent+static
    * Fixed tab/spaces in phpt file
Commits on Sep 10, 2012
  1. @ralphschindler

    "::class" Feature

    ralphschindler authored
    * Fixed memory leak by adding zval_dtor() on class_name in zend_compile.c's zend_resolve_class_name()
This page is out of date. Refresh to see the latest.
View
56 Zend/tests/class_name_scalar.phpt
@@ -0,0 +1,56 @@
+--TEST--
+class name as scaler from ::class keyword

Nitpicking: scaler -> scalar

Indeed. Thanks for pointing this out.

I am aware that this implementation needs more work (there are additional edge cases). But I wanted to bring it to a vote to ensure that additional work on this feature would not ultimately be all-for-naught, as outside of core/master inclusion, this feature is effectively useless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+--FILE--
+<?php
+
+namespace Foo\Bar {
+ class One {}
+ class Two extends One {
+ public static function run() {
+ var_dump(self::class);
+ var_dump(static::class);
+ var_dump(parent::class);
+ var_dump(Baz::class);
+ }
+ }
+ class Three extends Two {}
+ echo "In NS\n";
+ var_dump(Moo::CLASS); // resolve in namespace
+}
+
+namespace {
+ use Bee\Bop as Moo,
+ Foo\Bar\One;
+ echo "Top\n";
+ var_dump(One::class); // resolve from use
+ var_dump(Boo::class); // resolve in global namespace
+ var_dump(Moo::CLASS); // resolve from use as
+ var_dump(\Moo::Class); // resolve fully qualified
+ $class = One::class; // assign class as scalar to var
+ $x = new $class; // create new class from original scalar assignment
+ var_dump($x);
+ Foo\Bar\Two::run(); // resolve runtime lookups
+ echo "Parent\n";
+ Foo\Bar\Three::run(); // resolve runtime lookups with inheritance
+}
+
+?>
+--EXPECTF--
+In NS
+string(11) "Foo\Bar\Moo"
+Top
+string(11) "Foo\Bar\One"
+string(3) "Boo"
+string(7) "Bee\Bop"
+string(3) "Moo"
+object(Foo\Bar\One)#1 (0) {
+}
+string(11) "Foo\Bar\Two"
+string(11) "Foo\Bar\Two"
+string(11) "Foo\Bar\One"
+string(11) "Foo\Bar\Baz"
+Parent
+string(11) "Foo\Bar\Two"
+string(13) "Foo\Bar\Three"
+string(11) "Foo\Bar\One"
+string(11) "Foo\Bar\Baz"
View
58 Zend/zend_compile.c
@@ -2109,6 +2109,8 @@ void zend_resolve_class_name(znode *class_name, ulong fetch_type, int check_ns_n
zval **ns;
znode tmp;
int len;
+ int lctype;
+ zend_op *opline;
compound = memchr(Z_STRVAL(class_name->u.constant), '\\', Z_STRLEN(class_name->u.constant));
if (compound) {
@@ -2153,7 +2155,7 @@ void zend_resolve_class_name(znode *class_name, ulong fetch_type, int check_ns_n
*class_name = tmp;
}
}
- } else if (CG(current_import) || CG(current_namespace)) {
+ } else {
/* this is a plain name (without \) */
lcname = zend_str_tolower_dup(Z_STRVAL(class_name->u.constant), Z_STRLEN(class_name->u.constant));
@@ -2163,13 +2165,53 @@ void zend_resolve_class_name(znode *class_name, ulong fetch_type, int check_ns_n
zval_dtor(&class_name->u.constant);
class_name->u.constant = **ns;
zval_copy_ctor(&class_name->u.constant);
- } else if (CG(current_namespace)) {
- /* plain name, no import - prepend current namespace to it */
- tmp.op_type = IS_CONST;
- tmp.u.constant = *CG(current_namespace);
- zval_copy_ctor(&tmp.u.constant);
- zend_do_build_namespace_name(&tmp, &tmp, class_name TSRMLS_CC);
- *class_name = tmp;
+ } else {
+ lctype = zend_get_class_fetch_type(lcname, strlen(lcname));
+ switch (lctype) {
+ case ZEND_FETCH_CLASS_SELF:
+ if (!CG(active_class_entry)) {
+ zend_error(E_COMPILE_ERROR, "Cannot access self::class when no class scope is active");
+ }
+ zval_dtor(&class_name->u.constant);
+ class_name->op_type = IS_CONST;
+ ZVAL_STRINGL(&class_name->u.constant, CG(active_class_entry)->name, CG(active_class_entry)->name_length, 1);
+ break;
+ case ZEND_FETCH_CLASS_STATIC:
+ if (!CG(active_class_entry)) {
+ zend_error(E_COMPILE_ERROR, "Cannot access static::class when no class scope is active");
+ }
+ case ZEND_FETCH_CLASS_PARENT:
+ if (!CG(active_class_entry)) {
+ zend_error(E_COMPILE_ERROR, "Cannot access parent::class when no class scope is active");
+ }
+ zval_dtor(&class_name->u.constant);
+ opline = get_next_op(CG(active_op_array) TSRMLS_CC);
+ opline->opcode = ZEND_DO_FCALL;
+ opline->result.var = get_temporary_variable(CG(active_op_array));
+ opline->result_type = IS_VAR;
+ if (lctype == ZEND_FETCH_CLASS_STATIC) {
+ LITERAL_STRINGL(opline->op1, estrndup("get_called_class", sizeof("get_called_class")-1), sizeof("get_called_class")-1, 0);
+ } else {
+ LITERAL_STRINGL(opline->op1, estrndup("get_parent_class", sizeof("get_parent_class")-1), sizeof("get_parent_class")-1, 0);
+ }
+ CALCULATE_LITERAL_HASH(opline->op1.constant);
+ opline->op1_type = IS_CONST;
+ GET_CACHE_SLOT(opline->op1.constant);
+ opline->extended_value = 0;
+ SET_UNUSED(opline->op2);
+ GET_NODE(class_name, opline->result);
+ break;
+ case ZEND_FETCH_CLASS_DEFAULT:
+ if (CG(current_namespace)) {
+ /* plain name, no import - prepend current namespace to it */
+ tmp.op_type = IS_CONST;
+ tmp.u.constant = *CG(current_namespace);
+ zval_copy_ctor(&tmp.u.constant);
+ zend_do_build_namespace_name(&tmp, &tmp, class_name TSRMLS_CC);
+ *class_name = tmp;
+ }
+ break;
+ }
}
efree(lcname);
}
View
6 Zend/zend_language_parser.y
@@ -942,6 +942,7 @@ common_scalar:
static_scalar: /* compile-time evaluated scalars */
common_scalar { $$ = $1; }
+ | class_name_scalar { $$ = $1; }
| namespace_name { zend_do_fetch_constant(&$$, NULL, &$1, ZEND_CT, 1 TSRMLS_CC); }
| T_NAMESPACE T_NS_SEPARATOR namespace_name { $$.op_type = IS_CONST; ZVAL_EMPTY_STRING(&$$.u.constant); zend_do_build_namespace_name(&$$, &$$, &$3 TSRMLS_CC); $3 = $$; zend_do_fetch_constant(&$$, NULL, &$3, ZEND_CT, 0 TSRMLS_CC); }
| T_NS_SEPARATOR namespace_name { char *tmp = estrndup(Z_STRVAL($2.u.constant), Z_STRLEN($2.u.constant)+1); memcpy(&(tmp[1]), Z_STRVAL($2.u.constant), Z_STRLEN($2.u.constant)+1); tmp[0] = '\\'; efree(Z_STRVAL($2.u.constant)); Z_STRVAL($2.u.constant) = tmp; ++Z_STRLEN($2.u.constant); zend_do_fetch_constant(&$$, NULL, &$2, ZEND_CT, 0 TSRMLS_CC); }
@@ -959,6 +960,7 @@ static_class_constant:
scalar:
T_STRING_VARNAME { $$ = $1; }
+ | class_name_scalar { $$ = $1; }
| class_constant { $$ = $1; }
| namespace_name { zend_do_fetch_constant(&$$, NULL, &$1, ZEND_RT, 1 TSRMLS_CC); }
| T_NAMESPACE T_NS_SEPARATOR namespace_name { $$.op_type = IS_CONST; ZVAL_EMPTY_STRING(&$$.u.constant); zend_do_build_namespace_name(&$$, &$$, &$3 TSRMLS_CC); $3 = $$; zend_do_fetch_constant(&$$, NULL, &$3, ZEND_RT, 0 TSRMLS_CC); }
@@ -1200,6 +1202,10 @@ class_constant:
| variable_class_name T_PAAMAYIM_NEKUDOTAYIM T_STRING { zend_do_fetch_constant(&$$, &$1, &$3, ZEND_RT, 0 TSRMLS_CC); }
;
+class_name_scalar:
+ class_name T_PAAMAYIM_NEKUDOTAYIM T_CLASS { zend_resolve_class_name(&$1, ZEND_FETCH_CLASS_GLOBAL, 1 TSRMLS_CC); $$ = $1; }
+;
+
%%
/* Copy to YYRES the contents of YYSTR after stripping away unnecessary
Something went wrong with that request. Please try again.