Hi Dmitry,
I saw that you commited this patch, with the addition of only replacing
persistent constants (just mentioning for others on the list). The attached
patches have a few tweaks:
The main thing I noticed is that if "something" creates a persistent,
case-INsensitive constant (any of those in "standard" PHP, besides
TRUE/FALSE/NULL?), it could still wrongly be substituted. My change
eliminates that possibility.
Checking Z_TYPE(c->value) != IS_CONSTANT[_ARRAY] isn't needed with the
persistent check now, is it?
>From orginal patch (zend_constants.c part), the memcmp(...) != 0 isn't
needed as it will always be true. If it wasn't, the *first* hash lookup
wouldn't have failed. :-) Like I said in the original message, it's old
code from a long time ago, but was never removed...
- Matt
----- Original Message -----
From: "Dmitry Stogov"
Sent: Thursday, July 24, 2008
> I would propose the attached patch for this optimization.
>
> Opcode caches and encoders will have to disable this optimization with
> ZEND_COMPILE_NO_CONSTANT_SUBSTITUTION
>
> Any objections?
>
> Thanks. Dmitry.
>
----------------------------------------------------------------------------
----
> Index: Zend/zend_compile.c
> ===================================================================
> RCS file: /repository/ZendEngine2/zend_compile.c,v
> retrieving revision 1.647.2.27.2.41.2.74
> diff -u -p -d -r1.647.2.27.2.41.2.74 zend_compile.c
> --- Zend/zend_compile.c 24 Jul 2008 11:47:49 -0000 1.647.2.27.2.41.2.74
> +++ Zend/zend_compile.c 24 Jul 2008 14:40:12 -0000
> @@ -3804,6 +3804,12 @@ static zend_constant* zend_get_ct_const(
> if (c->flags & CONST_CT_SUBST) {
> return c;
> }
> + if (!CG(current_namespace) &&
> + !(CG(compiler_options) & ZEND_COMPILE_NO_CONSTANT_SUBSTITUTION) &&
> + Z_TYPE(c->value) != IS_CONSTANT &&
> + Z_TYPE(c->value) != IS_CONSTANT_ARRAY) {
> + return c;
> + }
> return NULL;
> }
> /* }}} */
> @@ -5169,12 +5175,14 @@ void zend_do_use(znode *ns_name, znode *
> void zend_do_declare_constant(znode *name, znode *value TSRMLS_DC) /*
{{{ */
> {
> zend_op *opline;
> + zend_constant *c;
>
> if(Z_TYPE(value->u.constant) == IS_CONSTANT_ARRAY) {
> zend_error(E_COMPILE_ERROR, "Arrays are not allowed as constants");
> }
>
> - if (zend_get_ct_const(&name->u.constant TSRMLS_CC)) {
> + c = zend_get_ct_const(&name->u.constant TSRMLS_CC);
> + if (c && (c->flags & CONST_CT_SUBST)) {
> zend_error(E_COMPILE_ERROR, "Cannot redeclare constant '%s'",
Z_STRVAL(name->u.constant));
> }
>
> Index: Zend/zend_compile.h
> ===================================================================
> RCS file: /repository/ZendEngine2/zend_compile.h,v
> retrieving revision 1.316.2.8.2.12.2.27
> diff -u -p -d -r1.316.2.8.2.12.2.27 zend_compile.h
> --- Zend/zend_compile.h 24 Jul 2008 11:47:49 -0000 1.316.2.8.2.12.2.27
> +++ Zend/zend_compile.h 24 Jul 2008 14:40:13 -0000
> @@ -762,6 +762,9 @@ END_EXTERN_C()
> /* generate ZEND_DECLARE_INHERITED_CLASS_DELAYED opcode to delay early
binding */
> #define ZEND_COMPILE_DELAYED_BINDING (1<<4)
>
> +/* disable constant substitution at compile-time */
> +#define ZEND_COMPILE_NO_CONSTANT_SUBSTITUTION (1<<5)
> +
> /* The default value for CG(compiler_options) */
> #define ZEND_COMPILE_DEFAULT ZEND_COMPILE_HANDLE_OP_ARRAY
>
>
Index: Zend/zend_compile.c
===================================================================
RCS file: /repository/ZendEngine2/zend_compile.c,v
retrieving revision 1.832
diff -u -r1.832 zend_compile.c
--- Zend/zend_compile.c 25 Jul 2008 04:54:56 -0000 1.832
+++ Zend/zend_compile.c 25 Jul 2008 09:30:00 -0000
@@ -3996,24 +3996,20 @@
zstr lookup_name = zend_u_str_case_fold(Z_TYPE_P(const_name),
Z_UNIVAL_P(const_name), Z_UNILEN_P(const_name), 1, &lookup_name_len);
if (zend_u_hash_find(EG(zend_constants), Z_TYPE_P(const_name),
lookup_name, lookup_name_len+1, (void **) &c)==SUCCESS) {
- if ((c->flags & CONST_CS) && memcmp(c->name.v,
Z_UNIVAL_P(const_name).v,
UG(unicode)?UBYTES(Z_USTRLEN_P(const_name)):Z_STRLEN_P(const_name))!=0) {
+ if ((c->flags & CONST_CT_SUBST) && !(c->flags &
CONST_CS)) {
efree(lookup_name.v);
- return NULL;
+ return c;
}
- } else {
- efree(lookup_name.v);
- return NULL;
}
efree(lookup_name.v);
+ return NULL;
}
if (c->flags & CONST_CT_SUBST) {
return c;
}
if ((c->flags & CONST_PERSISTENT) &&
!CG(current_namespace) &&
- !(CG(compiler_options) & ZEND_COMPILE_NO_CONSTANT_SUBSTITUTION) &&
- Z_TYPE(c->value) != IS_CONSTANT &&
- Z_TYPE(c->value) != IS_CONSTANT_ARRAY) {
+ !(CG(compiler_options) & ZEND_COMPILE_NO_CONSTANT_SUBSTITUTION)) {
return c;
}
return NULL;
Index: Zend/zend_constants.c
===================================================================
RCS file: /repository/ZendEngine2/zend_constants.c,v
retrieving revision 1.111
diff -u -r1.111 zend_constants.c
--- Zend/zend_constants.c 21 Jul 2008 09:36:21 -0000 1.111
+++ Zend/zend_constants.c 25 Jul 2008 09:34:30 -0000
@@ -278,7 +278,7 @@
lookup_name = zend_u_str_case_fold(type, name, name_len, 1,
&lookup_name_len);
if (zend_u_hash_find(EG(zend_constants), type, lookup_name,
lookup_name_len+1, (void **) &c)==SUCCESS) {
- if ((c->flags & CONST_CS) && memcmp(c->name.v, name.v,
UG(unicode)?UBYTES(name_len):name_len)!=0) {
+ if (c->flags & CONST_CS) {
retval=0;
}
} else {
Index: Zend/zend_compile.c
===================================================================
RCS file: /repository/ZendEngine2/zend_compile.c,v
retrieving revision 1.647.2.27.2.41.2.77
diff -u -r1.647.2.27.2.41.2.77 zend_compile.c
--- Zend/zend_compile.c 25 Jul 2008 04:54:08 -0000 1.647.2.27.2.41.2.77
+++ Zend/zend_compile.c 25 Jul 2008 09:30:00 -0000
@@ -3791,24 +3791,20 @@
char *lookup_name =
zend_str_tolower_dup(Z_STRVAL_P(const_name), Z_STRLEN_P(const_name));
if (zend_hash_find(EG(zend_constants), lookup_name,
Z_STRLEN_P(const_name)+1, (void **) &c)==SUCCESS) {
- if ((c->flags & CONST_CS) && memcmp(c->name,
Z_STRVAL_P(const_name), Z_STRLEN_P(const_name))!=0) {
+ if ((c->flags & CONST_CT_SUBST) && !(c->flags &
CONST_CS)) {
efree(lookup_name);
- return NULL;
+ return c;
}
- } else {
- efree(lookup_name);
- return NULL;
}
efree(lookup_name);
+ return NULL;
}
if (c->flags & CONST_CT_SUBST) {
return c;
}
if ((c->flags & CONST_PERSISTENT) &&
!CG(current_namespace) &&
- !(CG(compiler_options) & ZEND_COMPILE_NO_CONSTANT_SUBSTITUTION) &&
- Z_TYPE(c->value) != IS_CONSTANT &&
- Z_TYPE(c->value) != IS_CONSTANT_ARRAY) {
+ !(CG(compiler_options) & ZEND_COMPILE_NO_CONSTANT_SUBSTITUTION)) {
return c;
}
return NULL;
Index: Zend/zend_constants.c
===================================================================
RCS file: /repository/ZendEngine2/zend_constants.c,v
retrieving revision 1.71.2.5.2.7.2.10
diff -u -r1.71.2.5.2.7.2.10 zend_constants.c
--- Zend/zend_constants.c 21 Jul 2008 09:41:00 -0000
1.71.2.5.2.7.2.10
+++ Zend/zend_constants.c 25 Jul 2008 09:34:30 -0000
@@ -231,7 +231,7 @@
lookup_name = zend_str_tolower_dup(name, name_len);
if (zend_hash_find(EG(zend_constants), lookup_name, name_len+1,
(void **) &c)==SUCCESS) {
- if ((c->flags & CONST_CS) && memcmp(c->name, name,
name_len) != 0) {
+ if (c->flags & CONST_CS) {
retval=0;
}
} else {
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php