Hi Matt,
I updated your patch a little bit to make it more clear (from my point
of view).
Please take a look.
Thanks. Dmitry.
Matt Wilmas wrote:
> Hi Dmitry,
>
> Well, it's been awhile since Alpha 1 :-), so I wanted to finally resend this
> before Alpha 2! I agree that the additional optimization probably wouldn't
> happen often, as there won't be that much namespace usage right away, I
> assume. But I think it makes sense to handle :: prefix constants, since
> they're known to have global scope, and I can see the future [online]
> optimization tip: "When using namespaces, use :: for global constants to get
> compile-time substitution." ;-)
>
> The code is the same as before, just updated the patch. It doesn't really
> seem less clear, to me, than the current code. In zend_do_fetch_constant(),
> the "check_namespace" variable may be better named something like
> "from_namespace" according to my changes, to be more clear, but I left that
> out to make the patch as simple as possible.
>
> http://realplain.com/php/ct_const_fixes.diff
> http://realplain.com/php/ct_const_fixes_5_3.diff
>
>
> Thanks,
> Matt
>
>
> ----- Original Message -----
> From: "Dmitry Stogov"
> Sent: Thursday, July 31, 2008
>
>> Hi Matt,
>>
>> Now I know. :)
>> Anyway, I don't think this optimization will work often.
>>
>> Send me the patch after Alpha1 release.
>>
>> Thanks. Dmitry.
>>
>>
>> Matt Wilmas wrote:
>>> Hi Dmitry,
>>>
>>> Do you know that with your changes, no substitution will happen in a
>>> namespace even when using :: prefix? :-/ (That's what I would do when I
>>> know it's global, for optimization.) Or is that what you meant by "not
> so
>>> optimal?"
>>>
>>>
>>> - Matt
>>>
>>>
>>> ----- Original Message -----
>>> From: "Dmitry Stogov"
>>> Sent: Thursday, July 31, 2008
>>>
>>>> Thanks Matt. I committed near the same patch.
>>>> It's not so optimal, but little bit more clear.
>>>>
>>>> Thanks. Dmitry.
>>>>
>>>> Matt Wilmas wrote:
>>>>> Hi Dmitry,
>>>>>
>>>>> For the behavior change that I mentioned in the other thread, with
> this
>>>>> code:
>>>>>
>>>>> function foo() {
>>>>> static $a = -PHP_INT_MAX;
>>>>> }
>>>>>
>>>>> Which could work sometimes, and sometimes not (if in a namespace or
>>>>> ZEND_COMPILE_NO_CONSTANT_SUBSTITUTION is set). I changed things so
> that
>>>>> there is no substitution of constants (except with CT_SUBST flag, like
>>>>> always) for compile time constants (ZEND_CT mode). They don't create
> a
>>>>> FETCH_CONSTANT opcode anyway. :-)
>>>>>
>>>>> Another thing I realized wasn't getting optimized (runtime constants),
>>> which
>>>>> can be, is with:
>>>>>
>>>>> namespace foo;
>>>>> $a = ::PHP_INT_MAX; // :: for global scope
>>>>>
>>>>> So the patch allows substitution there as well.
>>>>>
>>>>> http://realplain.com/php/ct_const_fixes.diff
>>>>> http://realplain.com/php/ct_const_fixes_5_3.diff
>
Index: Zend/zend_compile.c
===================================================================
RCS file: /repository/ZendEngine2/zend_compile.c,v
retrieving revision 1.647.2.27.2.41.2.84
diff -u -p -d -r1.647.2.27.2.41.2.84 zend_compile.c
--- Zend/zend_compile.c 24 Aug 2008 18:22:33 -0000 1.647.2.27.2.41.2.84
+++ Zend/zend_compile.c 29 Aug 2008 09:09:48 -0000
@@ -1392,7 +1392,7 @@ void zend_do_begin_lambda_function_decla
zend_do_begin_function_declaration(function_token, &function_name, 0,
return_reference, NULL TSRMLS_CC);
result->op_type = IS_TMP_VAR;
- result->u.var = get_temporary_variable(current_op_array);;
+ result->u.var = get_temporary_variable(current_op_array);
current_op = ¤t_op_array->opcodes[current_op_number];
current_op->opcode = ZEND_DECLARE_LAMBDA_FUNCTION;
@@ -3767,7 +3767,7 @@ void zend_do_end_new_object(znode *resul
*result = CG(active_op_array)->opcodes[new_token->u.opline_num].result;
}
-static zend_constant* zend_get_ct_const(const zval *const_name, int mode
TSRMLS_DC) /* {{{ */
+static zend_constant* zend_get_ct_const(const zval *const_name, int
all_internal_constants_substitution TSRMLS_DC) /* {{{ */
{
zend_constant *c = NULL;
@@ -3786,9 +3786,8 @@ static zend_constant* zend_get_ct_const(
if (c->flags & CONST_CT_SUBST) {
return c;
}
- if (mode == ZEND_RT &&
+ if (all_internal_constants_substitution &&
(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) {
@@ -3798,9 +3797,9 @@ static zend_constant* zend_get_ct_const(
}
/* }}} */
-static int zend_constant_ct_subst(znode *result, zval *const_name, int mode
TSRMLS_DC) /* {{{ */
+static int zend_constant_ct_subst(znode *result, zval *const_name, int
all_internal_constants_substitution TSRMLS_DC) /* {{{ */
{
- zend_constant *c = zend_get_ct_const(const_name, mode TSRMLS_CC);
+ zend_constant *c = zend_get_ct_const(const_name,
all_internal_constants_substitution TSRMLS_CC);
if (c) {
zval_dtor(const_name);
@@ -3827,7 +3826,7 @@ void zend_do_fetch_constant(znode *resul
zval_dtor(&constant_container->u.constant);
check_namespace = 1;
constant_container = NULL;
- fetch_type = ZEND_FETCH_CLASS_RT_NS_CHECK |
IS_CONSTANT_RT_NS_CHECK;;
+ fetch_type = ZEND_FETCH_CLASS_RT_NS_CHECK |
IS_CONSTANT_RT_NS_CHECK;
}
switch (mode) {
@@ -3843,7 +3842,7 @@ void zend_do_fetch_constant(znode *resul
zend_do_build_full_name(NULL,
constant_container, constant_name TSRMLS_CC);
*result = *constant_container;
result->u.constant.type = IS_CONSTANT |
fetch_type;
- } else if (fetch_type ||
!zend_constant_ct_subst(result, &constant_name->u.constant, ZEND_CT TSRMLS_CC))
{
+ } else if (fetch_type ||
!zend_constant_ct_subst(result, &constant_name->u.constant, 0 TSRMLS_CC)) {
if (check_namespace && CG(current_namespace)) {
/* We assume we use constant from the
current namespace
if it is not prefixed. */
@@ -3860,7 +3859,7 @@ void zend_do_fetch_constant(znode *resul
break;
case ZEND_RT:
if (constant_container ||
- !zend_constant_ct_subst(result,
&constant_name->u.constant, ZEND_RT TSRMLS_CC)) {
+ !zend_constant_ct_subst(result,
&constant_name->u.constant, (!CG(current_namespace) || !check_namespace)
TSRMLS_CC)) {
zend_op *opline;
if (constant_container) {
@@ -5166,7 +5165,7 @@ void zend_do_declare_constant(znode *nam
zend_error(E_COMPILE_ERROR, "Arrays are not allowed as
constants");
}
- if (zend_get_ct_const(&name->u.constant, ZEND_CT TSRMLS_CC)) {
+ if (zend_get_ct_const(&name->u.constant, 0 TSRMLS_CC)) {
zend_error(E_COMPILE_ERROR, "Cannot redeclare constant '%s'",
Z_STRVAL(name->u.constant));
}
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
