Bug 67719

[bor0]

Make is_numeric() consistent. Since " 1" == 1 and "1 " == 1, is_numeric() should return true for both strings.

[datibbaw]

The skip code can be put much lower inside the code, just before when the trailing length check is performed; like this.

[bor0]

I don't understand how your code excludes trailing chars

[datibbaw]

Just before the trailing length check is done, ptr is expected to be at the last character of the string, i.e. \0; so the loop consumes any white space and only then perform the check.

[sameg14]

Am I missing something here?

php > var_dump(is_numeric(" 1"));
bool(true)

[datibbaw]

@sameg14 Yes, it's not about the case of leading whitespace but treating trailing whitespace in the same way.

[sameg14]

Aah ok I get it now, definitely agree this should get fixed!

php > var_dump(is_numeric("1 "));
bool(false)

[KalleZ]

Wouldn't it make more sense to run php_trim() internally on this to strip both leading and exceeding whitespace in one go here?

Agreed that it should be fixed tho

[nikic]

@KalleZ As this code is performance-critical, using php_trim() here is not appropriate.

I'm tentatively in favor of this change, but I think that it requires an RFC (or at the least an internals discussion). The impact here is larger than just is_numeric(), this change affects many (implicit) type conversions in the language.

[bor0]

@nikic, I haven't created an RFC before, but if you (or someone else) can guide me, I can try to create it :)

[krakjoe]

@bor0 The first thing to do is start an internals discussion, to try to gauge support/consensus ... if an RFC is necessary it will soon become apparent.

You could also give this as read.

Also, this has merge conflicts (not that important, nobody can merge it without consensus whatever).

[krakjoe]

Having waited 6 months for this 3 year old PR to get moving, I'm quite confident that this is going nowhere, so I'm closing the PR.

Please take this action as encouragement to move forward with a clean PR and RFC.