Re: Fixing string offsets of strings.

From: Date: Sat, 17 Dec 2011 23:54:56 +0000
Subject: Re: Fixing string offsets of strings.
References: 1 2 3 4 5  Groups: php.internals 
Request: Send a blank email to [email protected] to get a copy of this message
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

Thread (29 messages)

« previous php.internals (#56942) next »