On Tue, Feb 26, 2013 at 6:08 PM, Dmitry Stogov <dmi...@zend.com> wrote:
> On Tue, Feb 26, 2013 at 8:49 PM, Nikita Popov <nikita....@gmail.com>wrote:
>
>> On Tue, Feb 26, 2013 at 7:41 AM, Dmitry Stogov <dmi...@zend.com> wrote:
>>
>>> Hi Nikita,
>>>
>>> I like the idea.
>>> But note, that it may cause some unexpected behaviour and bugs.
>>> I didn't test the patch I just guess it on my previous experience
>>> introducing similar features.
>>>
>>> - usage expression in write context (e.g. passing constant by reference)
>>>
>>> function foo(&$foo) {}
>>> foo("abc"[0]);
>>>
>>> - destruction of temporary result
>>>
>>> ($a . $b)[4]; // if ($a.$b) is destroyed?
>>>
>>> - in some cases destruction of temporary result may cause destruction of
>>> final result
>>>
>>> ((object)(array("a"=>"b")))->a = "c"; // temporary object may be
>>> destroyed before assignment
>>>
>>> All these edge cases must be carefully analyzed.
>>> I like the current patch, and in case the edge cases won't require many
>>> hacks, I'm all for it.
>>>
>>> Thanks. Dmitry.
>>>
>>
>> I just played a bit with it and came up with the following (additional)
>> patch to support use in write context:
>> https://github.com/nikic/php-src/commit/31705dd8c53efe3bb52d6aae483a324e5a511ae9It
>> throws an error if the write is done on a TMP or CONST,
>>
>
> I like this patch more (it doesn't complicate executor).
>
This patch is on top of the other one, so the executor changes still apply.
This one is only about write-context, but the executor optype changes were
necessary for read-context (as in read-context CONST/TMP are valid, unlike
in write context).
However it doesn't handle BP_VAR_FUNC_ARG, where decision about read/write
> context must be done at run-time.
>
Good catch, I didn't consider that one (didn't even know it exists ^^).
This would require a few more changes:
https://github.com/nikic/php-src/commit/6f5483f006794dc78f8c341b1a6d7ad18c2e56be
Nikita
