On Sun, Dec 18, 2011 at 7:54 AM, Stas Malyshev <smalys...@sugarcrm.com> wrote:
> Hi!
>
> I think the idea behind this patch is good. I'll do final checks and apply
> it tomorrow if no objections heard until then.
>
> Some notes:
>
>
>> $s = "string"; isset($s['offset']) returns false
>> This is pretty critical, as it's the only way to detect this situation,
>> and ensure that array tests do not return positive results for strings.
>> It also catches an obvious, but previously hidden and probably serious
>> bugs in the PHP code..
>
>
> This change concerns me a bit since it's a behavior change and provided that
> most of the people aren't actually aware that $s['offset'] means $s[0] it
> may lead to subtle code breakage. Then again, doing $s['offset'] leads to
> code breakage right now - I just encountered another bug caused by this in
> production code mere days ago, code was checking that $s['offset'] is set
> without actually checking that $s is an array, and it happened to be not...
>
>
>> $s = "string"; $s[' 2 ']; and $s['2foo'] both emit errors, and return
>> false from isset() - the code pretty much uses is_numeric() internally.
>> - I'm pretty sure these would be un-intentional bugs, so behaving as
>> such seems consistent.
>
>
> Agreed, I think is_numeric is the right way.
>
>
>> I do not mind either way if $s['offset'] returns the first char or an
>> empty string, however it seemed a little inconstant to return false on
>
>
> Here I am conflicted. The right thing would be to return empty but it may
> cause some breakage. Then again, doing what we do now causes breakage right
> now too (not new in 5.4, same breakage happening in 5.3 too) so I'm inclined
> to go with false here unless somebody gives me a use case where it's useful
> for anything.
>
>
>> isset(), yet actually return a string. (although technically an empty
>> string does kind of indicate it is 'isset') - it at least leaves the
>> engine in a consistent state. Perhaps that change can wait's until 5.5..
>> thoughts...
>>
>> As for dropping the syntax for Strings eventually.. It would be nice,
>> but I'm not sure it is feasible anymore unfortunately.
>
>
> We tried to move to {} syntax but that didn't work, and probably isn't
> possible anymore without massive breakage.
> --
> Stanislav Malyshev, Software Architect
> SugarCRM: http://www.sugarcrm.com/
> (408)454-6900 ext. 227
>
Hi:
I think only trigger notice when a convertion of string to number
index is a good way (trivial bc break).
:)
thanks
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
--
Laruence Xinchen Hui
http://www.laruence.com/
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
