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

Reply via email to